Specification: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.
"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."
"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.
"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.
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.
class by_ptr {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?
int *i;
public:
foo() {i = new int(5);}
~foo() {delete i;}
};class by_value {
int i;
public:
bar() : i(5) {}
};
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.
class named_x {A client of named_x might write:
char *name;
public:
named_x(char *p) : name(p) {}
};
named_x *xp = new named_x("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:
my_list.push_back(xp); // Keep track of Bob
while (...)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.
{
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
}
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 {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.
enum {MAX_NAME_LEN=32};
char name[MAX_NAME_LEN];
public:
named_x(char *p) {strncpy(name,p,MAX_NAME_LEN);}
};class named_x2 {
char *name;
public:
named_x(char *p) : name(strdup(p)) {}
~named_x() {free(p);}
};class named_x3 {
std::string name;
public:
named_x(char *p) : name(p) {}
};
x=5; // Assign 5 to xis 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 isNote 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!).
// responsible for deleting the Patient.
Hospital::checkin(Patient *p)
{
// Assign the patient a doctor if
// they don't have one already
if (!p->getDoctor())
p->setDoctor(select_random_doctor());// Add patient to hospital database
patients.push_back(p);p->log_admission();
}
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.