Web-app for tracking containers

Question

Nowadays I am learning Domain Driven Design intensively. I am reading the book called Domain Driven Design by Eric Evans, and at the same time I try to apply the knowledge on a real life project I work on recently. My domain is the following: The web-app I am developing is responsible for tracking containers. These containers can contain different kind of building materials, like bricks or concrete, and they are transferred by trucks from one place to another. A customer can have an order, and an order can have multiple containers placed on the customer's address. Where I am busy right now is to build and design a container entity using DDD. I have done it, but there are several parts of the code where I am doubting and having questions. As a technical info: I am using .Net Core 2.0 with EF Core, and for database I use Azure SQL database.

Let's see first the my Container domain entity itself:

public class Container: Entity<Guid>
{
    public int Size {get; }
    public string Status { get; private set; }
    public int NumberOfChanges { get; private set; }

    public int? Price { get; set; }
    public string Remark { get; set; }
    public DateTime LayDownDate { get; set; }
    public DateTime? ChangeDate { get; set; }
    public DateTime? TakeUpDate { get; set; }

    public long LastTouchedById { get; private set; }
    public TruckDriver LastTouchedBy { get; set; }

    public long OrderId { get; private set; }
    public Order Order { get; set; }

    private bool IsNotPlaced => Status.Equals(ContainerStatus.NotPlaced, StringComparison.InvariantCulture);
    private bool IsBeingFilled => Status.Equals(ContainerStatus.BeingFilled, StringComparison.InvariantCulture);

    public Container(int size)
    {
        if(!ValidContainerSizes.SizeIsValid(size)) {
            throw new InvalidOperationException($"Invalid container size of {size}");
        }

        Size = size;
        Status = ContainerStatus.NotPlaced;
    }

    public void Place(long byTruckDriverId) {
        if (!IsNotPlaced)
        {
            throw new InvalidOperationException($"Container can only be placed when it is not placed yet. But the current container status is {Status}");
        }

        LastTouchedById = byTruckDriverId;
        Status = ContainerStatus.BeingFilled;
    }

    public void Change(long byTruckDriverId)
    {
        if (!IsBeingFilled)
        {
            throw new InvalidOperationException($"Container can only be changed when it is placed and being filled. But the current container status is {Status}");
        }

        NumberOfChanges++;
        LastTouchedById = byTruckDriverId;
    }

    public void TakeAway(long byTruckDriverId)
    {
        if (!IsBeingFilled)
        {
            throw new InvalidOperationException($"Container can only be taken away when it is placed and being filled. But the current container status is {Status}");
        }

        NumberOfChanges++;
        LastTouchedById = byTruckDriverId;
        Status = ContainerStatus.TakenAway;
    }
}

Where the ContainerStatus is the following:

public static class ContainerStatus
{
    public const string NotPlaced = "NotPlaced";
    public const string BeingFilled = "BeingFilled";
    public const string TakenAway = "TakenAway";

    public static IEnumerable<string> GetAll()
    {
        yield return NotPlaced;
        yield return BeingFilled;
        yield return TakenAway;
    }
}

And last but not least the ValidContainerSizes looks like:

public static class ValidContainerSizes
{
    public static bool SizeIsValid(int size) => GetSizes().Contains(size);

    public static IEnumerable<int> GetSizes()
    {
        yield return 3;
        yield return 4;
        yield return 5;
        yield return 6;
    }
}

My design decisions: First of all I tried to use as many as private setters I could and place the all the logics (relating to container) inside the class. As for the properties Price, Remark, LayDownDate, ChangeDate, TakeUpDate are just plain data holders, so I want to change them outside of the class so I left the properties with public setters. I know it is a so called anti-pattern, but I did not really wanted to add just public setters explicitly for them. As for Status, NumberOfChanges and LastTouchedById, they are used in the Container functions and they are privately set. As for ContainerStatus I use static string fields instead of enums, because it is easier to persist and maintain them in relational databases. I also have numerous tests written for the code, but I do not include them since it is irrelevant to the question.

My concerns and questions are the following about my Container domain entity.

1. I am coming from the Java world where the constructor is placed after the class fields. I see some examples where the constructor is on the first place in C# codes. To be honest I prefer to place the constructor after the fields so if I read a class first, I can see what the the properties of them are. What is the convention for placing the constructor in C#, using DDD?
2. I was thinking that my Container domain entity contains a lot of properties. What do you think, it is too rich for a domain entity, or reasonable?
3. As you can see I use contract validations in the constructor and in all the methods. Is it a good practice to throw an InvalidOperationEception or shall I use some other contract validation framework? If so, which contract validation framework you would recommend to use?
4. I have created two more properties IsNotPlaced and IsBeingFilled. To be honest it is the first time I use these expressions in C#. Is it a good practice, or it is anti-pattern and better to have just old plain methods for them? I personally like them!
5. I tried to extract some part of this code to a DDD ValueObject, but I could not really come up with a good idea. What is your opinion, there is room for extracting some properties and logic to a ValueObject?

Furthermore I am open for any other remark, improvement point, refactoring idea. What do you think where I could improve my code using DDD? I include additional code if needed for the question. Thanks!

Answer 1

First of all, congratulations with your efforts to apply DDD principles and finding Evans's excellent book. You're on the right track. Just a couple of comments:

What is the convention for placing the constructor in C#?

It doesn't matter. If you (and your team) expect to find constructors at the bottom of a code file, fine. Today there's an abundance of code navigation tools, built-in in Visual Studio or third-party plugins like Resharper. The exact position of type members isn't as important any more as it used to be.

