design question

[Tom Forsmo]

hi

In a project I just started in a came across a questionable design, and
I thought I'd ask this group how to improve on it. The simple design was
as follows (as I remember it right now), an Appointment class containing
a list of AppointmentItems (for a case worker). the AppointmentItems was
designed as follows (in uml):

appointmentItem:
...

bool preferMonday;
bool preferTuesday;
bool preferWednesday;
bool preferThursday;
bool preferFriday;
bool preferMorning;
bool preferAfternoon;


its quite obvious that this is bad design, the first alternative I could
think of would be to have a list of preferenceItems instead. But it
might be a bit overkill for this application, also I cant think of many
other preferences that might be applicable. The list would only contain
one or two items, so the extra class and code to process that list and
make decision based on that list might be a bit excessive.

An alternative solution is an enum typed array containing preferences,
but it would not be very much simpler, only a class less.

For both solutions there would have to be logic to process the
preference when used, but I am not sure what would be the best way of
doing that. The only thing that comes to mind is a switch statement, but
that of course depends on how the information would be used (which I am
not sure of). If its to be displayed, it only needs a getDescription(),
but if its for decision making then I am not sure what the best way is.

any comments on what the best design is and what's the best decision
logic, in general for this case, is?

tom

 

[Mark Rafn]

an Appointment class containing list of AppointmentItems (for a case
worker). the AppointmentItems was designed as follows (in uml):

Start with a glossary. What is an Appointment, and what is an
AppointmentItem?

appointmentItem:
bool preferMonday;
bool preferTuesday;
bool preferWednesday;
bool preferThursday;
bool preferFriday;
bool preferMorning;
bool preferAfternoon;

Do AppointmentItems have preferences? That seems strange for me. I'd expect
participants to have preferences for when their appointments (or appointment
items, whatever those are) are scheduled. Preferences of attendees are taken
into account when choosing that time.

its quite obvious that this is bad design, the first alternative I could
think of would be to have a list of preferenceItems instead.

Perhaps. It's not clear that these attributes are on the right entity, so
mucking with keeping separate values or a list of values may be insufficient
to sanitize the model.

 

[Chris Uppal]

Do AppointmentItems have preferences? That seems strange for me. I'd
expect participants to have preferences for when their appointments (or
appointment items, whatever those are) are scheduled. Preferences of
attendees are taken into account when choosing that time.

Not necessarily. I'm (like you) unsure of what exactly an "AppointmentItem" is
supposed to represent, but if it's a single appointment (i.e. an event
scheduled for a specific purpose, where the purpose would be unchanged if the
event were rescheduled); then I can easily imagine the appointment having a
preferred time (and place). E.g. if I have to go for a blood test, and the
test requires 12-hours fasting before hand (a realistic example), then I /much/
prefer the appointment to be in the afternoon, but don't much care which day of
the week it's on. But if the test didn't require fasting (or required
24-hours) then I would prefer morningtime. So, when the appointment is
scheduled, or if it is rescheduled, there will be time preferences associated
with /that/ appointment.

In a way it depends on the role of the Appointment object. If it is something
that merely /records/ an agreement ("we'll meet on Thursday at 13.00") then
there doesn't seem to be much need to record the reasons why that date/time was
chosen; but if the Appointment is the focus of the negotiation which /leads/ to
the agreement (I create an Appointment object, fill in my preferences, then
send it to you for ratification, rejection, or refinement) then it should do
so. If rescheduling (renegotiating) an existing Appointment is part of the
requirements, then that is a force pushing towards the latter design.

-- chris

 

[Chris Uppal]

bool preferMonday;
bool preferTuesday;
bool preferWednesday;
bool preferThursday;
bool preferFriday;
bool preferMorning;
bool preferAfternoon;


its quite obvious that this is bad design,

Well, it's not coded how I would prefer -- this seems like an ideal place to
use a BitSet or EnumSet -- but it's difficult to call it bad /design/ without
knowing what these objects are supposed to do (or, perhaps, what they /should/
be supposed to do ;-)

Off the top of my head, it strikes me as a bit odd that the class appears to
have no way of recording "dis-preferences" (I can't think of a real word for
that) like "can't make Monday", "mornings are out", and so on. Also the scheme
seems too weak to be able to express plausible real-world preferences like
"either Monday afternoon or Tuesday morning is OK".

