In the manufacturing world, you would never find a company that assembles a bunch of parts into a final product before inspecting any of the individual parts, and they would not wait until the end of the assembly line to test for the quality of the product. The very notion of waiting until the product is “done”, to test it, would be appalling. How much time and money would be lost trying to figure out why something didn’t fit together properly, why it didn’t work, and why there was a quality issue with the final product? Rather, we see the manufacturing world taking an active role in preventing defects. Yes, they still do a final quality inspection, but the primary means of ensuring a quality product is delivered is not by waiting until the product is assembled to test it. They build quality in from the start and maintain that quality throughout the manufacturing process.

Prying Open The Case

Imagine the inner workings of the phone to the left. This is a very complex piece of technology. Do you think Cisco would wait until they have assembled this phone and then try to pry open the case so that they can insert a set of electrodes to test and see that the circuit board is connected correctly? I certainly hope they don’t. Instead, when a manufacturing company is building something – anything – they start with the idea of preventing defects, not waiting until they are identified and correcting them. Many companies have an active Zero Defects policy where defect prevention is paramount and quality inspection is almost just a verification of what they already know – that the product is defect free.

When a part is stamped out, formed, molded, or otherwise created, it is done so to an exacting specification. After the part has been created, the part is then tested against the same specification to which it was originally built. If the part does not fall within the tolerance and guidance of the specification, it is scrapped and a new one is made. If a series of parts are found to be out of specification, it’s usually a sign that something in the process, tooling, or other portion of the manufacturing process is not right. When this happens, they fix the cause of the problem – whether the machines need to be calibrated, the people running the machines need better instructions or whatever the cause is. In the end, the specifications for the parts were used to create the part, identify whether or not the part was up to standards, and decide whether or not to keep that part.

What’s more, the manufacturing company doesn’t wait until after they start creating parts to create the specification. Rather, they take the time to properly engineer the specifications up front. They take measurements, create prototypes with varying specifications to see what works best, record the success and failure rates of the various specifications that are tried, and use other design and engineering principles to scientifically calculate the exacting specifications that will be used to produce the parts. This occurs at all levels of the product’s design and creation. For every resistor, capacitor and microchip that builds a circuit board, each one of them has their own specifications that have been carefully engineered. If any single capacitor does not meet the specifications, it is not sent to Cisco with the hopes that it works anyways. Only when all of the specifications of each part are met will they solder the parts to the circuit board, creating a subassembly.

When a subassembly is created, it also has a specification to which it was built. The subassembly then undergoes the same quality assurance process – verification that it meets the specifications and operational requirements. The process continues from here – each subassembly gets connected to a larger system which is built to a set of specifications, with rigorous testing of the larger system as it is built, ensuring it meets the specifications. When the final phone is assembled, we don’t have to worry about whether or not a specific capacitor is soldered to the correct location – we don’t have pry open the case on this phone and insert a set of electrodes to see if the electrical current is flowing correctly. Instead, we only need to plug this phone into the correct connections (an Cisco IP phone system in this case) and verify that the phone actually performs all of it’s functions, according the functional specifications of the phone. There simply is no need to verify the capacitor that was used in the very first step. We know it works because it was built in a system that actively prevents defects.

The manufacturing world is obsessed with testing. They are willing to test from the lowest possible levels of the system, out to the end-product and the behavior that is expected. They do this because the consumers of manufactured products demand perfect. Why, then, are so many software development companies so willing to only test from one end of the process? To only test once, and only from the user interface, just before the product is shipped?

A Specification By Any Other Name

Unfortunately, the software development industry as a whole, is years behind the manufacturing industry. Our definition of quality and success are often skewed and we may consider fifty or more known bugs in a system of moderate to large size to be acceptable. It doesn’t have to be this way, though. We have the technical capabilities of following in the footsteps of the manufacturing industry, and we should.

I’m sure there would be no small number of people that would say we already employ the use of specifications in software development. After all, that’s what the requirements gathering phase is for, right? So many software development companies have put so much effort, time and money into the process of producing a piece of paper that can be understood by humans, and labeled this a specification. The problem we face with paper, though, is how to effectively verify the software against what the paper says. How do we verify that series of software lines and I/O statements that are understood by a computer have actually implemented the human readable text on the paper?

We are fortunate, actually. We don’t have to accept a Word document or a piece of paper as the specification to build to. We have the ability to create executable specifications! We can, and should, be creating specifications that can measure and verify our code. Most people call it test driven development (TDD). Some call it Behavior Driven Development (BDD). I like to think of it as Specification Driven Development (SDD? Not sure if that really exists. And really, do we need another xDD acronym?). We write code that exercises our code in the form of unit tests, integration tests, functional tests, acceptance tests, or whatever you want to call them.

We’re Not Just Stamping Out Parts

One of the major problems that I have with the manufacturing/software development analogy is the obvious statement that we don’t stamp out the same parts over and over again. In spite of my previous comments on this analogy, I now think that we are more analogous to a specific part of manufacturing than I had previously understood. A more accurate representation of software development in the manufacturing world is new product design and development. The parallels work quite well from this perspective. I am not going to expound on this in great detail at this point. It should suffice to say, for the moment, that the process of product development described in Wikipedia is a fairly accurate representation of what we go through for the average software development project.

When a manufacturing company is working on a new product, they once again don’t stamp out parts without knowing what they are doing. Many different parts may need to be tested, many different designs may need to be tried, but every one of these is still built to a specification. The major difference is that the specifications used are expected to change over time, until the final specification for the final pieces are accurate enough to produce a production-ready prototype.  The same notions can be applied to the software development processes in TDD, with some additional benefits.

Built To Specs, Regression Tests And Change

Change happens. It’s a simple fact of software development. A customer thought they wanted X, but in reality they needed X-1/B – not quite what we originally thought. When this happens, we once again have a significant benefit created by our executable specifications. We only need to identify those specifications that are now wrong, correct them, and change the affected portions of the system to match the new specifications.

Our ability to change is direct evidence to one of the many benefits of TDD: regression tests. Every specification that we write becomes a regression test the moment we fulfill that specification’s requirements. With this in mind, we can work with an almost reckless abandon, free to add features, remove features, fix bugs (because let’s face it – we’re still going to find some issues somewhere in the system) and refactor the system to a higher standard, all without worry of breaking the system. We can act with this level of confidence because we have a safety net in our regression tests. If (when) we do break something in our new efforts, we will be notified the moment we re-execute our specifications – that is, run our regression tests. A specification test will fail and we will have a clear indication of what failed and why. This deep insight into the system gives us even further confidence in correcting any issues that we introduce. When a failed specification test tells you exactly which value from which class is wrong, and the context in which that class was executed is known (the exact input and expected output), pinpointing the problem becomes a rote process. Fixing the issue becomes relatively simple, and we begin to see true productivity improvements in our processes.

Start With Quality, End With Quality

