Entities and the Law of Demeter

The Law of Demeter, and its corresponding code smell, Inappropriate Intimacy, are some of the best bang-for-your-buck code smells that you can address.  The basic idea behind each of these concepts is code related to an object should probably be inside that object.  It’s also known as the Principle of Least Knowledge, and I’ve found it very helpful in creating objects with rich behavior.

It’s not always something I catch immediately, but I tend to notice later.  For example, I noticed this code in one of my code behinds:

protected decimal GetTotal()
{
    return _cart
             .GetItems()
             .Sum(item => item.Price * item.Quantity);
}

This method was then called by markup in the template file to display the current total in the cart.

Now, it seems rather obvious that something like the cart’s total should belong on the ShoppingCart object itself.  In some situations, there might not even be a ShoppingCart class, only a generic list of ShoppingCartItems.  In that case, the best thing to do would be to create the ShoppingCart class, if nothing else than to become a magnet for cart behavior.

With the Move Method refactoring, I moved this behavior to the ShoppingCart class:

public class ShoppingCart
{
    private readonly List<ShoppingCartItem> _items;

    public ShoppingCart()
    {
        _items = new List<ShoppingCartItem>();
    }

    public ShoppingCartItem[] GetItems()
    {
        return _items.ToArray();
    }

    public void AddItem(Product product, int quantity)
    {
        var item = new ShoppingCartItem(product);
        item.Quantity = quantity;
        _items.Add(item);
    }

    public decimal GetTotal()
    {
        return _items.Sum(item => item.Price * item.Quantity);
    }

}

Again, it seems like more total behavior is in the wrong place.  Why should ShoppingCart have to perform the price/quantity calculation?  Let’s move this to the ShoppingCartItem class:

public class ShoppingCartItem
{
    private readonly Product _product;

    public ShoppingCartItem(Product product)
    {
        _product = product;
    }

    public int Quantity { get; set; }

    public decimal Price
    {
        get { return _product.Price; }
    }

    public Product Product
    {
        get { return _product; }
    }

    public decimal GetItemTotal()
    {
        return Quantity * Price;
    }
}

I also noticed several other places that cared way too much about the Product class.  Here’s an example I found in the code-behind:

protected decimal CalculateMargin(ShoppingCartItem item)
{
    return item.Product.Price / item.Product.Cost;
}

Again, this is probably obvious to a lot of people (except me).  This many dots poking into the ShoppingCartItem means the code behind class cares much too deeply about the inner workings of the ShoppingCartItem.  Something like a margin seems like a fundamental concept of a Product, so let’s just move it there:

public class Product
{
    public string Name { get; set; }
    public decimal Price { get; set; }
    public decimal Cost { get; set; }

    public decimal CalculateMargin()
    {
        return Cost == 0 ? 0m : Price / Cost;
    }
}

I would also create a delegating method on ShoppingCartItem, so callers wouldn’t need to poke around the insides of Product to find what they needed.  I find it best to expose exactly what’s needed on the container class, creating an explicit contract with the callers and telling them exactly what they should care about.

The behavior is out there

In many legacy code applications, I find lots of examples of anemic domain models, where the “business objects” have zero behavior and lots of properties.  Essentially, they just become containers of data.  One step above DataSets, but that’s it.

Lots of duplication exists around these business objects, where I find the same methods and snippets poking around the business objects, getting the information they need.  These helper methods often have nothing to do with the class they’re in, as we saw with the CalculateMargin method.  They do something related to some other class, but for whatever reason, don’t exist on that class.

These data container classes don’t have any behavior on them, but their behavior is out there somewhere, scattered among the rest of the application.  Moving behavior concerning a type actually to that type creates very cohesive domain models.  Centralizing data and behavior into a single type creates powerful domain models and a much richer client experience.

Related Articles:

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

About Jimmy Bogard

I'm a technical architect with Headspring in Austin, TX. I focus on DDD, distributed systems, and any other acronym-centric design/architecture/methodology. I created AutoMapper and am a co-author of the ASP.NET MVC in Action books.
This entry was posted in Domain-Driven Design, Refactoring. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • Peter

    Is this part of the MVC Storefront Challenge? Did you find a source repository for this yet?

  • Miika

    Just out of interest: What type of “Margin” is “Price / Cost”? I know not the point here, but it just jumped on me :)

  • http://jimmybogard.lostechies.com Jimmy Bogard

    @Peter

    Not yet, still working on the details of branching the existing storefront.

    @Miika

    Yes, it’s a Bizarro margin, where left is right and up is down :)

  • Dimitris Menounos

    The behavior is out there… but for a good reason.

    Data classes are easy to serialize – Service classes are easy to remote.

    Entities, in the anemic model, represent application data types. They essentially are, as you also said, rich data containers. They may apply the all of OO characteristics, encapsulation, inheritance and polymorphism. They may also have behavior but only such that operates on the entities themselves and is not dependent on any non entity code.

    The important thing though is that such entities are vertically reusable across different layers, environments and processes. Because they are mainly data holders they can be easily serialized and converted into various forms (from database records -> to objects instances -> to xml -> to json -> to …).

    Services on the other hand are more appropriate for remoting. Because they live in a fixed place, they may depend on other code (for example a persistence library). They are the ones that do the “dirty work” so that the entities shall remain “pure”.

  • http://www.hanselman.com/blog Scott Hanselman

    Great pragmatic example of the Law of Demeter…Thanks!

  • http://jimmybogard.lostechies.com Jimmy Bogard

    @Dimitris

    I use different types to cross application boundaries. These types are based on messages and operations, not necessarily entities. Something along the lines of CreateQuoteRequest or an UpdateCustomerDetailsRequest. As a rule, entity types don’t cross service boundaries, while their identity might.

  • http://www.bluespire.com/blogs Christopher Bennage

    I know that I’m late with it, but excellent post.

  • Dave B

    That works great until you have to serialise the object across the wire. How do you propose we handle this situation with DTOs consumed from a service layer?

  • http://jimmybogard.lostechies.com Jimmy Bogard

    @Dave B

    Different behavior is required at different points. I expose operations, and people are only interested with the results, not a remoting-style always connected smart object. They want, “Give me the price list for this customer”. Those are the capabilities I’m interested in exposing.

    In short, Total is passed along, precalculated in the above example. If someone’s wanting to do something more granular, I have to question what operations I’m exposing.