3

According to DDD principles, external objects should only call methods on an aggregate root, not on other entities in the aggregate, right ?

In case of nested entities, for example: SeatingPlan -> Sections -> Rows -> Seats

SeatingPlan is the aggregate root, while sections, rows and seats are entities that are meaningless outside its parent entity.


Lets say I want to add seats in the seating plan.

I would create SeatingPlan.AddSeat(sectionId, rowId, seatNo) in order to prevent external objects to call SeatingPlan.Sections[x].Rows[y].Seat[s].Add, which is bad, right ?

But still, the AddSeat method of SeatingPlan must delegate the seat creation to the row object, because the seat is a composite of the row, the row owns the seats. So it has to call Sections[x].Rows[y].AddSeat(seatNo).


Now my question is how can I prevent external objects from calling Row.AddSeat method, while allowing the aggregate root to call it ?

internal visibility is too large, even namespace visibility (assuming it would even exists in c#) would be too large. I need an aggregate visibility.

I thought about nesting the class Row in the SeatingPlan class, and making the Row.AddSeat method private. But is it a good practice ? Because the class would have to be public and I remember having read something about it saying that we should avoid public nested classes.

Jonathan
  • 1,276
  • 10
  • 35
  • Two interfaces, a read-only interface and a full interface. Expose the readonly interface to users, use the full interface internally. – spender Nov 21 '18 at 14:10
  • If you are developing this as an assembly then there is an ‘internal’ keyword – Markiian Benovskyi Nov 21 '18 at 17:31
  • 1
    Why do you need to expose entities to the outside in the first place? For queries? Don't use the domain model for queries and this problem most likely goes away ;) – plalx Nov 21 '18 at 17:39

4 Answers4

3

Conflicting domain model role: commands vs queries

I'm suspecting that the reason you allowed external deep navigation of your aggregate root is for querying needs and that's what's causing issues. If you could avoid exposing entities outside the root then this problem goes away. You must not forget that the primary role of the domain model is to process commands while protecting invariants; not fulfilling query needs.

A single model can't be optimized for both, commands & queries. When the model is starting to fail you in one of the two roles it may be time to segregate them. That's called Command Query Responsibility Segregation (CQRS). You'd by-pass the domain model entirely for queries and go straight to the DB, after which you could get rid of most state-exposing members in your aggregate roots.

CQRS scares me...

If you dont want to go that route then you will have to live with the pains of having a single model stretched in different directions. One thing you could do to mitigate the problem you described is to use readonly interfaces, such as IRowView which do not expose any mutating methods. Alternatively, you could also return value objects describing the state of the sub-entity, such as RowDescriptor or RowState. What you will start to realize however is that you will start being forced to invent new concepts that dont exist in your Ubiquitous Language, just to solve technical problems (e.g. preserve encapsulation).

Beware large aggregate roots

The Stadium AR seems to be very large as a consistency boundary. That's usually a good indicator that the boundaries might be wrong. You shouldn't attempt to model the real world: just because a stadium contains sections which have rows, etc., in the real world doesn't mean your model should be composed that way.

Also, dont rely on the rule "A can't exist or doesn't make sense without B" to model aggregates. It's does more harm than good most of the time.

Since that's not the core of the question I'll just leave you to read this excellent Vaughn Vernon article, Effective Aggregate Design.

plalx
  • 42,889
  • 6
  • 74
  • 90
  • I like the first read of your answer, i already heard of CQRS, i can't say i like it either but i will look deeper at it. There's a thing you said that made me frown a little: "You shouldn't attempt to model the real world"... Isn't DDD about "modeling the real world (from the domain point of view at least)" ?? – Jonathan Nov 22 '18 at 22:14
  • @Jonathan Why don't you like CQRS? You don't need a secondary datastore nor you have to use event sourcing for CQRS. It can be as simple as querying the written state of your AR from the DB directly. It gives much more flexibility and allows you to keep your ARs simple and free of unnecessary state-exposing methods and relationships. – plalx Nov 23 '18 at 14:07
  • I thought of CQRS more for queries that would span data of multiple aggregates into some report/list rather than only for the purpose of querying and display one aggregte's state. Also in my example I changed some names but maybe I should'nt because indeed "Statium" would be too big for a single aggregate. A better name would have been a SeatingPlan – Jonathan Nov 23 '18 at 14:10
  • @Jonathan Also, when I say don't model the real world, I'm mainly saying: don't get fooled into composing objects together just because that's how they physically are in the real world or you will end up with very large impractical ARs that will cause both, contention issues and performance issues. It's also not because something can't exist without something else that they should be part of the same AR. For instance, a `Post` can most likely not exist without a `Thread`, but that doesn't mean the `Thread` has a collection of `Post`. – plalx Nov 23 '18 at 14:14
  • 1
    For every use case, identify which data needs to be changed and which data needs to be checked in order to validate invariants. That should set your initial AR boundaries. However, you may still end up with impractical ARs due to concurrency. At that point you have to challenge the rules and see if you can further break the AR and make some of it's parts eventually consistent. – plalx Nov 23 '18 at 14:16
  • I understand, but when actions on an entity in the domain makes no sense without involving a root entity, that is an aggregate entity – Jonathan Nov 23 '18 at 14:16
  • @Jonathan Yes, that is right, but I just want to emphasis that a simple identity involvement may not be enough to justify putting them both in the same boundary (e.g. none of the other entity state is used but it's identity). `"I thought of CQRS more for queries that would span data of multiple aggregates into some report/list"`: CQRS certainly helps here, but it can also be helpful in scenarios involving a single AR. Like I explained above, using CQRS also solves this very problem, which only involves 1 AR. – plalx Nov 23 '18 at 14:22
  • It's up to you to choose the right strategy given the downsides of each solution. If you are using CQRS, then the downside is that you have to write extra data fetching logic, most likely in pure SQL, but the upside is that your ARs are purely focused on maintaining the state they need and exposing the behaviors they need to process commands, which means they become much easier to maintain, simpler to unit (smaller interface) and encapsulation is preserved. – plalx Nov 23 '18 at 14:38
  • On the other hand, if you choose to use interfaces, then the data fetching problem is pretty much solved already, but you have to pollute the domain with concepts that are purely technical and which aims at preserving encapsulation. These interfaces will have to be very carefully crafted not to allow any form of mutation and purists might say all form of getters, etc. may have to be unit tested as well, which adds a lot of overhead. As the system evolves a query that needed a single AR may then suddenly need 2 or more as well which could force you towards CQRS anyway. – plalx Nov 23 '18 at 14:44
  • I accepted Yair's answer because it is a short and clean solution to my question. However, I also like a lot your answer that makes me rethink my design. I would really like to discuss a little further about it, can we continue this in a better place ? – Jonathan Nov 27 '18 at 13:18
  • @Jonathan Sure, I guess you can create a chat room or something like that in here? – plalx Nov 27 '18 at 14:16
  • alright, let's continue this conversation: https://chat.stackoverflow.com/rooms/184523/ddd-aggregates – Jonathan Nov 30 '18 at 14:41
0

Firstly I would point out that DDD is a set of guidelines, not rules. Do whatever makes sense in your situation, don't just follow DDD blindly.

That said you can use interfaces/base classes to do what you want. Here is a simple example.

public interface IRow
{
    IReadOnlyList<Seat> Seats {get;}
}

public class Stadium
{
    private List<Row> _rows = new List<Row>();
    public IReadOnlyList<IRow> Rows => _rows;
    public void AddSeat(Seat seat, int rowNum) => _rows[rowNum].AddSeat(seat);
    private class Row : IRow
    {
         private List<Seat> _seats = new List<Seat>();
         public IReadOnlyList<Seat> Seats => _seats;
         public void AddSeat(Seat seat) => _seats.Add(seat);
    }
}
Yair Halberstadt
  • 5,733
  • 28
  • 60
0

In java, I protect the access to internal objects of an aggregate by the scope of those objects. I structure the code so that each aggregate is in a package , named with aggregate name. Internal entities and value objects would be package scoped (no visibility scope keywords when defining them), so that those objects are just accessible from the same package. The aggregate root entity would be public.

choquero70
  • 4,470
  • 2
  • 28
  • 48
0

According to DDD principles, external objects should only call methods on an aggregate root (AR), not on other entities in the aggregate

Id rather say that an aggregate root is a consistency boundary. That's why "external objects should only call methods on the aggregate root".

On the other hand your value objects (VOs) or entities can be quite rich and encapsulate a lot of their internal rules.

E.g SeatNumber cannot be negative, Seat can have a method Book(Person person) that makes sure that it's booked by only one person, Row can have methods BookASeat(SeatNumber seatId, Person person) and AddASeat(Seat seat), ...

public class Seat : Entity
{
    private Person _person;
    public Seat(SeatNumber id)
    {
        SeatId = id;
    }
    public SeatNumber SeatId { get; }

    public void Book(Person person)
    {
        if(_person == person) return;
        if (_person != null)
        {
            throw new InvalidOperationException($"Seat {SeatId} cannot be booked by {person}. {_person} already booked it.");
        }

        _person = person;
    }

    public bool IsBooked => _person != null;
}

I would create SeatingPlan.AddSeat(sectionId, rowId, seatNo) in order to prevent external objects to call SeatingPlan.Sections[x].Rows[y].Seat[s].Add, which is bad, right?

But still, the AddSeat method of SeatingPlan must delegate the seat creation to the row object, because the seat is a composite of the row, the row owns the seats. So it has to call Sections[x].Rows[y].AddSeat(seatNo).

It's not bad to call Sections[sectionNumber].Rows[rowNo].Seat[seat.SeatNo].Add(seat) as long as Sections is a private collection (dictionary) and SeatingPlan doesn't expose it to an outside world.

IMHO: A disadvantage of this approach is the following - all domain rules are maintained by your aggregate root. It cam make you aggregate root too complex too understand or maintain.

In order to keep your aggregate simple I'd recommend to split into multiple entities and make each of them responsible for enforcing their own domain rules:

  • Row is responsible for maintaining an internal list of its seats, has methods AddASeat(Seat seat) and BookASeat(SeatNumber seatId, Person person)
  • Section is responsible for maintaining an internal list of rows, knows how to add an entire valid row (AddARow(Row row)) or just to add a seat to an existing row (AddASeat(RowNumber rowId, Seat seat))
  • Stadium (or a seat plan) can have methods like AddASection(Section section), AddARow(Row row, SectionCode sectionCode), AddASeat(Seat seat, RowNumber rowNumber, SectionCode sectionCode). It all depends on the interface that you provide to your users.

You can describe your aggregate root without exposing internal collections:

/// <summary>
/// Stadium -> Sections -> Rows -> Seats
/// </summary>
public class Stadium : AggregateRoot
{
    private readonly IDictionary<SectionCode, Section> _sections;
    public static Stadium Create(StadiumCode id, Section[] sections)
    {
        return new Stadium(id, sections);
    }
    public override string Id { get; }
    private Stadium(StadiumCode id, Section[] sections)
    {
        _sections = sections.ToDictionary(s => s.SectionId);
        Id = id.ToString();
    }

    public void BookASeat(SeatNumber seat, RowNumber row, SectionCode section, Person person)
    {
        if (!_sections.ContainsKey(section))
        {
            throw new InvalidOperationException($"There is no Section {section} on a stadium {Id}.");
        }
        _sections[section].BookASeat(row, seat, person);
    }

    public void AddASeat(Seat seat, RowNumber rowNumber, SectionCode sectionCode)
    {
        _sections.TryGetValue(sectionCode, out var section);
        if (section != null)
        {
            section.AddASeat(rowNumber, seat);
        }
        else
        {
            throw new InvalidOperationException();
        }
    }

    public void AddARow(Row row, SectionCode sectionCode)
    {
        _sections.TryGetValue(sectionCode, out var section);
        if (section != null)
        {
            section.AddARow(row);
        }
        else
        {
            throw new InvalidOperationException();
        }
    }

    public void AddASection(Section section)
    {
        if (_sections.ContainsKey(section.SectionId))
        {
            throw new InvalidOperationException();
        }
        _sections.Add(section.SectionId, section);
    }
}

public abstract class AggregateRoot
{
    public abstract string Id { get; }
}

public class Entity { }
public class ValueObject { }
public class SeatNumber : ValueObject { }
public class RowNumber : ValueObject { }
public class SectionCode : ValueObject { }
public class Person : ValueObject { }
public class StadiumCode : ValueObject { }

public class Row : Entity
{
    private readonly IDictionary<SeatNumber, Seat> _seats;

    public Row(RowNumber rowId, Seat[] seats)
    {
        RowId = rowId;
        _seats = seats.ToDictionary(s => s.SeatId);
    }
    public RowNumber RowId { get; }

    public void BookASeat(SeatNumber seatId, Person person)
    {
        if (!_seats.ContainsKey(seatId))
        {
            throw new InvalidOperationException($"There is no Seat {seatId} in row {RowId}.");
        }
        _seats[seatId].Book(person);
    }
    public bool IsBooked(SeatNumber seatId) { throw new NotImplementedException(); }
    public void AddASeat(Seat seat)
    {
        if (_seats.ContainsKey(seat.SeatId))
        {
            throw new InvalidOperationException();
        }
        _seats.Add(seat.SeatId, seat);
    }
}

public class Section : Entity
{
    private readonly IDictionary<RowNumber, Row> _rows;

    public Section(SectionCode sectionId, Row[] rows)
    {
        SectionId = sectionId;
        _rows = rows.ToDictionary(r => r.RowId);
    }
    public SectionCode SectionId { get; }

    public void BookASeat(RowNumber rowId, SeatNumber seatId, Person person)
    {
        if (!_rows.ContainsKey(rowId))
        {
            throw new InvalidOperationException($"There is no Row {rowId} in section {SectionId}.");
        }
        _rows[rowId].BookASeat(seatId, person);
    }

    public void AddASeat(RowNumber rowId, Seat seat)
    {
        _rows.TryGetValue(rowId, out var row);
        if (row != null)
        {
            row.AddASeat(seat);
        }
        else
        {
            throw new InvalidOperationException();
        }
    }
    public void AddARow(Row row)
    {
        if (_rows.ContainsKey(row.RowId))
        {
            throw new InvalidOperationException();
        }
        _rows.Add(row.RowId, row);
    }
} 

how can I prevent external objects from calling Row.AddSeat method, while allowing the aggregate root to call it ?

If you do not expose a Row or Rows as public property it automatically prevents others from calling it. E.g. in my example only Section has access to its own private collection of _rows and calls method AddSeat on a single row.

If you keep a state of the aggregate root private to itself it means that it can be changed through aggregate root methods only.

Ilya Palkin
  • 14,687
  • 2
  • 23
  • 36