Our industry is currently suffering from a lack of quality. We ship horrendously bad user experiences in products that are late and well over budget, yet we call this a ‘success’. It doesn’t have to be this way. If we change our perspective and start to take some cues from the manufacturing and product design and development world, we can dramatically increase our effectiveness as software developers. We can create high quality, low cost solutions like the world expects from manufacturers. Built to specification is certainly not a silver bullet. It is, however, the definition of quality in a Zero Defect environment.

By employing a built to specification mindset in our software development efforts, we can start with quality and maintain that quality throughout the life of our projects. This is the same process that is undertaken when a manufacturing company is working on new product design and development. It works well, it’s a proven process, and most of all – it makes sense. Build your software to specifications. Just make sure they are executable specifications.

This post is part of the November 2008 Pablo's Topic Of The Month (PTOM) - Design Patterns and will primarily outline the State pattern, with an Enumeration or Descriptor pattern thrown in for good measure.

Switch statements and if-then statements are not object oriented code. They are conditional logic for procedural control and processing. This doesn't mean that they are bad - by no means. It only means that they are not object oriented. There are many legitimate reasons for using if-then statements and a few legitimate reasons for using switch statements. However, there are also many bad reasons for using these statements.

Setting Up Shop

Before we get into the guts of the State pattern, let's consider the Point of Sale system of our coffee shop again. When a customer is placing an order, we need to track what they are ordering. This would likely be done through an Order object. While the customer is placing their order, the order is considered "in process". Once the customer is done with their order, the cashier can "total" the order. Then, the order is "tendered" (paid for). And finally, the order is "delivered" to the customer. Each of these quoted items is a state that the order goes through during it's brief run through the coffee shop. In C#, it would be very easy to represent these states with a simple enum:

public enum OrderStatus

{

   InProcess = 0,

   Totaled = 1,

   Tendered = 2,

   Delivered = 3

}

With this enum in hand, we can have our point of sale system set the correct order status according to what is going on at the moment.

//when a coffee is ordered

Product regularCoffee = new Product("RegularCoffee", 2.99);

Order order = new Order();

order.Add(regularCoffee);

order.Status = OrderStatus.InProcess;

//when an order is totaled

double total = order.GetTotal();

order.Status = OrderStatus.Totaled;

//when the customer pays for the order

order.AmmountPaid = ammountPaidByCustomer;

order.Status = OrderStatus.Tendered;

//and finally, the order is delivered

order.Status = OrderStatus.Delivered;

Now this code we have so far is technically correct - it works and it gets the job done. Unfortunately, though, this code will likely lead us down a bunch of dark paths of code duplication, ugly switch statements, and an unmaintainable mess. For example, what happens when we start integration our Order object into the rest of the coffee shop and we need to display orders to fulfill on a screen for the order runners to fill. In such a case, we only want to show orders that have been tendered but not delivered, and to try and get orders out the door faster, we'll also show orders that are totaled.  If we were going to examine a single order to determine whether or not we wanted to show it, we would likely have one of the two following pieces of code in place (yes, I know this could be done with a database query. The point is that somewhere in the code you will likely end up with switch statements or if-then statements like this):

//one way to know

bool shouldShowOrder = false;

if (order.Status == OrderStatus.Totaled || order.Status == OrderStatus.Tendered)

     shouldShowOrder = true;

//another way to know

bool shouldShowOrder;

switch (order.Status)

{

   case OrderStatus.Totaled:

   {

       shouldShowOrder = true;

       break;

   }

   case OrderStatus.Tendered:

   {

       shouldShowOrder = true;

       break;

   }

   default:

   {

       shouldShowOrder = false;

   }

}

//after we know, we show it

if (shouldShowOrder)

     ShowTheOrderForFulfillment(order);

So - what happens when the order status list change? Or if we now have a need to show a different order status on our display? We would have to find all of the locations that used this status enum and check to see if it needs to be changed, to handle the new status or change in how the status is handled. This is where the problems really start - hoping that you got all of the uses changed and all of the right states handled in the right ways. Fortunately, there's a design pattern that we can use to simplify this situation and eliminate many of the associated switch and complex if-then statements.

The State Pattern

Wikipedia describes the State Pattern as "a clean way for an object to partially change its type at runtime." The C2 Wiki gives a little more insight into the State Pattern, as well: "Allow an object to alter its behavior when its internal state changes. The object will appear to change its class."

What both of these descriptions are really getting at, is that a single concept in the real world is likely going to be modeled as more than one class in code - composed of many different pieces. When any one part of a concepts model changes, the state of that concept is changed. The relationship between our Order class and OrderStatus enum are a prime example of this composition. What the state pattern allows us to do is change the behavior of our modeled concept by partially changing the composition of our model.

Modeling Our State As State Model

To model the state of our Order with a state pattern instead of a state enum, and begin introducing the ability to change our systems behavior without changing the code that relies on the state, we need to introduce an abstraction that can represent any state we care about. To start with, we need to know what orders should be shown on our display for the order runners.

public interface IOrderStatus

{

   bool DisplayForFulfillment { get; }

}

After introducing this abstraction, we can introduce any state implementation that we need.

public class InProcessStatus: IOrderStatus

{

   public bool DisplayForFullfillment { get { return false; } }

}

public class TotaledStatus: IOrderStatus

{

   public bool DisplayForFullfillment { get { return true; } }

}

public class TenderedStatus: IOrderStatus

{

   public bool DisplayForFullfillment { get { return true; } }

}

public class DeliveredStatus: IOrderStatus

{

   public bool DisplayForFullfillment { get { return false; } }

}

Once we have these states in place, we can replace our code that sets the state with these objects.

double total = order.GetTotal();

order.Status = new TotaledStatus();

And we can also significantly reduce the code that needs to know if we should display this order in our order display system. Now, instead of having to do those if-then or switch statements, we can simply check the order status:

//the order class now relies on the IOrderStatus for it's composition

public class Order

{

   public IOrderStatus Status { get; set; }

}

Yes, we are still using an if-then statement. However, this one simple statement is no longer an issue in terms of maintainability. We don't have to change this code as we add, remove, or change the states of our system. Since we only depend on the IOrderStatus interface for our composition now, we can freely change the implementation of the states as we need - change the composition of the Order's model.

But Wait! There's More!

We don't have to stop at simple boolean values on our state abstraction - we can provide actual behavior via methods in our state. And the best part is - when we can add and remove any behavior or boolean values, or whatever else we need to change with the state of our Order, we don't have to change any of the existing code that uses the existing DisplayForFullfillment value. This property is defined on the interface, letting us use it where we need it and ignore it where we don't need it.

By using a state pattern, as we've seen here, we have introduced the ability to change the behavior of our system without having to change the places that need to know about the state of the order. We've gained a great level of encapsulation. We've gained flexibility in changing states when we need to - and we have an abstraction that hides the details when we don't need them. Seems like a winning situation to me.

