Strengthening your domain: Encapsulated collections

Previous posts in this series:

One of the common themes throughout the DDD book is that much of the nuts and bolts of structural domain-driven design is just plain good use of object-oriented programming.  This is certainly true, but DDD adds some direction to OOP, along with roles, stereotypes and patterns.  Much of the direction for building entities at the class level can, and should, come from test-driven development.  TDD is a great tool for building OO systems, as we incrementally build our design with only the behavior that is needed to pass the test.  Our big challenge then is to write good tests.

To fully harness TDD, we need to be highly attuned to the design that comes out of our tests.  For example, suppose we have our traditional Customer and Order objects.  In our world, an Order has a Customer, and a Customer can have many Orders.  We have this directionality because we can navigate this relationship from both directions in our application.  In the last post, we worked to satisfy invariants to prevent an unsupported and nonsensical state for our objects.

We can start with a fairly simple test:

[Test]
public void Should_add_the_order_to_the_customers_order_lists_when_an_order_is_created()
{
    var customer = new Customer();
    var order = new Order(customer);

    customer.Orders.ShouldContain(order);
}

At first, this test does not compile, as Customer does not yet contain an Orders member.  To make this test compile (and subsequently fail), we add an Orders list to Customer:

public class Customer
{
    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Province { get; set; }
    public List<Order> Orders { get; set; }

    public string GetFullName()
    {
        return LastName + ", " + FirstName;
    }
}

With the Orders now exposed on Customer, we can make our test pass from the Order constructor:

public class Order
{
    public Order(Customer customer)
    {
        Customer = customer;
        customer.Orders.Add(this);
    }

And all is well in our development world, right?  Not quite.  This design exposes quite a bit of functionality that I don’t think our domain experts need, or want.  The design above allows some very interesting and very wrong scenarios:

[Test]
public void Not_supported_situations()
{
    // Removing orders?
    var customer1 = new Customer();
    var order1 = new Order(customer1);

    customer1.Orders.Remove(order1);

    // Clearing orders?
    var customer2 = new Customer();
    var order2 = new Order(customer1);

    customer2.Orders.Clear();

    // Duplicate orders?
    var customer3 = new Customer();
    var customer4 = new Customer();
    var order3 = new Order(customer3);

    customer4.Orders.Add(order3);
}

With the API I just created, I allow a number of rather bizarre scenarios, most of which make absolutely no sense to the domain experts:

  • Clearing orders
  • Removing orders
  • Adding an order from one customer to another
  • Inserting orders
  • Re-arranging orders
  • Adding an order without the Order’s Customer property being correct

This is where we have to be a little more judicious in the API we expose for our system.  All of these scenarios are possible in the API we created, but now we have some confusion on whether we should support these scenarios or not.  If I’m working in a similar area of the system, and I see that I can do a Customer.Orders.Remove operation, it’s not immediately clear that this is a scenario not actually coded for.  Worse, I don’t have the ability to correctly handle these situations if the collection is exposed directly.

Suppose I want to clear a Customer’s Orders.  It logically follows that each Order’s Customer property would be null at that point.  But I can’t hook in easily to the List<T> methods to handle these operations.  Instead of exposing the collection directly, I will expose only those operations which I support through my domain.

Moving towards intention-revealing interfaces

Let’s fix the Customer object first.  It exposes a List<T> directly, and allows wholesale replacement of that collection.  This is the complete antithesis of intention-revealing interfaces.  I will now only expose the sequence of Orders on Customer:

public class Customer
{
    private readonly IList<Order> _orders = new List<Order>();

    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Province { get; set; }
    public IEnumerable<Order> Orders { get { return _orders; } }

    public string GetFullName()
    {
        return LastName + ", " + FirstName;
    }
}

This interface explicitly tells users of Customer two things:

  • Orders are readonly, and cannot be modified through this aggregate
  • Adding orders are done somewhere else

I now have the issue of the Order constructor needing to add itself to the Customer’s Order collection.  I want to do this:

public class Order
{
    public Order(Customer customer)
    {
        Customer = customer;
        customer.AddOrder(this);
    }

Instead of exposing the Orders collection directly, I work through a specific method to add an order.  But, I don’t want that AddOrder available everywhere, I want to only support the enforcement of the Order-Customer relationship through this explicitly defined interface.  I’ll do this by exposing an AddOrder method, but exposing it as internal:

public class Customer
{
    private readonly IList<Order> _orders = new List<Order>();

    public string FirstName { get; set; }
    public string LastName { get; set; }
    public string Province { get; set; }
    public IEnumerable<Order> Orders { get { return _orders; } }

    internal void AddOrder(Order order)
    {
        _orders.Add(order);
    }

There are many different ways I could enforce this relationship, from exposing an AddOrder method publicly on Customer or through the approach above.  But either way, I’m moving towards an intention-revealing interface, and only exposing the operations I intend to support through my application.  Additionally, I’m ensuring that all invariants of my aggregates are satisfied at the completion of the Create Order operation.  When I create an Order, the domain model takes care of the relationship between Customer and Order without any additional manipulation.

If I publicly expose a collection class, I’m opening the doors for confusion and future bugs as I’ve now allowed my system to tinker with the implementation details of the relationship.  It’s my belief that the API of my domain model should explicitly support the operations needed to fulfill the needs of the application and interaction of the UI, but nothing more.

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. Bookmark the permalink. Follow any comments here with the RSS feed for this post.
  • Henning Anderssen

    Good post.

