Rich Domain Model Implementation

Question

I recently started reading about rich domain model instead of anemic models. All the projects I worked on before, we followed service pattern. In my new new project I'm trying to implement rich domain model. One of the issues I'm running into is trying to decide where the behavior goes in (in which class). Consider this example -

public class Order
{

   int OrderID;
   string OrderName;

   List<Items> OrderItems;
}

public class Item
{
   int OrderID;
   int ItemID;
   string ItemName;

}

So in this example, I have the AddItem method in Item class. Before I add an Item to an order, I need to make sure a valid order id is passed in. So I do that validation in AddItem method. Am I on the right track with this? Or do I need create validation in Order class that tells if the OrderID is valid?

Answer 1

Wouldn't the Order have the AddItem method? An Item is added to the Order, not the other way around.

public class Order
{

   int OrderID;
   string OrderName;
   List<Items> OrderItems;
   bool AddItem(Item item)
   {
     //add item to the list
   }
}

In which case, the Order is valid, because it has been created. Of course, the Order doesn't know the Item is valid, so there persists a potential validation issue. So validation could be added in the AddItem method.

public class Order
{

   int OrderID;
   string OrderName;
   List<Items> OrderItems;
   public bool AddItem(Item item)
   {
     //if valid
     if(IsValid(item))
     {
         //add item to the list
     }

   }

  public bool IsValid(Item item)
  {
     //validate
  }

}

All of this is in line with the original OOP concept of keeping the data and its behaviors together in a class. However, how is the validation performed? Does it have to make a database call? Check for inventory levels or other things outside the boundary of the class? If so, pretty soon the Order class is bloated with extra code not related to the order, but to check the validity of the Item, call external resources, etc. This is not exactly OOPy, and definitely not SOLID.

In the end, it depends. Are the behaviors' needs contained within the class? How complex are the behaviors? Can they be used elsewhere? Are they only needed in a limited part of the object's life-cycle? Can they be tested? In some cases it makes more sense to extract the behaviors into classes that are more focused.

So, build out the richer classes, make them work and write the appropriate tests Then see how they look and smell and decide if they meet your objectives, can be extended and maintained, or if they need to be refactored.

Answer 2

First of all, every item is responsible of it's own state (information). In good OOP design the object can never be set in an invalid state. You should at least try to prevent it.

In order to do that you cannot have public setters if one or more fields are required in combination.

In your example an Item is invalid if its missing the orderId or the itemId. Without that information the order cannot be completed.

Thus you should implement that class like this:

public class Item
{
   public Item(int orderId, int itemId)
   {
       if (orderId <= 0) throw new ArgumentException("Order is required");
       if (itemId <= 0) throw new ArgumentException("ItemId is required");

      OrderId = orderId;
      ItemId = itemId;
   }

   public int OrderID { get; private set; }
   public int ItemID { get; private set; }
   public string ItemName { get; set; }
}

See what I did there? I ensured that the item is in a valid state from the beginning by forcing and validating the information directly in the constructor.

The ItemName is just a bonus, it's not required for you to be able to process an order.

If the property setters are public, it's easy to forget to specify both the required fields, thus getting one or more bugs later when that information is processed. By forcing it to be included and also validating the information you catch bugs much earlier.

Order

The order object must ensure that it's entire structure is valid. Thus it need to have control over the information that it carries, which also include the order items.

if you have something like this:

public class Order
{
   int OrderID;
   string OrderName;
   List<Items> OrderItems;
}

You are basically saying: I have order items, but I do not really care how many or what they contain. That is an invite to bugs later on in the development process.

Even if you say something like this:

public class Order
{
   int OrderID;
   string OrderName;
   List<Items> OrderItems;

   public void AddItem(item);
   public void ValidateItem(item);
}

You are communicating something like: Please be nice, validate the item first and then add it through the Add method. However, if you have order with id 1 someone could still do order.AddItem(new Item{OrderId = 2, ItemId=1}) or order.Items.Add(new Item{OrderId = 2, ItemId=1}), thus making the order contain invalid information.

imho a ValidateItem method doesn't belong in Order but in Item as it is its own responsibility to be in a valid state.

A better design would be:

public class Order
{
   private List<Item> _items = new List<Item>();

   public Order(int orderId)
   {
       if (orderId <= 0) throw new ArgumentException("OrderId must be specified");
       OrderId = orderId;
   }

   public int OrderId { get; private set; }
   public string OrderName  { get; set; }
   public IReadOnlyList<Items> OrderItems { get { return _items; } }

   public void Add(Item item)
   {
       if (item == null) throw new ArgumentNullException("item");

       //make sure that the item is for us
       if (item.OrderId != OrderId) throw new InvalidOperationException("Item belongs to another order");

       _items.Add(item);
   }
}

Now you have gotten control over the entire order, if changes should be made to the item list, it has to be done directly in the order object.

However, an item can still be modified without the order knowing it. Someone could for instance to order.Items.First(x=>x.Id=3).ApplyDiscount(10.0); which would be fatal if the order had a cached Total field.

However, good design is not always doing it 100% properly, but a tradeoff between code that we can work with and code that does everything right according to principles and patterns.