But Wait! There's Still More!

Unfortunately, one thing we lose is the friendly syntax of our enumeration. For example, "OrderStatus.Totaled." Personally - I like the simplicity that enumerations offer us in our code. Something about saying "OrderStatus.Totaled" seems to be right to me. So how do we marry the state pattern to the enumeration syntax? The answer is simple - an Enumeration pattern (also called a Descriptor pattern).

We can make a simple change to our order status abstraction - make it an abstract base class. Then we can provide some static fields to provide access to the class instances that we want. To enforce the enumeration like syntax, we can also make the constructor private.

public abstract class OrderStatus

{

   public static InProcess = new InProcessStatus();

   public static Totaled = new TotaledStatus();

   public static Tendered = new TenderedStatus();

   public static Delivered = new DeliveredStatus();

   public abstract DisplayForFullfillment { get; }

   private OrderStatus() { }

}

public class InProcessStatus: OrderStatus

{

   public bool DisplayForFullfillment { get { return false; } }

}

public class TotaledStatus: OrderStatus

{

   public bool DisplayForFullfillment { get { return true; } }

}

public class TenderedStatus: OrderStatus

{

   public bool DisplayForFullfillment { get { return true; } }

}

public class DeliveredStatus: OrderStatus

{

   public bool DisplayForFullfillment { get { return false; } }

}

//the order class now relies on the OrderStatus base class for it's composition

public class Order

{

   public OrderStatus Status { get; set; }

}

And now we have our nice friendly enumeration syntax back!

//when an order is totaled

double total = order.GetTotal();

order.Status = OrderStatus.Totaled;

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
2. Create a "CurrentConsultation" object that is mapped to a CurrentConsultation view and have the view coded to return the correct consultation object
3. Add a CurrentConsultationId field to the Patient table, as a foreign key to the Consultations table, and map to the existing Consultation object

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.

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.

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.

A coworker recently asked if we should always abstract every object into an interface in order to fulfill the Dependency Inversion Principle (DIP). The question stunned me at first, honestly. I knew in my head that this was a bad idea - abstracting into interfaces for the sake of abstraction leads down the path of needless complexity. However, I wasn't able to clearly answer his question with specific examples of when you would not want to do this, at the time. I've been thinking about this for a few days now and I think I have a good, albeit very long winded, answer.

Before the question is answered, though, we need to step back and look at what DIP is all about. I've previously shown how to implement DIP and talked about why it's beneficial, so I won't be repeating that here. Rather, I want to talk about the language that describes DIP and what it really means.

Robert Martin's original definition of DIP is this:

A. High level modules should not depend upon low level modules. Both should depend upon abstractions.
B. Abstractions should not depend upon details. Details should depend upon abstractions.

The word 'abstraction' is used three times in the definition for DIP. So, in order to understand DIP, we have to first understand some of the basics of Abstraction.

Some Background On Abstraction

From Wikipedia (emphasis mine):

"In computer science, abstraction is a mechanism and practice to reduce and factor out details so that one can focus on a few concepts at a time"

I can't say it any better than this.

Abstraction can very directly lead to a system that is more understandable by helping us ignore the detail and implementation specifics, allowing us to focus on something at a higher level. This reduction in cognitive load can benefit someone that is reading the code by not forcing them to know the detail immediately. Additionally, abstraction is a form of encapsulation or information hiding, which again helps us to reduce cognitive load and produce better systems. From Wikipedia's entry on Information Hiding:

"In computer science, the principle of information hiding is the hiding of design decisions in a computer program that are most likely to change, thus protecting other parts of the program from change if the design decision is changed."

At the heart of abstraction and information hiding, we find the ability to change the system. The ability to change is an absolute requirement in software development and produces good design that is easier to work with, modify, and put back together as needed. The inability to change is directly called "bad design" by Robert Martin, in the DIP article.

Applying Abstraction And Encapsulation To DIP

So how does our knowledge of abstraction and information hiding play into DIP?

First and foremost, DIP never states that we should depend on explicit interfaces. Yes, in C# we have an explicit Interface as a form of abstraction. It is a separation of the implementation detail from the publicly available methods, properties, etc, of a class. Some languages, such as C++, don't have an explicit construct for interfaces, though. From Robert Martin's original DIP article, again:

"In C++ however, there is no separation between interface and implementation. Rather, in C++, the separation is between the definition of the class and the definition of its member functions."

A String As An Abstraction

Abstraction does always mean explicit interface constructs, as evidenced in C++. Nor does it always mean an abstract base class, which we also have available in .NET. In fact, languages such as Ruby don't really need either of these constructs. The duck-type nature of Ruby allows an implementation to be replaced at any point, without any special constructs. In .NET, though, there are a number of abstraction forms that we can rely on, explicitly. We have the obvious interfaces and base classes (abstract or not) - but we also have constructs like delegates and lambda expressions, and even the simple types that are built into the base class library.

Let's look at a simple string to illustrate abstraction. As I said in my SOLID presentation at ADNUG, we can invert our dependency on database connection information.  Rather than putting a connection string directly into our code that calls the database, we can use the string as our dependency and our abstraction. All we need to do is follow the basic DIP principle and provide the string as a parameter to the class that calls the database. We certainly don't need (or want, for that matter) to introduce a new interface or base class at this point. Our abstraction is simple enough to use a common type found in the .NET framework.

Other Forms Of Abstraction

Even if we are talking about an object, who says that the interface we are depending on has to be an explicit interface construct or base class? When I write a Domain Service that uses an Entity from my Domain, I don't create an explicit interface for that Entity. Rather, I use the Entity's inherent interface - it's public methods, properties, etc.

I also use delegates on a regular basis. By specifying my abstraction as a delegate, I can further decouple the depending object from the dependant code that it needs to call. I'd be willing to bet you have used delegates as abstractions as well. Have you ever created an event handler for something like a button click? There's a delegate's abstraction at work.

Abstract Judgement