    However, I do see a potential problem with your implementation.
    Ignoring the fact that you’re allowing your Domain model to be using in queries and reporting (ref, CQRS), exposing the Orders collection as an IEnumerable can also lead you into trouble.

    If an unknowing or new team member casts Orders to IList they can still add to the collection in the wrong way. I guess a better way would be to return a ReadOnlyCollection instead.
    However, as I experienced in my current project, returning a readonly collection has its own set of issues, especially if you’re lazyloading the collection.

    When new’ing up a ReadOnlyCollection it actually iterates over the list and thereby lazyloading the entire collection. In our case that caused our system to hang, and it took me a few hours to find the cause of the problem.

    I guess one could argue that you can’t protect against every dumb thing every developer could think of, the fact of the matter is that if it can be done it will be done, either through ignorance or just plain of lazyness

  • Paco

    @Henning Anderssen: I would tell the new teammember that upcasting is almost never a good thing. A .net readonly collection violates lsp, so I don’t understand where it’s useful for.

  • http://stefanmoser.blogspot.com Stefan Moser

    Great article! Exposing Lists and Dictionaries in the domain is a battle I’ve been fighting for years. Like you, I usually end up going with an IEnumerable, but I’ve started using specific collection classes more and more recently. So exposing an OrdersCollection which contains the List, taking advantage of Containment over Inheritance.

  • Travis

    Excellent article! Does anyone see a problem with Order[] GetOrders() {return _orders.ToArray();}

  • http://www.adverseconditionals.com Harry M

    or even return _orders.Select(o => o);

  • http://www.lostechies.com/members/bogardj/default.aspx bogardj

    @Travis @Harry

    Yeah, either one works. Array has started to annoy me because its methods are conflicting now with ones I create, and that its contents are mutable. Otherwise, I don’t see the harm of exposing the list directly through IEnumerable.

  • http://shane.jscconsulting.ca Shane Courtrille

    The only problem is that some “crafty” member of your team can still go

    var orders = (IList) customer.Orders;

    Which leads to using yield return inside of your get {}. Of course that’s a lot uglier so really your method & a hard foam bat to deal with any “crafty” people is preferable.

  • http://simon-says-architecture.com/ Szymon Pobiega

    Guys, if you are afraid that someone can use your read-only collections as writable ones, simply don’t expose them. Build a parallel read-only model on top of the same database and use it for queries and displaying detailed info. You’ll free your domain from UI-related stuff (in majority of cases read-only collections are user exclusively for UI purposes)

  • Edin

    I like this approach, but there can be a problem when using ORM. In that case you would need to do mapping to private fields. Sure, it is possible, but it’s not elegant, at least not with Fluent NHibernate.

  • http://www.lostechies.com/members/bogardj/default.aspx bogardj

    @Edin

    This approach is *super* easy with fluent nhibernate. In fact, I have a convention to do so:

    public class CollectionAccessConvention : ICollectionConvention
    {
    public void Apply(ICollectionInstance instance)
    {
    instance.Access.CamelCaseField(CamelCasePrefix.Underscore);
    }
    }

    Done. Don’t need to do anything different in your collection mappings themselves, just use the expression as normal.

  • Markus Zywitza

    I’d rather object against the internal AddOrder() method than agaionst IEnumerable. IEnumerable is a clear hands-off sign for modifying.
    But rather than have internal AddOrder, I would add
    Order CreateOrder() {…}
    to Customer. I know that this is slightly less testable, because I cannot stub the Customer for testing, but it is also cleaner. I remember a post from Udi Dahan that emphasizes this method of creating objects in a domain model.

  • Edin

    @Jimmy

    I wasn’t aware of this approach. Thanks a lot!

  • Craig

    This might not be the place to ask this but I am wondering how you would configure AutoMapper to map the readonly IEnumerable to a DTO’s readonly IEnumerable? Or should the DTO just contain an IList or array?

  • http://www.lostechies.com/members/bogardj/default.aspx bogardj

    @Craig

    Typically, we make the DTOs contain all writable members, including lists and collections. This is also what you’d need to do with serializable types, for example.

  • http://shane.jscconsulting.ca Shane Courtrille

    Agreed with Markus. Udi had a good post on the idea that almost everything needs to come from something existing.

  • Ryan Hartzog

    I can see how the Customer Order relationship would not want to expose a RemoveOrder() method, but what about Order > OrderLine? In that instance the Order would publicly expose OrderLines() and RemoveOrderLine(OrderLine line) while keeping the AddOrderLine(OrderLine line) behavior internal?

  • http://www.lostechies.com/members/bogardj/default.aspx bogardj

    @Ryan

    If there’s a screen to allow removing an order line, sure! The important thing here is to not expose the entire collection directly, but only the operations needed by the application.

  • RhysC

    I dont want to harp on Jimmy, but why would you not just return an actual IEnum ?

    public IEnumerable Orders {
    get
    {
    foreach(var o in _orders)
    yield return o;
    }
    }

    Its a trival amount of additioonal code and gives you what you want. throw an extenion method to just make it return _orders.AsIEnumerable() and you are laughing :)

  • http://www.lostechies.com/members/bogardj/default.aspx bogardj

    @Rhys

    Perhaps you can answer, why _would_ I do this? What does this extra code tax give me? Keep in mind I don’t care about anyone casting to IList or something else like that.

  • Josh Kodroff

    This article really helped me pimp up my domain model. One thing, though…

    I don’t get how you can have “private readonly IList _orders = new List();” and still do lazy loading (at least without adding extra variables to track whether you’ve tried to load the collection). What’s the solution for that?