Mark has raised the issue of whether an AppointmentItem (whatever that is)
should be holding preferences at all. He may have a point (see my reply to
him), but if there is a genuine need for this, and if "AppointmentItem" isn't
just a (extraordinarily) badly chosen name for a Preference (or
AppointmentPreference, or DateTimePreference); /and/ if this logic has any real
significance within the application (I'd guess it does); then it seems odd not
to have explicit preference objects somewhere in the system. Making them into
full objects would, amongst other things, allow them to be used more generally
(e.g. attached to people, attached to recurring appointments, etc).

Another reason for suspecting that Preferences should exist separately from
Appointments is that it takes at least two set of (real world) preferences to
forge an agreement -- mine and yours. Maybe the system doesn't need to reflect
that reality (e.g. a system which records a patient's preference, but actual
appointments are scheduled unilaterally by the doctor).

But it isn't really possible to say more without knowing what the system does.

-- chris

 

[Tom Forsmo]

Tom Forsmo wrote:

> But it isn't really possible to say more without knowing what the
> system does.

I was moved to another project, so I dont have access to the
documentation any more. Additionally I dont have all the details because
the description was at a junior programmer level with only a module
specification. I.e. they gave me a 3 page technical design document
which outlined the two classes to program and a description of the
methods and variables to add to it. Along with this they gave me a
requriements specification which detailed what tests the finished
classes must comply with.

I did not have time to read the system architecture documents or ask any
questions about why they designed it like this. Additionally I know the
architecture group consists of many people coming straight from
university, and I think the design shows that. (its a big and
prestigious contract with a large the consulting firm in charge, and
they need people so any people are good enough...)

Well, it's not coded how I would prefer -- this seems like an ideal place to
use a BitSet or EnumSet -- but it's difficult to call it bad /design/ without
knowing what these objects are supposed to do (or, perhaps, what they /should/
be supposed to do ;-)

Off the top of my head, it strikes me as a bit odd that the class appears to
have no way of recording "dis-preferences" (I can't think of a real word for
that) like "can't make Monday", "mornings are out", and so on. Also the scheme
seems too weak to be able to express plausible real-world preferences like
"either Monday afternoon or Tuesday morning is OK".

as far as I understood it, its supposed to hold information about
possible appointments, which a person requests with a case worker.

Mark has raised the issue of whether an AppointmentItem (whatever that is)
should be holding preferences at all. He may have a point (see my reply to
him), but if there is a genuine need for this, and if "AppointmentItem" isn't
just a (extraordinarily) badly chosen name for a Preference (or
AppointmentPreference, or DateTimePreference); /and/ if this logic has any real
significance within the application (I'd guess it does); then it seems odd not
to have explicit preference objects somewhere in the system. Making them into
full objects would, amongst other things, allow them to be used more generally
(e.g. attached to people, attached to recurring appointments, etc).

I agree, but I dont know why they did it like that. Maybe its because
they thought they could refactor that part later, or maybe its because
the case worker only needs a signal of some preference, and the person
just needs to accept the date of the appointment after that.

Another reason for suspecting that Preferences should exist separately from
Appointments is that it takes at least two set of (real world) preferences to
forge an agreement -- mine and yours. Maybe the system doesn't need to reflect
that reality (e.g. a system which records a patient's preference, but actual
appointments are scheduled unilaterally by the doctor).

I agree there can be many possible preference designs, thats why I
thought that using a preference list would be much better because you
could add new types of preferences or even an entire new preferences
engine if that is what you want. But at the same time that adds a lot of
complexity to a very very small part of this very very large system.
Another thing is, that I am not sure they need that kind of preference
alternatives. Its a goverment office so its not necessarily up to the
person to decide the appointment time, but rather it might be the case
worker that decides and the preference is just an indication. I am not
sure, though, because its a project to move the service to a more
internet friendly era, so allowing the system to match appoinement
preferences, instead of the case worker doing so, would be a good thing.

In any case, just the very idea of using a several individual instance
variables to record alternative preferences is not a very good design,
at least they could have used an enum array. That way, they could use a
for loop to test for preference match instead of a large and ugly switch
statement.

tom

 

[trippy]

Well, it's not coded how I would prefer -- this seems like an ideal place to
use a BitSet or EnumSet -- but it's difficult to call it bad /design/ without
knowing what these objects are supposed to do (or, perhaps, what they /should/
be supposed to do ;-)

Off the top of my head, it strikes me as a bit odd that the class appears to
have no way of recording "dis-preferences" (I can't think of a real word for
that) like "can't make Monday", "mornings are out", and so on. Also the scheme
seems too weak to be able to express plausible real-world preferences like
"either Monday afternoon or Tuesday morning is OK".

Mark has raised the issue of whether an AppointmentItem (whatever that is)
should be holding preferences at all. He may have a point (see my reply to
him), but if there is a genuine need for this, and if "AppointmentItem" isn't
just a (extraordinarily) badly chosen name for a Preference (or
AppointmentPreference, or DateTimePreference); /and/ if this logic has any real
significance within the application (I'd guess it does); then it seems odd not
to have explicit preference objects somewhere in the system. Making them into
full objects would, amongst other things, allow them to be used more generally
(e.g. attached to people, attached to recurring appointments, etc).

You could put in an extra attribute for the patient class for preferred
days or times for appointments. Just an idea off the top of my head.

Another reason for suspecting that Preferences should exist separately from
Appointments is that it takes at least two set of (real world) preferences to
forge an agreement -- mine and yours. Maybe the system doesn't need to reflect
that reality (e.g. a system which records a patient's preference, but actual
appointments are scheduled unilaterally by the doctor).

But it isn't really possible to say more without knowing what the system does.

-- chris

--
trippy
mhm31x9 Smeeter#29 WSD#30
sTaRShInE_mOOnBeAm aT HoTmAil dOt CoM

NP: "Heart And Soul" -- T'Pau

"Now, technology's getting better all the time and that's fine,
but most of the time all you need is a stick of gum, a pocketknife,
and a smile."

-- Robert Redford "Spy Game"