Encapsulation: Entities, Collections And Business Rules

Yesterday, I was involved two very separate yet very related conversations. One was via twitter with Colin Jack and Jimmy Bogard (which I was only a partial contributor to – mostly just reading their conversation) and another after work with a coworker. The short version of both conversations can be boiled down to encapsulation of logic surrounding collections that are held by entities. Rather than rehash all of the conversations, I wanted to specifically address a violation of encapsulation that I’ve seen many times when dealing with collections and business rules.

Let’s look at a small example where we have a medical system that deals with Patients that are having Consultations with doctors. There is a need to keep track of the historic consultations and also the current consultation, if any. For simplicity we’ll say that the current consultation is identified as being the most recent, based on a starting date, and that any previous consultation must have an ending date. A very simplistic implementation of an object model to represent patients and consultations may look something like this:

public class Patient
{
  private IList<Consultation> _consultations = new List<Consultation>();
 
  public IList<Consultation> Consultations { get { return _consultations; } }
 
  public Consultation CurrentConsultation { get; set; }
}
 
public class Consultation
{
  public DateTime StartingDate { get; set; }
  public DateTime EndingDate { get; set; }
}

Then, when the time comes to add a new consultation to the patient, making it the current one and closing a previous consultation, code may get called from somewhere in the application (like a code behind of a form, or if you’re lucky, in the Presenter or Controller of an MVP/C setup), like this:

Consultation consultation = new Consultation{ StartingDate = DateTime.Now };
 
patient.Consultations.Add(consultation);
 
if (patient.CurrentConsultation != null)
  patient.CurrentConsultation.EndingDate = DateTime.Now;
 
patient.CurrentConsultation = consultation;

From a purely technical standpoint, there is nothing wrong with this code. It implements the business rule as defined. However, this code misses out on some great opportunities to encapsulate the rules we have into a process that can be called from anywhere that the system needs it – whether or not the code is in the specific presenter / controller that creates a new consultation or not. The very same code that comprises this implementation could easily be placed in the Patient object, abstracting the rules and process of creating a new consultation into a simple, single method call.

Before I show how I would approach that solution, though, there is one other implementation that I’ve seen recently that not only breaks encapsulation, but has to compensate for the lack of rules enforcement with logic in the wrong place. Instead of storing the current consultation as a set value, the CurrentConsultation property may have some logic in it to dynamically determine which consultation is the current one.

public class Patient
{
  private IList<Consultation> _consultations = new List<Consultation>();
 
  public IList<Consultation> Consultations { get { return _consultations; } }
 
  public Consultation CurrentConsultation 
  {
      get 
      {
          Consultation currentConsultation;
          DateTime mostRecent = DateTime.MinValue;
          foreach(Consultation consultation in _consultations)
          {
              if (consultation.StartingDate > mostRecent)
              {
                  mostRecent = consultation.StartingDate;
                  if (consultation.EndingDate == DateTime.MinValue)
                  {
                      currentConsultation = consultation;
                  }
              }
          }
      }
  }
}

This type of code is a huge encapsulation violation smell. Since our Patient object has no enforcement of the consultations that it holds, there is no way for us to really know which consultation is the current one. Because of this, the retrieval of the current consultation has to process the entire consultation collection and try to find the most recent consultation that has no ending date.

On top of the encapsulation issue, we have lost a great deal of performance. We now have to loop through the list every time we need the current consultation. If the list is small, this might not be such a bad problem, but as the list grows and as this code is used more and more, the performance problem may have a serious impact on the system.

Fortunately, the solution to the encapsulation violation, the enforcement of the business rules and the performance problem can all be wrapped up in to some very simple code. The first thing we want to do is prevent the ad-hoc addition of consultations to the patient. We still need to access the list of consultations, but we don’t really have a need to modify it outside of the patient class itself. This can be done with a one-line code change to the Patient class’s Consultations property:

public class Patient
{
  private IList<Consultation> _consultations = new List<Consultation>();
 
  public IEnumerable<Consultation> Consultations { get { return _consultations as IEnumerable; } }
 
