4
\$\begingroup\$

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!

\$\endgroup\$
3
\$\begingroup\$

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.

\$\endgroup\$
  • 1
    \$\begingroup\$ Instead of void methods throwing exceptions you should create methods that return useful information on what happened, successful or not. This is actually a very bad advice. Returning status codes is mostly considered a bad practice see here and here. \$\endgroup\$ – t3chb0t Jun 23 '18 at 9:37
  • \$\begingroup\$ @t3chb0t Maybe I shouldn't have touched the subject. Your links at least demonstrate that there's much more to it than these few words. I wasn't aiming at "magic values" by the way. \$\endgroup\$ – Gert Arnold Jun 23 '18 at 11:15
  • \$\begingroup\$ @GertArnold Waoo, thank you very much for the detailed answers! I love it! As for your question about using strings instead of enums: I did not like to way that the enum values stored in dabatase as integers. When I read the tables, it was always a pain to figure what is the corresponding enum value of an integer and took time. Check this answer: codereview.stackexchange.com/questions/154676/… . \$\endgroup\$ – mirind4 Jun 25 '18 at 5:39
  • \$\begingroup\$ @GertArnold There is of course a workaround for for the mentioned issue, f.e.: nodogmablog.bryanhogan.net/2014/11/… . But I do not want to place data layer relevant logic into my domain layer, it does not belong to it as far as i can judge the situation. Or maybe do you know a better solution for that? :) \$\endgroup\$ – mirind4 Jun 25 '18 at 5:42
  • \$\begingroup\$ @GertArnold I think the answer of my previous question lies in your main answer, the look-up table is the way to go for it ;) \$\endgroup\$ – mirind4 Jun 25 '18 at 17:00
4
\$\begingroup\$

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.

\$\endgroup\$
  • \$\begingroup\$ Actually, there should be no way to get that exception at all as long as real DDD solution is considered. Have a look at my answer - API shape captures operation availability. I would say - one just do not need DDD if it looks like overkill :) \$\endgroup\$ – Dmitry Nogin Jun 25 '18 at 3:41
  • \$\begingroup\$ @t3chb0t Thanks for your answer, I learned again something new from it! :) As Dmitry Nogin advised, I am going to refactor my code with introducing an abstract container class, so probably the exceptions will not be thrown anymore. \$\endgroup\$ – mirind4 Jun 25 '18 at 5:56
4
\$\begingroup\$

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; }
}
\$\endgroup\$
  • \$\begingroup\$ Iike that design and I like reading your answers and your point of view. You should write more often :-) \$\endgroup\$ – t3chb0t Jun 25 '18 at 3:55
  • \$\begingroup\$ Thanks @t3chb0t. DDD is my favorite – how I wish to find a hardcore business logic job without messing with corporate organizational bs – creativity always comes from chaos! :) Never had a chance to see it this way by myself using Microsoft tech though… It looks like that such projects are all on Scala or something :) \$\endgroup\$ – Dmitry Nogin Jun 25 '18 at 4:25
  • \$\begingroup\$ @DmitryNogin nice one, I like your design as well! and also thanks for the book recommendations, I am definitely going to read the mentioned parts! ;) \$\endgroup\$ – mirind4 Jun 25 '18 at 5:46
  • \$\begingroup\$ @DmitryNogin Yes, you are right about that I might not need DDD for this application, it is not that complex. It is a real application though, already deployed to production. But I needed a "learning project" in order to learn and apply DDD, and this project is ideal for me, since it is partly mine (used in family business) \$\endgroup\$ – mirind4 Jun 25 '18 at 5:52
  • 1
    \$\begingroup\$ I also like this design for immutability. Maybe one drawback is that it takes a relatively large code modifications to change any flow in container states. \$\endgroup\$ – Gert Arnold Jun 26 '18 at 7:27

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.