Are names like OrderCreation and UserRegistration suitable names for business logic / domain classes

Question

We have moved to a more SRP model and found coming up with class names challenging. Previously we had a Order class that looked something like this:

public class Order
{
    public void Create()
    {
    }

    public void Dispatch()
    {
    }

    public void Cancel()
    {
    }
}

We are now splitting these 3 methods into seperate classes so they look like what i have described below:

public class OrderCreation
{
    public void CreateOrder(OrderDTO order)
    {
    }
}

public class OrderDispatch
{
    public void DispatchOrder(OrderDTO order)
    {
    }
}

public class OrderCancellation
{
    public void CancelOrder(OrderDTO order)
    {
    }
}

Is the naming of these classes and public methods sensible? Do they follow what naming should look like in a business logic or domain class? (Remember we are following SRP)

UPDATE: The classes have been modified so they are not anemic. Based on feedback we think this is a better solution.

public class OrderCreation
{
    public string CustomerName {get;set;}
    public string CustomerEmail {get;set;}
    public string Address {get;set;}

    public void CreateOrder()
    {
    }
}

public class OrderDispatch
{
    public string OrderNumber {get;set;}
    public string Courier {get;set;}

    public void DispatchOrder()
    {
    }
}

public class OrderCancellation
{
    public string OrderNumber {get;set;}

    public void CancelOrder()
    {
    }
}

Answer 1

If you push it even little further, and do this :

public interface ICommand
{
    void Invoke();
}

public class OrderCreation : ICommand
{
    private readonly OrderDTO _order;

    public OrderCreation(OrderDTO order)
    {
        _order = order;
    }

    public void Invoke()
    {
        // do the logic
    }
}

public class OrderDispatch : ICommand
{
    private readonly OrderDTO _order;

    public OrderDispatch(OrderDTO order)
    {
        _order = order;
    }

    public void Invoke()
    {
        // do the logic
    }
}

public class OrderCancellation : ICommand
{
    private readonly OrderDTO _order;

    public OrderCancellation(OrderDTO order)
    {
        _order = order;
    }

    public void Invoke()
    {
        // do the logic
    }
}

You end up with command pattern. This has major advantages :

• The execution is separated from invocation. This allows other code to do something interesting with every executed command.
• Allows to use inheritance to implement common behavior for invocation. For example, database transaction can be started and committed in base class and child class just uses database connection.
• Allows composition of the commands to form more complex commands.

The major disadvantage is increase in complexity. This increase is only really viable if each command is has high complexity in itself. If each method is just few lines of code, building a whole command around it may seem unnecessary.

Answer 2

These look like gang of 4 Command objects (a bit). I dont like them but if thats what you want then go for it - and these names make perfect sense

From my point of view I now have my code that knows all about orders spread out way too much.

I would only abstract like this if I truly needed common behavior between 'Command's - like being able to persist then, pipeline them, indirectly execute them from scripts etc. But even then I would have a real class underneath. Maybe that would be a static class that has Order.Create, Order.Delete etc

Answer 3

In your rewrite, what does OrderDTO do? If it is indeed just an anemic "value object", as implied by the name, what you have done is separate the logic from the data, moving from OOP to old fashioned procedural programming. IMO a bad thing. It happens when SRP is overused and becomes the ZRP, a.k.a. Zero Responsibility Principle.

OTOH, if OrderDTO retains a lot of logic, first, give it a better name to reflect that. And I'd agree that possibly your original Order object did " too much" and it is a candidate for refactoring using SRP.

Answer 4

In and of itself, new OrderCancellation().CancelOrder(OrderDTO order); doesn't seem like much of an improvement to me.

I do think, however, that it is appropriate to separate concerns between an "Ordering System" and the "Order Submission Form". The ordering system's responsibility is to accept or reject orders (and otherwise manage orders, like assign an order id), whereas the order form conveys the content of the order request. This kind of goes to the change you're showing regarding the introduction of the OrderDTO as separate (responsibility/class) from creation and cancellation. (An order can be a complex entity with numerous line items, payment info, etc.. so it is reasonable to have an order form separated from an ordering system.)

I consider that an ordering system having both accept and cancel is a reasonable notion of Single Responsibility (i.e. managing orders or in some sense, the queue of orders); meaning that accept and cancel ought not be separated into different classes due to SRP. (You wouldn't separate the add and remove methods on a collection into separate classes (it wouldn't work), managing the collection is just one responsibility.)

Still, if you accept cancellations as recorded events and/or submission requests that have their own lifetime, what you're showing could make some sense as each such event (order, cancel) is a request of a different logical type. I would consider these items as requests, not guaranteed actions, i.e. named like OrderCancellationRequest.