  //ignoring other implementation details for the sake of illustrating the IEnumerable change
}

Now that we have prevented the ability to do ad-hoc consultation additions, we need a way to actually add consultations. While we are doing this, we also want to enforce the business rules of the current consultation as described earlier. This is where we are going to take much of the original code that we found in the presenter / controller and place it into the patient class directly.

public class Patient
{
  private IList<Consultation> _consultations = new List<Consultation>();
 
  public IEnumerable<Consultation> Consultations { get { return _consultations as IEnumerable; } }
 
  public Consultation CurrentConsultation { get; private set; }
 
  public void StartConsultation()
  {
      Consultation newConsultation = new Consultation{ StartingDate = DateTime.Now };
 
      if (CurrentConsultation != null)
          CurrentConsultation.EndingDate = DateTime.Now;
 
      CurrentConsultation = newConsultation;
 
      Consultations.Add(newConsultation);
  }
}

In the end, this code shows a better encapsulation of the business rules and logic that surrounds the need to maintain a list of consultations and a current consultation. With this code in place, we could simplify the presenter / controller that we talked about initially. Rather than being forced to know all of that logic in the presenter, we can make one simple method call:

 
 patient.StartConsultation();
 

With this one simple call, we have a guaranteed execution of the business rules that we need. This will allow us to recreate the ability to add a new consultation at any point in the application that we need, not just in the original presenter / controller that we were working with.

Side Note:

Part of the conversation via twitter revolved around where this type of logic should be encapsulated. From what I gathered, Colin tends to place this logic in custom collection objects, which would allow him to call patient.Consultations.Add() and still encapsulate the same business rules into that method. Like everything else in software development, there are multiple ways to solve the same problem. What does your specific situation, project, team, and business context need? Whatever your implementation needs are, though, we need to keep this type of logic and business rules enforcement well encapsulated in our systems.


Post Footer automatically generated by Add Post Footer Plugin for wordpress.

About Derick Bailey

Derick Bailey is an entrepreneur, problem solver (and creator? :P ), software developer, screecaster, writer, blogger, speaker and technology leader in central Texas (north of Austin). He runs SignalLeaf.com - the amazingly awesome podcast audio hosting service that everyone should be using, and WatchMeCode.net where he throws down the JavaScript gauntlets to get you up to speed. He has been a professional software developer since the late 90's, and has been writing code since the late 80's. Find me on twitter: @derickbailey, @mutedsolutions, @backbonejsclass Find me on the web: SignalLeaf, WatchMeCode, Kendo UI blog, MarionetteJS, My Github profile, On Google+.
This entry was posted in .NET, Analysis and Design, Design Patterns, Domain Driven Design, Model-View-Presenter, Principles and Patterns, Refactoring. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • http://jimmybogard.lostechies.com Jimmy Bogard

    I think wherever it is, the closer to the model the better. On the model == fairly rad.

  • http://www.lostechies.com/blogs/joe_ocampo/ Joe Ocampo

    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.

  • http://derickbailey.lostechies.com derick.bailey

    @Joe

    SSHHHH!!! You’re giving away all the secrets of my next post! :)

    yeah, we came to that same conclusion in the second half of our conversation last night, and re-discussed it today concluding that it really is the right thing to do. What’s really interesting is that our NHibernate mappings were the final straw that confirmed we were doing things wrong, so my next post was going to talk about design smells showing up in interesting places.

  • http://www.lostechies.com/blogs/joe_ocampo/ Joe Ocampo

    Darn…I was going to create a whole post on this concept…I won’t steel another brothers thunder. :-)

  • http://derickbailey.lostechies.com derick.bailey

    There’s plenty of room to share the stage on this subject… I believe the “short” version by Eric Evans is about 300 pages. :)

  • http://lo Colin Jack

    @Joe
    The other side of that is if/when that second requirements comes along you can refactor the model so you don’t need to design for it upfront.

    Having said that in general though I think association/relationship objects (which Consultations are) are an under-developed part of DDD, Randy Stafford raised that a few times on the forum and I hope its dealt with in any second edition.