Finding Design Smells In Non-Design Related Places

In my last post, I talked about the idea of encapsulation and using it to ensure that our business rules were enforced correctly. What I didn’t talk about, though, was the second half of the conversation that my coworker and I had, concerning the patent -> consultation relationship. It turns out that we had the relationship wrong. That’s not to say that patients don’t have consultations, but that the logical model we were traveling with had an incorrect perspective and was causing us to create some very ugly workarounds in various parts of the system. What really stuck out in my mind, though, was not the idea that we had the model wrong, but how we came to the conclusion of the model being wrong. It has become apparent to me, upon reflection of the conversations and situation as a whole, that design smells are not always evidenced by design related activities, if ever.

A Persistent Problem – Duplicate Redundancy

After some initial coding of the patient -> consultation relationship, we start working on the persistence model via NHibernate. What we have is a patient with a collection of consultations – this is easy to map with NHibernate’s one-to-many capabilities. We also have a CurrentConsultation property which needs to be mapped. This property is mapped to the same Consultation table, but only pull one specific consultation based on the business rules that state the current consultation is chronologically the most recent and has not ending date set.

After some thought, we found that there were a few possibilities for handling the CurrentConsultation property in our current model:

1. Create a “CurrentConsultation” object that is mapped to the Consultation table and use a “where” class attribute in the NHibernate mapping that would limit the returned result
   • Create a “CurrentConsultation” object that is mapped to a CurrentConsultation view and have the view coded to return the correct consultation object
     • Add a CurrentConsultationId field to the Patient table, as a foreign key to the Consultations table, and map to the existing Consultation object</ol> After some additional thought, though, we found that each of these solutions has a few significant problems that were going to cause a lot of trouble.

     Options 1 and 2

     Both of these options have the problem of duplicating business rules into more than one language and location. We would either have the business rules of what constitutes the current consultation in the NHibernate mapping (the ‘where’ attribute) or in a database view, in addition to the already existing code. Changing the rule would mean changing a minimum of two locations where that rule is handled. This is a bad idea no matter how you look at it.

     Both of the options have also created a duplication of knowledge from the concept of a Consultation by creating a “CurrentConsultation” class and a separate NHibernate map for it. We would have the original Consultation class and the new CurrentConsultation class both representing the same data, making an artificial distinction in our code. Again, this is a bad idea. We don’t want duplication of these logical concepts. We’re also not dealing with a bounded context or any other logical separation of concerns at this point, so there is no need to separate the concept of a consultation into multiple classes.

     Option 3

     This doesn’t appear to have the duplication issue in code, but there is a potential for duplication of data. When we get down to the implementation of NHibernate, we could easily cause duplicate data in the consultation table by saving the current consultation class. We might be able to get around this by not cascading the saves of the current consultation property, but then we’d be forced to ensure the consultation collection was persisted prior to the patient so that we could update the current consultation object’s id before saving the patient. Both of these problems sound like a serious pain to me. I’m betting it’s possible, but I’m also betting that it would be a nightmare of trial and error to get it right and a lot more code than we should really have to write.

     Changing Perspectives

     As Joe Ocampo pointed out in the comments of my original post, we had a problem in our system that was really caused by our lack of correct perspective in the situation. Rather than forcing the idea of a patient being the root aggregate in this situation, causing us a lot of headache and frustration in trying to model our persistence layer, a simple change in how we looked at the situation helped us solve the persistence problem and greatly simplified how the application worked.

     Joe’s comment (with some formatting added):

     “One thing I like to challenge developers with when I teach DDD is to flip the aggregate to determine if the model is sound.

     I know this is only an example but work with me here.  You indicated you are dealing with a medical system.  We can assume there are certain entities such as Patient, Consultations, Doctor and Practice. In your example you created a model where the patient is the aggregate root for consultations but what if the Doctor simply asked what consultations do I have today?  In this paradigm the Practice is the aggregate root and Consultations are aggregate within where Patient is an aspect of the consultation.  The code would look something like this.

     consultations = practiceService(IConsultationService).GetConsultationsFor(doctor);

     This also allows the consultation service to encapsulate its own logic for creating a consultation for creating a consultation. You can’t get any closer than that 🙂

     consultationService.CreateConsultationFor(patient).with(doctor).at(date);

     The point I am trying to make is be careful of aggregate roots.  Once you go down that path it is really difficult to back the train up and break it apart.”

     Though our actual implementation was different, this was the same basic conclusion that we had come to – our perspective on the situation was simply wrong. When we stepped back from the problem and realized that the consultation was the primary focus of the situation, and that a nurse or doctor would be the primary user of that portion of the system, it became rather obvious that our aggregate was in dire need of rework.

     A Reflective Perspective

     What we ended up with was a Patient object that dealt with all of it’s demographics information, billing information, etc, without a CurrentConsultation property or even a Consultations list. Then, on the the separated Consultation object, we added a child property of Patient. Once we realized that our Consultation object was the primary focus and made this distinction in our code, we also realized that the Patient object was carrying far too much information around the system. We found that we actually had two very distinct concepts of a patient, determined by two very distinct bounded contexts.

     • In the ‘Billing’ context, we needed all of the address , billing, and other demographics information about the patient – who they are, where they live, what their insurance is, etc. The existing Patient class filled this need.
       • In the ‘Consultations’ context, we did not need anything from the Billing context, except for the person’s name and patient id. What we really care about in the consultations is medical information about the patient – their current prescriptions, allergies, past medical care, etc. So, we created a ‘ patient’ class to represent these needs.</ul> These changed allowed for a much more clearly defined model that was truly reflective of the systems needs. We could easily see the difference between a ‘billing’ patient and a ‘medical’ patient, and we were able to code each of these areas of the system without the concerns bleeding into each other. Essentially, we decoupled the system at a module level, not just at a class level.

       We also found that the NHibernate mapping problems suddenly went away. Since the Consultation class had a child of Patient, it was a simple many-to-one mapping with no strange sequencing or duplicate data issues. In the screens that deal with the consultation directly, we load the consultation as the aggregate root and go from there. In the screens that need to show patient consultation history, we did a simple query and returned all of the consultations for the given patient. Again, we found a way to decouple our system – this time, at the persistence model.

       Design Smells: Not Just A Design Problem

       In the end, we were able to recognize a serious design smell in our system – not by the design itself, though. After all, the original code had encapsulated the needs quite well. But, as it turns out, it was a bad encapsulation at a higher level. It wasn’t until we started working with the model we had created, specifically trying to persist the model, that we realized our design was not right.

       This change was a huge breakthrough for us, not necessarily in the code or the system that was being built, but in how we look at our systems and our domain models. The realization that design smells are often evidenced not by the design itself, but by how the design is used in the infrastructure and other supporting roles of the system, has had a profound impact on how we look at system design. I’m now seeing areas of different systems that are encapsulated incorrectly, at a higher level than class design. Recognizing the problem is the first step – and we’re now working to rearrange and invert these models to more accurately reflect reality.

       Pay attention to the pain that your application, infrastructure and other supporting services are causing you. You may be staring at evidence of a design problem, without realizing it.