The point is, there is not always a need to introduce an explicit interface or base class when inverting our dependencies. We still need to apply dependency inversion and provide our implementation as a constructor parameter (or setter, though I don't like setter injection). But, that dependency doesn't have to be anything more than the interface inherent to the object, or a simple type found in .NET.

You do have to be careful when making the call to not use an explicit abstraction with DIP, though. You can quickly turn your system into a ball of mud if you rely on a concrete class that is not intention revealing or well encapsulated to begin with. At the same time, too many abstractions can lead to needless complexity and make it very difficult to see the big picture of a system. Too few abstractions, though, will certainly lead to a rigid, immobile design that is hard to change. All of these problems are equally vicious - and I've been bitten by all of them in recent months.  It takes good judgement calls to determine when you do and do not need an explicit abstraction for a dependency. Unfortunately, good judgement comes from experience and experience comes from bad judgement. Don't be afraid to make bad decisions - make a decision, just be sure you can reverse that decision as easily as possible.

A few months ago, I posted some thoughts and questions on the proper use of Inversion of Control (IoC) containers and the Dependency Inversion (DI) principle. Since then, I've had the opportunity to do some additional study and teaching of DI, and I've had that light bulb moment for the proper use of an IoC container. I haven't talked about it or tried to present the info to my team(s) yet, because I had not verified my thoughts were on the right track - until recently. I got to spend a few hours at the Dovetail office with Chad, Jeremy, Josh, etc, and had the pleasure of being able to pick their brains on some of the questions and thoughts that I've had around DI and IoC. In the end, Chad confirmed some of my current thoughts and helped me put into a metaphor that I find to be very useful in understanding what Dependency Inversion really is - a cloud objects that can be strung together into the necessary hierarchy, at runtime.

Consider a set of classes that need to be instantiated into the correct hierarchy so that we can get the functionality needed. It's really easy to have the highest level class - the one that we really want to call method on - instantiate the class for next level down, and have that class instantiate it's next level down, and so-on, like this:

This creates the necessary hierarchy, but breaks the core object oriented principle of loose coupling. We would not be able to use ThisClass without bringing ThatClass along with it, and we would not be able to use ThatClass without bringing AnotherClass along with it.

By introducing a better abstraction for each class and putting Dependency Inversion into play, we can break the spaghetti mess apart and introduce the ability to use any of these individual classes without requiring the specific implementation of the dependent class.

For starters, let's introduce an interface for ThisClass to depend on and an interface for ThatClass to depend on.

Now that we have an interface that both of these classes can depend on, instead of the explicit implementation of the child object, we need to have the expected child object implement the interface in question. For example, we expect ThatClass to be used by ThisClass, so we will want ThatClass to implement the IDoSomething interface. By the same notion, we want AnotherClass to implement the IWhatever interface. This will allow us to provide AnotherClass as the dependency implementation for ThatClass. Our object model now looks like this:

What we have now is not just a set of classes that all depend on each other, but a "cloud" of objects with dependencies and interface implementations that will let us build the hierarchy we need, when we need it.

The real beauty of this is that we no longer have to care about the implementation specifics of IDoSomething from ThisClass. ThisClass can focus entirely on doing it's duty and calling into IDoSomething when it needs to. And, by passing in the dependency as an abstraction, we're able to replace the dependency implementation at any time - runtime, unit test time, etc. This also makes our system much easier to learn and understand, and most importantly - easier to change.

Now that we have our cloud of implementations and abstractions in place, we will need to reconstruct the hierarchy that we want so we can call into ThisClass and have it perform it's operations. Here's where Dependency Inversion meets up with Inversion of Control.

• To create ThisClass, we need an implementation of IDoSomething
• ThatClass implements IDoSomething, so we'll instantiate it before ThisClass
• ThatClass needs an instance of IWhatever
• AnotherClass implements IWhatever, so we'll instantiate it before ThatClass
• Once we have AnotherClass instantiated, we can pass it into ThatClass's constructor
• Once we have ThatClass instantiated, we can pass it into ThisClass's constructor

We end up with a hierarchy of objects that is instantiated in reverse order, like this:

We have now successfully inverted our system's construction - each implementation detail is created and passed into the the object that depends on it, re-creating our hierarchy from the bottom up. In the end, we have an instance of ThisClass that we can call into, with the same basic hierarchy of classes that we started with. The real difference is that now we can change this hierarchy at any time without worrying about breaking the functionality of the system.

Once we have our Dependency Inversion and Inversion of Control in place, we can start utilizing the existing IoC frameworks to automatically create our hierarchy of objects based on the advertised dependencies (an advertised dependency is a dependency that is specified as a constructor parameter of a class). Tools like StructureMap, Spring.net, Windsor, Ninject, and others, all provide automagic ways of creating each dependency of the object that is requested, all the way up/down the hierarchy. Utilizing one of these IoC containers can greatly simplify our code base and eliminate the many object instantiations that would start to liter our code. As I said in my previous post, I know all about what not to do with IoC containers. Good IoC usage, though, is another subject for another post.

After some additional Twitter discussion with Jimmy last night, I realized that my previous response had neglected to distinguish between two very important contexts - new code and legacy code.

Considering a legacy system (as defined by Michael Feathers), I'm 100% on board with everything Jimmy is talking about and all of the techniques, troubles, and diminishing returns that he identifies.

I still maintain that 100% coverage should be the default result of true TDD when writing new code - and I apply this philosophy when writing new code inside of a legacy system. Write a test to prove you need the new code that you are writing... unfortunately, the distinction between new and legacy code does get blurry and can be difficult to navigate when working in a legacy system.

For more info on legacy code, in this context, and how to deal with it:

• Working Effectively With Legacy Code - the book by Michael Feathers
• Working Effectively With Legacy Code - Article on ObjectMentor by Michael Feathers (consider it the cheat-sheet for the book)
• Jimmy Bogard - He's done a "Legacy Code" presentation several times, based on his own use of the Michael Feathers techniques.

I've talked about code coverage and software quality with a number of people on Twitter in the last month or so, including Jimmy Bogard, who has a nicely written post about Quality and code coverage.

It's no secret that I have some fairly radical opinions on code coverage and that I don't agree with Jimmy in these regards. I firmly believe that 100% code coverage, in whatever form we can get, is a reasonable goal and not past the point of diminishing returns. As such, I'd like to respond to some of what Jimmy is saying and expand on it with my own thoughts.

But first - I don't like to draw hard lines between "Unit" tests and "Integration" tests - that just blurs the problem even more because we can argue that you are covering code or not, in "unit" tests all day long, based on this blurry line. This is not the discussion I want to have, so I am lumping "Unit" and "Integration" tests into "Unit" tests as a whole.

Secondly, I'd like to say that code coverage is not always an indication of quality - in poorly written systems, it's merely a measure of code coverage. However, quality code is another subject of much debate.

On to the response!

.............................................

"The idea is that the team, all practicing TDD, should dutifully measure and add unit tests until they reach the assumed pinnacle of unit testing: 100% coverage."

There are several assumptions and statements in this that I don't believe are true.

For one, TDD and Unit Testing are not the same - no where near the same. Unit Testing lets you add tests as you see fit - after you write the code, while you write the code, whenever you want. You can even do test-first unit testing. True TDD, however, is a pure pragmatic approach to software development in that you never write code that does not have proof of need to exist - a test that says it needs to be there. 

The "pinnacle" of TDD and Unit Testing are not the same thing. Unit Testing - the act of adding unit tests when you see the need - may have a goal of 100% coverage. TDD, on the other hand, has a goal of only implementing the code that has been specified as needed. 100% coverage is the default in TDD because you never write code you don't need.

If you are going through the "measure and add unit tests" process, I would put money down to say that you are doing Unit Testing and very little TDD. The process should be something more like "measure to make sure we haven't added anything we don't need yet". 100% coverage is a side effect of TDD.

"The general motivation behind 100% coverage is that 100% coverage equals zero bugs."

Unfortunately, this is the motivation for many people who write unit tests - but it's the wrong motivation. You will never be 100% bug free just because you have 100% code coverage. There will always be some business case or external system factors that cause a bug now and then. That doesn't mean we should be ok with letting bugs into our system. On the contrary, one of our motivations (not the only one) should be to prevent as many bugs as possible from getting into the system.

"NCover is a powerful tool, but it still doesn't support all types of coverage.  Attaining 100% coverage in NCover still means there are paths that we haven't tested yet, which means there are still potential bugs in our code. "

I think the logical conclusion of this argument is that if we don't have a tool that supports true 100% coverage, then we shouldn't use that tool at all. I know I'm inserting my own conclusions into Jimmy's words - that's simply the conclusion that I came to from these statements. And this is a very bad conclusion. Do you walk around naked simply because wearing clothes would get them dirty and you'd have to wash them? Both of these are akin to the broken window syndrome. Use the tools that are available to the extent that they can operate, and find better tools when you can.

"If 100% coverage is a goal,"

100% coverage should NEVER be a goal - it's merely an indication that you are on the right path and working pragmatically.

"In recent projects where we measured coverage several months in to the project, we saw regular numbers of 90% coverage.  This was on a team doing 100% TDD." ... "So are we doing TDD wrong?"

So, are you doing TDD wrong? Yes. You are unit testing in the guise of Test-First-Development and not adhering to the spirit or pragmatism that TDD wants.

"Every test introduced covered behavior we considered interesting.  If behavior isn't interesting, we don't care about it. "

If you have code that is not interesting and you don't care about it - why is it in the system? If you don't care about the code, then it makes no difference whether or not that code works correctly. I would be appalled to hear that this is true. If it is true, then that code should not be in the system to begin with.

"If tests are a description of the behavior of the system, why fill it with all the boring, trivial parts?  The effort required to cover triviality is just too high compared to other ways we can increase value."

I think your definition of "behavior" is wrong. From wordnet.princeton.edu/perl/webwn:

Behavior: "the action or reaction of something (as a machine or substance) under specified circumstances;"

So, how is throwing an exception when a parameter is null, not behavior? If you are doing defensive programming to make sure you have all the data you need, then you are creating behavior - system behavior, not business process behavior. Behavior is still behavior, though. The only possibility of code not being behavior is a simple data point. If you have a data point in your code that is not covered by a unit test of behavior, then you don't need that data point. Yes, yes, I know - "integration", "nhibernate", "not my data source", "it has to be there for ...". If you argue that you need a property for some external portion of the system to operate correctly, then you should be proving that you need the property by covering it with a unit test that specifically says what it is for and shows how it is used. If you don't, then there's no way of knowing why the property exists and it may be deleted because it looks like nothing in the system uses it. Or, possibly worse, you end up with a lot of dead code in your system because you are afraid of deleting anything.

"Missing in the 100% coverage conversation is the effort required to get to 100%.  Attempting to get another 5% takes equal effort of the previous 10%.  The next 2% takes equal effort of the previous 15%, and so on."

I'd like to know where you got the math. If you are talking about unit testing - even test-first unit testing - i can understand the pain you're talking about, here. Why should I bother setting up another test fixture and getting all the state of my objects in place for a simple null reference check? blech - that totally sucks, is boring, is tedious, etc.

However, my typical process of achieving 100% is to only write code that is needed for the system to work, by specifying how the system will work through tests and then coding it. If I have a null reference check somewhere in my code it should be because I already have a test that specifies why it's needed.

"This is called the law of diminishing returns.  As we get closer and closer to 100%, it takes vastly more effort to get there.  At some point, you have to ask yourselves, is there value in this effort? "

Here's the real crux of the problem - you are assuming that you started with, or at some point has less than 100% coverage. It takes zero additional effort to stay at 100% coverage if you start at 100% coverage and maintain it through pragmatic TDD.

"Often, bending code to get 100% can decrease design quality, as you're now twisting the original intent solely for coverage concerns, not usability, readability or other concerns."

If you find yourself bending code to get 100% coverage, you have one of two situations (if not both): 1) poor design in your code, 2) poorly written tests. I would bet that you have both because they are a vicious circle.