better to have just old plain methods for them? (properties)

Properties are methods below the hood, so the difference seems futile. But there are good reasons to consider carefully which of the two should be chosen:

• EF will try to map properties in mapped entities to database columns. Unmapped properties should explicitly be excluded by the NotMapped attribute.
• UI components (esp. grids) may inadvertently display these properties, while they won't automatically display method results.
• Serializers will serialize properties, not method results. The common Json serializers today have an opt-out policy.
• Semantics: properties convey identity, methods convey capability. That is, properties shouldn't really do much, whereas methods are all about doing. Properties aren't expected to change an object's internal state other than the value they expose, whereas methods may do just that. Since properties are about identity it's very common to have "Is" properties, like IsPlaced (more about naming later).

too rich for a domain entity?

That's up to you. One important design principle is Single Responsibility. Many properties (and methods) may be a tell-tale of too many responsibilities in one class. But real-world "things" may just have many characteristics that comprise their identity, why not? The world is complex after all. Having many methods is more suspicious than many properties.

good practice to throw an InvalidOperationException?

Probably not. This is a wide topic, but the general idea is to handle anomalies instead of throwing them back. The object itself is probably the expert in deciding what to do. For example, if someone tries to place a container that is already placed, the container could decide to simply ignore the effort. Instead of void methods throwing exceptions you should create methods that return useful information on what happened, successful or not. Throwing exceptions forces the calling code to be needlessly defensive.

there is room for extracting some properties and logic to a ValueObject?

That sounds like a hammer looking for a nail. I don't see any obvious reasons to transfer properties to a value object here. Immutability is a different topic altogether.

A couple of design considerations

• Naming: Don't use negations in property names. Double negations like if (!IsNotPlaced) quickly cause a brain fry.

• I use static string fields instead of enums

  I don't think that's a good idea. Why is it "easier to persist and maintain them in relational databases"? A database is not a human being, it couldn't care less whether it stores strings or integers. Integers can easily serve as foreign keys to lookup tables. Having that, changing a description will be trivial compared to updating large numbers of strings.

• Data layer entities aren't domain entities. At least, not always.

• Finally, apparently containers have a fixed set of allowed state transitions. Maybe a state machine could be helpful here. Also, I think you should keep record of these transitions in transaction records instead of counting number of changes.

Answer 2

I'll answer questions 3 & 4:

Is it a good practice to throw an InvalidOperationEception or shall I use some other contract validation framework?

I find that all but the first InvalidOperationException usages are almost correct. In the first case it should have been ArgumentOutOfRangeException because it's thrown after checking the argument.

The other validations would be correct if the user had access to the properties IsNotPlaced & IsBeingFilled so he can prevent those exceptions from beign thrown. Actually he could use the Status property to do the same checks but why should he implement this again if you already provided shortcuts?

MSDN says here:

The InvalidOperationException that is thrown when a method call is invalid for the object's current state.

but since both properties are private the user cannot (easily) know that the object's state is invalid and cannot do anything about it and thus cannot avoid this exception.

These two properties might not be perfect (you could have used enum for Status) but they are convenient and if they were public they would greatly complement the InvalidOperationException that then would be avoidable.

Answer 3

EF has zero compatibility with DDD. Any attempt to make your DDD code be suitable for direct EF mapping prevents you from implementing DDD solution – there is no way to do it directly. But any technical problem could be solved by extra level of indirection - have a look at .NET DDD books:

• Patterns, Principles, and Practices of Domain-Driven Design on Amazon – Chapter 21 Repositories

• Domain Modeling Made Functional: Tackle Software Complexity with Domain-Driven Design and F# on Amazon.com – First part of the book is language neutral

It will save you a lot of very lower back pain trying to adapt Java concepts to C# world.

Also, do not use Entities/Aggregates as attribute holders. That kind of crime is never paying out. One should not use business object to read data, only to modify them. It could also happen that you might not need DDD here, as your case is too simple and you just overcomplicate things… I do not know your use cases and business events, but as for the stuff actually captured in your code:

public abstract class Container : Entity<Guid>
{
    public static NotPlacedContainer New(int size) => new NotPlacedContainer(Guid.NewGuid(), size);
    protected Container(Guid id, int size) : base(id) => Size = size;
    public int Size { get; }
}

Where:

public class NotPlacedContainer : Container
{
    public NotPlacedContainer(Guid id, int size) : base(id, size) { }
    public PlacedContainer Place(long byTruckDriverId) =>
        new PlacedContainer(Id, Size, byTruckDriverId, 0);
}

Where:

public class PlacedContainer : Container
{
    public PlacedContainer(Guid id, int size, long lastTouchedBy, int numberOfChanges)
        : base(id, size) => (LastTouchedBy, NumberOfChanges) = (lastTouchedBy, numberOfChanges);

    public long LastTouchedBy { get; }
    public int NumberOfChanges { get; }

    public PlacedContainer Change(long byTruckDriverId) =>
        new PlacedContainer(Id, Size, byTruckDriverId, NumberOfChanges + 1);

    public TakenAwayContainer TakeAway(long byTruckDriverId) =>
        new TakenAwayContainer(Id, Size, byTruckDriverId, NumberOfChanges + 1);
}

Where:

public class TakenAwayContainer : Container
{
    public TakenAwayContainer(Guid id, int size, long lastTouchedBy, int numberOfChanges)
        : base(id, size) => (LastTouchedBy, NumberOfChanges) = (lastTouchedBy, numberOfChanges);

    public long LastTouchedBy { get; }
    public int NumberOfChanges { get; }
}