Comments on Assignment 1

"A hospital has many patients.  Each patient has one doctor, although a doctor may have several patients.  Tests are performed on each patient resulting in many measurements that must be recorded in a data base.  In some cases, the measurements can be complicated data structures.  Examples of measurements include blood pressure, temperature, and pulse.  It's important to know the time of a measurement."
There were a few recurring issues which came up in the solutions for assignment 1.  I'll divide these into design issues and implementation issues.

Design Issues

Relationship between Measurement and Patient

Are we supposed to keep track of multiple measurements per patient?  Some solutions assumed that the system only needed to store one measurement for a given patient (presumably, the most recent measurement).  Others assumed that there could be multiple measurements per patient.  The specification doesn't address this explicitly, but there are some implicit clues:
"Tests are performed on each patient resulting in many measurements that must be recorded in a data base."
So, there are multiple tests and many measurements per patient.  Do we need to keep track of multiple measurements?  Well, it says that the measurements must be recorded in a "data base".  For our purposes, the "data base" is the storage in the program itself.  But this could also be interpreted to mean that we have some external system that's responsible for storing the measurements.

This is an important point: In the real world, specifications are always ambiguous to one degree or another.  We need to make sure that our design and implementation reflects the intent behind the spec.  Sometimes the only way to figure out the intent is to do some investigation and ask the customer.  (For the purposes of this class, I'm the customer.)

For my design, I asked the customer and found that we needed to keep track of all the measurements for a given Patient, each timestamped with the time it was taken.

Kinds of Measurements

How many kinds of Measurements can there be?  There are three listed explicitly:
"In some cases, the measurements can be complicated data structures.  Examples of measurements include blood pressure, temperature, and pulse."
But more important, the wording makes it clear that the set of measurements is open-ended; these are just examples, there may be other kinds of measurements now or in the future.  This is a good signal that we need to make sure our system can accomodate additional kinds of Measurements without much change.

A good way to do this is to use polymorphism.  Have Patient, Doctor, and Hospital deal with the Measurement base class only.  Then you don't have to change them when you add a new Measurement (BloodSugar, say).  We can use inheritance to model the situation as long as we make sure that we have an IS-A relationship:  BloodPressure IS-A Measurement if we can use a BloodPressure object wherever a Measurement object is expected.  This seems to be the case for our system; you can't do much with a Measurement other than keep track of it, ask for its timestamp, and destroy it.

Some people used aggregation to put BloodPressure, Temperature, and Pulse into a single object.  These designs seemed to assume that all three measurements are always taken at the same time.  This is a new constraint that isn't in the specification; it's easy to imagine real-world situations where this isn't the case (pulse, for example, would be measured much more often than white cell count).  Another interpretation of this design might be that you only use the measurement that is relevant at any given time; so you only fill out one of the three sub-objects.  This becomes unwieldy as the number of possible measurement types grows.  Also, each time you add a new measurement type, you have to modify existing interfaces -- never a good thing.

In my design, I derived the various measurements from a Measurement base class that is responsible for maintaining a timestamp and which provides an abstract interface to print out a measurement (a virtual printOn() function).  The various classes deal only with Measurement, so they don't need to be changed when a new type of Measurement is added.

Are Doctor and Patient Persons?

In the real world, of course, both Doctor and Patient are Persons.  (Every Doctor IS-A Person, and every Patient IS-A Person as well.)  Should this be part of our design?

I would say no.  Although this is true of the real world, in our design there is no need to treat Doctors and Patients polymorphically.  They are different roles that people can play.  Note that a real-world person can be simultaneously a Doctor and a Patient.  This makes inheritance unwieldy -- we can't change the type of an object dynamically.  One way to achieve this would be to use the Delegation pattern, in which Doctor and Patient are just "roles" that a given object can play.  But this seems like overkill for our system, since there's no place where we need to treat Doctors and Patients the same way.  We can just keep track of the roles "Doctor" and "Patient" and be happy.

On the other hand, there may be duplicated implementation that we would like to share.  For example, both Doctors and Patients might have names and IDs.  We might want to re-use the code to deal with this via a private base class or perhaps an "Identity" sub-object.  But this is really more an implementation detail than a design issue.

In my design, I did not have any relationship between Doctor and Person.  This means that there is some duplicated code to deal with the name attribute.

Implementation Issues

Dynamic Allocation

Dynamic allocation using "new" and "delete" is a powerful tool.  But in many cases, it's overkill and leads to more work than you need to do.  Consider the following classes:
class by_ptr {
    int *i;
    foo() {i = new int(5);}
    ~foo() {delete i;}

class by_value {
    int i;
    bar() : i(5) {}

Which one is easier to write?  To maintain?  Certainly by_ptr is potentially more flexible -- we could let foo objects share their integer, provide the integer from outside the class, etc.  But we're not doing any of that right now, so why pay for it?

Many people used dynamic allocation when static allocation would do.  Ask yourself, "Would I dynamically allocate this object if it were an integer?"  If not, think about making it a by-value variable.

In my implementation, I used new and delete only for those objects that need to be dynamically allocated -- objects which are polymorphic or which may outlive their creation function.

Memory Management

This is related to the last point.  Many of the implementations worked well, but didn't address memory management issues at all.  Some of these leaked dynamically allocated memory; others required that callers provide the memory and keep it around until it is no longer needed.  For example:
class named_x {
    char *name;
    named_x(char *p) : name(p) {}
A client of named_x might write:
named_x *xp = new named_x("Bob");
my_list.push_back(xp); // Keep track of Bob
This is fine as long as all strings are static literals.  Things break down when, say, we want to let users type their own names in:
while (...)
    char buf[128];
    read_name(buf,sizeof(buf)); // Reads name from terminal
    named_x *xp = new named_x(name.c_str());
    my_list.push_back(xp); // Keep track of each user
What happens the second time through the loop?  [Hint: Every named_x shares the same buffer in this program.] The client could allocate the buffer dynamically, of course.  But when can the client free it?  Only when the objects on my_list are destroyed.  This rapidly becomes a big headache.

Remember:  Objects should manage their own resources.  In this case, the name of the object might require memory for storage, so the object should manage that memory.  In this case, there are at least three reasonable ways to do it:

class named_x1 {
    enum {MAX_NAME_LEN=32};
    char name[MAX_NAME_LEN];
    named_x(char *p) {strncpy(name,p,MAX_NAME_LEN);}

class named_x2 {
    char *name;
    named_x(char *p) : name(strdup(p)) {}
    ~named_x() {free(p);}

class named_x3 {
    std::string name;
    named_x(char *p) : name(p) {}

In my implementation, I used classes from the Standard C++ Library (SCL) to help with memory management.  For example, I used std::string for strings, and std::vector to manage pointers to objects.  I still had to deal with deleting the individual objects in the vector, however.  Such is life.


Some projects were commented well, others not so well.  In general, comments should not restate what the code is obviously doing:
x=5; // Assign 5 to x
is a pretty useless comment.  Comments should be reserved for explaining things that aren't obvious from the code itself; usually this means that a comment on every line is too much, but a comment at the start of each nontrivial function is a good idea.

Here are some reasonable comments:

// Add a patient to a hospital; the hospital is
// responsible for deleting the Patient.
Hospital::checkin(Patient *p)
    // Assign the patient a doctor if
    // they don't have one already
    if (!p->getDoctor())

    // Add patient to hospital database


Note that not every line is commented.  The comment at the top should really be in the header file -- clients need to know how to use the interface, and they need to know who owns the Patient object.  The first internal comment summarizes what the next two lines of code do, and helps readers understand what rules the code is implementing.  The next comment is the lowest-level, and just translates "push_back" into "add patient to hospital database".  Note that the last line is uncommented; the function name alone should be enough to tell what it does (if not, we should change the function name!).

There are many other reasonable styles of commenting.  The most important thing is to pick a style that lets you concentrate on providing information to the reader.  A couple of good books that covers commenting practices is Code Complete by Steve McConnell and The Practice of Programming by Kernighan and Pike.

In my implementation, there were not enough comments.  Mea culpa.