"Measuring coverage is an interesting data point, as are other measures such as static analysis.  But in the end, it's only a measure, an indication. "

100% agreed!

"It's still up to the team to decide on the value of addressing missing areas,"

This implies that you don't have 100% coverage, already. In this situation, 100% agreed. I'm in this situation right now - it's hard to know what tests create the most value, at this point. However, any new code - even code changes to existing code - are done TDD style. So, while we don't yet have 100% coverage, we are increasing coverage all the time by never writing code without a test (that's part of the goal, anyway... no one is perfect, and it takes serious discipline to do this)

"with the full knowledge that they are still limited to what the tool measures."

Agreed, again. If you don't know the limitations of the tools you are using, you're in trouble.

.............................................

The boilerplate argument that I am making is that you should never write code that is not required by a customer / consumer. However, as I've Previously Talked About, there's a high likelihood that you have not identified all of your customers / consumers. Take NHibernate, for example. I have a project that needs about 30 different properties for a specific NHibernate query to be executed. The NHibernate query is the only place they are ever used - no code reads them, only a search screen writes to them, then the NHibernate query loads data from the table that they are mapped to. When I first wrote this code, my coverage was at around 30%. This situation is one of many that I've been in recently, that has lead me to believe that other code must be considered a customer / consumer of your code. If I deleted any of these properties, then the NHibernate query would fail. Therefore, the properties are valuable to NHiberate. Since these properties are valuable to at least one of the consumers of my code, I should prove that they need to exist by writing a test that proves they are needed and why.

I gave a presentation to my team on S.O.L.I.D. software development principles, talking about how these five principles enable us to achieve High Cohesion, Low Coupling, and proper Encapsulation in our software projects. If you're unfamiliar with the SOLID principles, they are:

• SRP: Single Responsibility - One reason to exist, one reason to change
• OCP: Open Closed Principle - Open for extension, closed for modification
• LSP: Liskov Substitution Principle - An object should be semantically replaceable for it's base class/interface
• ISP: Interface Segregation Principle - Don't force a client to depend on an interface it doesn't need to know about
• DIP: Dependency Inversion Principle - Depend on abstractions, not concrete detail or implementations

You can find a lot of good info on SOLID via Robert Martin (the man who coined the acronym) and the Los Techies crew:

• http://butunclebob.com/ArticleS.UncleBob.PrinciplesOfOod
• http://www.lostechies.com/blogs/chad_myers/archive/2008/03/07/pablo-s-topic-of-the-month-march-solid-principles.aspx 

The final slide in my presentation shows the before and after of migrating a small app that reads a file and sends an email, from being 100% coded in the Winforms code, to a SOLID code structure.

During the summary / review, I related the five SOLID principles to High Cohesion, Low Coupling, and Encapsulation through these descriptions:

Low Coupling:

By abstracting many of our implementation needs into various interfaces and introducing the concepts on OCP and DIP, we’ve created a system that has very low coupling. Many of these individual pieces can be taken out of this system with little to no spaghetti mess trailing after it. Separating the various concerns into the various object implementations has also helped us ensure that we can change the system’s behavior as needed, with very little modification to the overall system – just update the one piece that contains the behavior in question.

High Cohesion:

This really is a direct result of low coupling and SRP – we have a lot of small pieces that can be stacked together like building blocks to create something larger and more complex. Any of these individual pieces may not represent much functionality or behavior, but then, an individual piece isn’t much fun to use without a bunch of other pieces. DIP has also allowed us to tie the various blocks together by depending on an abstraction and allowing that abstraction to be fulfilled with different implementations, creating a system that is much greater than the mere sum of it’s parts.

Encapsulation:

True encapsulation is not just making fields private and hiding data from external objects – but hiding implementation details from other objects, depending only on the abstractions and expected behaviors of those abstractions. LSP, DI, and SRP all work hand in hand to create true encapsulation in the new project structure. We’ve encapsulated our behavioral implementations in many individual objects, preventing them from leaking into each other – and we’ve ensured that the dependency on those behaviors is encapsulated behind a known interface. We’ve hidden the implementation details and allowed for any implementation to be put in place for that interface definition through DIP. At the same time, we’ve done the necessary due-diligence to ensure that we are not violating any of the individual abstraction’s semantics or purpose (LSP), ensuring that we can properly replace the implementation as needed.

The end result of our effort has created a system that appears to be complex on the surface – after all, there’s now 13 blocks in our diagram compared to the original 2. However, the apparent complexity of this system is diminished by the simplicity of each individual interface and object. Many small, independent, simple pieces have been wired together to create a larger overall system behavior that can be described as complex. Yet any of these implementations can be changed and/or replaced – very easily.

At some point, I'm hoping to post the entire slide deck and presentation / code, but for now I thought the summary that I sent to the team was worth sharing with the rest of the world.

Earlier this week, I was doing a training session with my current team, building a sample Org Chart application. One of the user stories we were working with, talked about assigning managers to department and had the following acceptance criteria:

• When assigning a manager to a department, ensure that they are an employee of that department, and remove them from their old department

The basic code that we came up with worked well and was encapsulated into a service object in the domain.

public class AssignmentService

{

    private IDepartmentRepository _repository;

    public AssignmentService(IDepartmentRepository repository)

    {

        _repository = repository;

    }

    public void AssignManagerToDepartment(Employee manager, Department assignedDepartment)

    {

        Department previouslyAssignedDepartment = _repository.GetDepartmentForEmployee(employee);

        if (previouslyAssignedDepartment != assignedDepartment)

        {

            previouslyAssignedDepartment.RemoveEmployee(manager);

            _repository.Save(previouslyAssignedDepartment);

        }

        assignedDepartment.AssignEmployee(manager);

        assignedDepartment.AssignManager(manager);

        _repository.Save(assignedDepartment);

    }

}

Problems

At first glance, this code seems simple enough, but we quickly ran into a problem when we started coding for another story and acceptance criteria.

• When creating a department, allow a manager to be assigned

Off-hand, this seems like a very simple situation. We'll just call the AssignmentService object after we create the Department. However, there's a large potential problem with this - the AssignmentService handles the entity persistence for both of the departments that were changed, when the manager is assigned. With that in mind, the developer working on the new requirement would have to ensure that the department is created with all of it's required fields and information before calling this service object. If the developer allows the manager to be assigned before the department's info has been completely filled in, we may accidentally allow an invalid state for a department object when calling the AssignmentService. Since the AssignmentService tries to save the department being assigned, we may also have created an semantic coupling between the department creation process and the AssignmentService - the developer needs to know that the AssignmentService will save the department. If they don't know this, they may try to save the same entities more than once, may try to save an invalid state, etc, etc, etc.

Unfortunately, we didn't have time to fix the problem during the training session - we had to stop the training almost immediately after we discovered this issue. I've got a few ideas running around in my head, on how to fix this, but I'm not 100% sure that I like any of them.

Possible Solutions

First and foremost, though, it's becoming rather apparent to me that we want to avoid entity persistence calls from within the domain service objects. This essentially means that the AssignmentService object would have to return the assigned department, and the 'previously assigned department' objects, so that the coordinating presenter could save them both. That seems a little odd, if you ask me.

One of my coworkers suggested that we allow the service to save the previously assigned department, since that is an self-contained operation in the method (the method loads the department, modifies it, and saves it) and then return the assigned department so that the presenter can save it. This option seems a bit better off-hand, but I'm still worried about the possible side effects of the AssignmentService saving the previously assigned department.

Questions

What are your thoughts on this situation... How would you solve the dilemmas that I'm pointing out? Is there a better design for the model that would eliminate this problem and make the department persistence more obvious, without having to 'know' when it is persisted?

Is the idea of not allowing entity persistence from a domain service object valid? Should we go with allowing the self-contained operations to persist the entity in that operation?

Or ???

During a TDD/BDD training session with my team, today, we wrote the following code:

public void SaveRequested()

{

    Employee employee = Department.CreateEmployee(firstName, lastName, email);

    if (employee == null)

        repository.Save(Department);

}

I ignored this when we first wrote it, as it was "technically correct", but later returned to this code as a point of refactoring for greater insight and expressiveness. When we came back to it, though, I started off by asking a series of questions:

• Why would that method return a null?
• Why would it not return a null?
• Why is it important to not call repository.Save if the employee is null?

The code is essentially a mystery to anyone that hasn't memorized the details of the Department.CreateEmployee method. Worse yet, I can make a lot of assumptions about this code - but every one of them is going to be wrong until I can prove that they are right by reading the code in Department.CreateEmployee.

So how do we refactor this code for better insight and more expressiveness? How do we change this code so that the assumptions a reader creates in their own mind are more likely to be correct - or better yet, are irrelevant because you don't need to even care about the details of the Department.CreateEmployee method? First and foremost, you need to understand what this code is supposed to be expressing - what it's supposed to be doing and why. If you don't know, you need to ask the person that wrote it. If they don't know (or aren't around), you need to find someone that can help you determine the intended behavior. Ultimately, you may need to dig into the Department.CreateEmployee method to figure it out for yourself.

In this situation, we were trying to express some business rules from a set of acceptance criteria on a user story:

• When creating an employee, the employee must belong to a department
• When creating an employee, require a first name, last name, and email address

This code is a factory method on the Department object that is used to ensure these rules are satisfied. In this case, if CreateEmployee returns a null, one of these rules was violated and we should not attempt to save the Department (with the associated employee, or lack there-of). If CreateEmployee returned an employee, we satisfied the rules and we can save the Department and it's associated employees.

The code that we have in place doesn't express any of that intent, though. What we really want is code that tells us that the employee creation failed - and probably why it failed. Fortunately, the changes can be extraordinarily simple.

public void SaveRequested()

{

    EmploymentResult employmentResult = Department.Employ(firstName, lastName, email);

    if (employmentResult.Success)

    {

        repository.Save(Department);

    }

    else

    {

        view.DisplayError(employmentResult.FailureReason);

    }

}

UPDATE: Thanks to Martin Linkhorst for the comments, and making this code even more expressive (changing from Department.CreateEmployee to Department.Employ)

By creating an AddEmployeeResult object and supplying a few additional bits of information in it, our code became much more expressive of it's intent. We can now read the SaveRequested method and have a much better chance of knowing what it is really doing - try to create an employee and if it successfully created, then save it. Otherwise, display the failure reason.

What assumptions does your code create in the minds of the readers? Are these assumptions correct, incorrect, or unintelligible? And finally, what can you do to make your code assumption-free and/or assumption-irrelevant?

Over the weekend, I spent some time refactoring some business rules in my current project, to use the ISpecification<t> framework that I previously created.

One of my coworkers had asked about logical groupings - making sure that we can handle complex logic such as, "(this and this) or this". Since my project actually made use of logical groupings I wanted to share with the world and show how easy it is.

In this example, I have a Date Range that is being selected – a From date and a To date. The rules are as follows for validating the range:

• If a From date and a To date are provided, the From date must be equal to, or before, the To date
• Or, you can specify a From date without To date
• Or, you can specify a To date without From date

The following is the actual code that I’m using to define these business rules, via the ISpecification<t> framework. 

PredicateSpec<ExtractSearchCriteria> fromDateProvided = new PredicateSpec<ExtractSearchCriteria>(crit => crit.FromDate > DateTime.MinValue);

PredicateSpec<ExtractSearchCriteria> toDateProvided = new PredicateSpec<ExtractSearchCriteria>(crit => crit.ToDate > DateTime.MinValue);

PredicateSpec<ExtractSearchCriteria> fromDateBeforeToDate = new PredicateSpec<ExtractSearchCriteria>(crit => crit.FromDate <= crit.ToDate);  

ISpecification<ExtractSearchCriteria> validDateRangeSpec =

    (

        (fromDateProvided.And(toDateProvided))

        .And(fromDateBeforeToDate)

    )

    .Or(

        fromDateProvided.Not(toDateProvided)

    )

    .Or(

        toDateProvided.Not(fromDateProvided)

    );

Here, I am defining the core of the business rules – to check if the From date was provided, to check if the To date was provided, and to check if the From date is before or equal to the To date. From these core specifications, I can build the aggregate spec that handles all of the stated rules for validating the date range.

There are three logical groupings using parenthesis to represent each of the business rules that can comprise a valid date range. The first logical grouping checks to see if both a From and To date are provided, and checks if the From date is before or equal to the To date, if both are provided.

(

    (fromDateProvided.And(toDateProvided))

    .And(fromDateBeforeToDate)

) 

The second group is specified with an Or, and checks to see if we provided a From date but not a To date.

.Or(

    fromDateProvided.Not(toDateProvided)

)

And the third group also uses an Or, and checks to see if we provided a To date but not a From date.

.Or(

    toDateProvided.Not(fromDateProvided)

)

The end result is that we have one of three possible ways for this date range to be valid – all logically grouped and relatively easy to read (compared to a bunch of if-then statements, at least). Even better – we re-used two out of three simple ISpecification<t> objects to create all of the rules for this validation check. This helps to create a validation that is not only easier to read, but easier to maintain and enhance in the future.

Myself and 10 other developers in my company went through a day of BDD / TDD training, with Scott Bellware, yesterday. It was a lot of fun, very challenging at times, and covered a lot of topics including an overview of Agile and Behavior Driven Development, all the way down to writing Specification Tests, doing Test Driven Development  and refactoring the model to improve readability, maintainability, flexibility, etc. I took notes via index cards (love that cool-aid) and wanted to share. I don't expect these notes to make sense to everyone. Hopefully it will spark some dialog in someone's mind and cause them to dig further.

First off - the quote of the day.

"Can I be honest with you and say that I've been wanting to touch your keyboard, all day?"

Now for my notes.

User Stories

[Role], [Goal], [Motiviation]

• As a [role], I want to [goal], so that [motivation]
• Example: As a nurse, I want to record a patients vital signs, so that I can determine their medication and care needs
• Motivation is critical - it determines how the development team understands and implements the story. It determines the user experience, how things are integrated, how the software is designed, etc.

Acceptance Criteria

• Acceptance Criteria is used to drive code, not the story, directly
• may change at any point, up to implementation
• is used to drive code design, test design, implementations, etc.
• should be spoken in domain language
• may include non-functional, technical details such as database tables, infrastructure, performance, etc
• All acceptance criteria must be met and tested / verified before a story is considered done

Specification Tests

• Test Fixture per Class is an anti-pattern (on a personal note, this problem bothered me for months before I discovered BDD)
• Context Specification or Behavior Specification testing
• When [verb] then [verb]
  • "When [verb]" is the context
  • "Then [verb]" is an observation of the behavior
• Based on Acceptance Criteria, but not "code-gen'd" from acceptance criteria

Story Estimation

• Agile Poker: uses generalized Fibonacci sequence as order of complexity
  • "?", 0, 1/2, 1, 2, 3, 5, 8, 13, 20, 40, 100, infinite
• everyone throws their estimate at same time
• if estimates have significant outliers, discussion occurs to understand why, get more detail, etc. and re-throw may happen

Entity Data vs. Aggregate Data

• Entities should never contain aggregate data
• Aggregate data is for reporting and other aggregate needs
• If you need aggregate data to process something, write an SQL query, stored proc, etc. - don't use an ORM like NHibernate
• We don't want a "Customer" entity to need 10,000 "Order" entities, to aggregate data for processing; write a query to aggregate instead
• We don't want to persist data that can be calculated / aggregated, generally (performance issues may override this)

Domain Services

• Can have dependencies on external systems
• are part of domain logic, therefore are in domain model / assembly
• are "Doers" of process that don't fit into entity and entity logic, directly
• coordination of entity logic
• can include calls to data access, logging, etc.

Continuous Integration

• Not just continuous compilation of code
• Full end to end integration of all code, components, databases, services, etc
• Full suite of integration testing including database testing
• Do not allow commits if build is currently broken
• do not allow defects to live - fix immediately, to fix build
• "Defect" is broken software, "Bug" is functional but wrong

Daily Scrum

• 3 Questions everyone answers:
  • What did I do yesterday?
  • What am I doing today?
  • What issues am I having?
• Each person should answer quickly - 1 or 2 minutes, max
• further discussion happens outside of the Scrum meeting

Productivity of Dev Team

• RAD and other non-review, non-iterative based management causes problems and loss of productivity
• we need constant review of the design to ensure good design
• shorten the feedback loop and get constant review of the design, to always improve the design, via pair programming, work cells, retrospectives, etc.
• good design will cause productivity gains in the development team beyond the capabilities of any tools

Whiteboard Diagramming vs. Details Specs

• White board diagramming and human interaction is always better than detailed documents and specs in UML
• Human interaction leads to knowledge crunching and learning, not just reading a repeating
• Take pictures, don't re-draw in UML; don't waste time with it
• Video the entire conversation is even better, so others can learn from the knowledge crunching that occurs; capture the human interaction, body language, etc.

For the last few months, I've had a very small base class that abstracts out the NHibernate configuration, session creation, etc. It works very well, but is very limited in what it can do. basically, every method in my actual repository implementation would have to open a new session, execute a criteria and close the session. A typical implementation would look like this:

public ICollection<Invoice> GetAll()

{

    ICollection<Invoice> invoices = null;

    try

    {

        OpenSession();

        invoices = Session.CreateCriteria(typeof(Invoice)).List<Invoice>(); 

    }

    finally

    {

        CloseSession();

    }

    return invoices;

}

That certainly is easy and keeps the code fairly clean, removes a lot of duplication, etc.

However, there is a significant limitation - I can't have any code re-use for multi-query scenarios, without duplicating code. In other words, if I want to load that list of invoices and then load some other collection from another repository, I have to use two different Sessions across two different repository implementations. This really becomes an issue when dealing with transactions - I want my entire change set to pass or fail in a single transaction. In my current abstraction, this can't be done.

Fortunately, NHibernate has the solution to my dilemma built right in - all I need to do is create my criteria objects without a session, and then I can execute any / all of them from any session that I want.

public ICollection<Invoice> GetAll()

{

    ICollection<Invoice> invoices = null;

    try

    {

        DetachedCriteria criteria = DetachedCriteria.For<Invoice>();

        OpenSession();

        invoices = criteria.GetExecutableCriteria(Session).List<Invoice>(); 

    }

    finally

    {

        CloseSession();

    }

    return invoices;

}

I don't have a complete abstraction of the separate execution, yet. However, a very basic implementation could look like this (idea stolen from Ray Houston):

protected void Do(Action unitOfWork)

{

    try

    {

        OpenSession();

        unitOfWork();

    }

    finally

    {

        CloseSession();

    }

}

public ICollection<Invoice> GetAll()

{

    ICollection<Invoice> invoices = null;

    DetachedCriteria criteria = DetachedCriteria.For<Invoice>();

    Do(() =>{

        invoices = criteria.GetExecutableCriteria(Session).List<Invoice>();

    });

    return invoices;

}

This simple abstraction provides a lot of benefit for us.

• Eliminates duplicate code (not calling OpenSession / Close Session from all repository methods)
• Removes ugly Try / Catch blocks from Repository methods
• Allows multiple Criteria to be executed from a single Session / Transaction

And most importantly - this gives us the ability to create a better abstraction of NHibernate, to support business level transactions, not just repository level transactions. Obviously, this simple example is not going to provide business level transactions. It does get us down the path, though.

A coworker and I ran into a problem yesterday - we were trying to re-use an assembly from a WinForms app, in a WebService and ran into this code:

Directory.SetCurrentDirectory(Application.StartupPath);

The problem with this, is that it uses the Application static class, which is part of System.Windows.Forms - and we're in a web service now, not a WinForms app. So, after some headache and thought, we tried to use this:

Assembly.GetExecutingAssembly().Location

That doesn't work well, either, because in the web, it gives you the ShadowCopy location of the assembly, not the original location of the assemblies. A little more thought, and a few hours later, we finally came up with this:

private static string GetBinFolder()

{

    AppDomain appDomain = AppDomain.CurrentDomain;

    string binFolder;

    if (appDomain.RelativeSearchPath != null && appDomain.RelativeSearchPath != string.Empty )

        binFolder = Path.Combine(appDomain.BaseDirectory, appDomain.RelativeSearchPath);

    else

        binFolder = appDomain.BaseDirectory;

    return binFolder;

}

The AppDomain.BaseDirectory will give you the root folder that the application is being run from - no matter what type of app you are in; windows or web. This is perfect for Windows because it alone gives us the folder that the code is running from and lets us find the assembly we need. The RelativeSearchPath is important for the web - it gives us the "bin" folder where our assemblies live. So a simple check to see if there is a relative search path (it returns null in a standard WinForms app) and combine the two if there are, otherwise just get the base directory, and we now have our folder that the assemblies are located in, so we can call:

Directory.SetCurrentDirectory(GetBinFolder());

...

Of course, this problem could have been avoided if there was proper Inversion of Control in the code... don't have time to introduce it right now, but at least we removed some code duplication by creating a single GetBinFolder() method.

As a kid, I was never part of the boy-scouts or anything; but my family and I went camping a lot, and I went camping with my youth group on several occasions. I remember hearing my parents and the various youth leaders talking about we should always leave the camp site cleaner than we found it. I always thought this was annoying - why should I clean up someone else's mess? If I clean up my own mess, isn't that good enough?

Yesterday, while helping a coworker fix some bugs in an application that I wrote around a year ago, I was suggesting ways to improve various parts of the code; move this property to a parameter of that method, make this method private and only call it from here in the owning class, and items as simple as making an if-then statement easier to read.

After a few of these suggestions, he asked me if I always clean up the code that I'm working with, even if the bug is not directly related to the code that we are cleaning up. My answer was emphatically, "yes". If I'm reading code, trying to find a bug and I'm having a hard time understand what's going on with the code, then it becomes much more difficult to find the actual bug. Even if this code does not end up being part of the bug I was looking for, by cleaning up the code I am making it more likely that I will be able to understand what this code is doing the next time I have to look at it.

Here's my basic perspective that drives all of this: if you have a hard time reading the code and seeing what it is doing, chances are, you or someone you know will have to debug that code at some point. I don't want to debug hard to read code - that's annoying, at best. I want to debug code that is easy to read and easy to understand. And I certainly don't want to make any of my coworkers debug hard to read code. I try not to torture coworkers like that. So, if my motivation is to not debug hard to read code, then doesn't it make sense that I would want to clean up that ugly code? It makes sense to me...

What does this really come down to, then? Two things:

1. Leave the code cleaner than when you arrived, by
2. Micro-refactoring - make that one line of code easier to read

(By the way - there's no such thing as "micro-refactoring". Refactoring, by definition, is exactly what I described above. Stop trying to change the architecture and learn to change that one line of ugly code. By doing this, you'll find that the architecture does change, because you clean up more than you realize and change become natural.)