Improving the design of state transitions in a business application: the case of cancellation

Question

I have to load certain object from the DB, and propagate it through few state and save after each state propagation.

Let us illustrate this with the example of an Order entity. I now have to load few pending orders from DB and Cancel them based on few criteria. If these criteria don't match I should not cancel them.

I have the below code in my Application layer:

public void CancelPendingOrders() {
  var pendingOrders = _orderRepository.GetPendingOrders();
  _orderService.CancelOrders(pendingOrders);
}

public class CancelOrderCommand {
  //other classes which are used in OrderCanBeCancelled are injected in .ctor
  public void Execute(Order order) {
    if(CanExecute(order))    // OrderCanBeCancelled 
      order.SetStatus(OrderStatus.Cancelled);
    }
}

And my domain service like below

public class OrderCancelService : IOrderCancel{
  void CancelPendingOrders(Orders[] orders){
    foreach (var order in orders)
    {
      _orderCancelCommand.Execute(order);  
    }   
  } 
}

public class Order{
    public Order(OrderId orderId, OrderStatus orderStatus){
        // assigned to local private fields.
    }

    public event EventHandler<OrderStatusChangedEventArgs> OrderStatusChanged;
    internal void ChangeOrderStatus(OrderStatus status){
      var oldStatus = new OrderStatus(_status);
      _status = status;
      if(OrderStatusChanged != null){
         OrderStatusChanged(this, new OrderStatusChangedEventArgs(oldStatus,_status));
     }
    }
}

I have few classes which listen to the OrderStatusChangedEvent and perform other tasks like for example sending a notification and saving the updated status back to the database.

If you observe the above Order class I don't have separate commands for each OrderStatus like Cancel() and is rather anaemic.

I have it this way because I do not want to inject other commands and services into the Order class, because I have to perform CanOrderBeCancelled for each status change.

From what I have currently it is pretty straight forward for me to introduce a new Command class for each status if required and inject other services in those classes leaving my Order class pure.

Other option

    public class Order{
      public Order(OrderId orderId, OrderStatus orderStatus, ICommandFactor orderCommandFactory){
        //assigned to local private fields.
      }

      public event EventHandler<OrderStatusChangedEventArgs> OrderStatusChanged;

      public void Cancel(){
        _orderCommandFactory.GetCancelCommand().Execute(this);
      }

      public void Success(){
        _orderCommandFactory.GetSuccessCommand().Execute(this);
      }

      internal void SetStatus(OrderStatus status){
        var oldStatus = new OrderStatus(_status);
        _status = status;
        if(OrderStatusChanged != null){
          OrderStatusChanged(this, new OrderStatusChangedEventArgs(oldStatus,_status));
        }
      }
    }

The above approach has well defined methods on the Order class but for ideal separation of concerns I'll have to inject a command factory, and each commands would call the internal SetOrderStatus method like earlier. The commands without being injected to the IOrderService will now be injected to the Order themselves.

I can also inject a IOrderService into the Order class instead of the command factory and my class would look like below:

    public class Order{
      public Order(OrderId orderId, OrderStatus orderStatus, IOrderService orderService){
        //assigned to local private fields.
      }

      public event EventHandler<OrderStatusChangedEventArgs> OrderStatusChanged;

      public void Cancel(){
        if(orderService.CanCancel(this)){
           SetStatus(OrderStatus.Cancel);
        }
      }

    public void Success(){
      if(orderService.IsSuccessful(this)){
          SetStatus(orderStatus.Success);
      }
    }

    internal void SetStatus(OrderStatus status){
      var oldStatus = new OrderStatus(_status);
      _status = status;
      if(OrderStatusChanged != null){
         OrderStatusChanged(this, new OrderStatusChangedEventArgs(oldStatus,_status));
      }
    }
  }

Also my Application class will look like below:

    public class OrderCancelApplicaiton{
      public void CancelPendingOrders(){
        var pendingOrders = _orderRepository.GetPendingOrders();
        foreach (var pendingOrder in pendingOrders)
        {
            pendingOrder.Cancel();
        }
      }
    }

Finally I can also think of not having any commands which check if an Order can be cancelled, but rather have an additional OrderStatusChanged event handler that checks if the Order can indeed be cancelled, if not move it back to the previous state, the problem with this approach is that the 'Order' object will remain in an invalid state even if it is in-memory and momentarily before it is moved back to its previous state if it should not be cancelled.

Are you comfortable injecting Commands and Services into your domain models ?
What would you have done and what is the prescribed method for such problems ?
Would you rather Cancel on the domain object and let a handler catch it and 'Revert' it back if it should not have been cancelled ?

Answer 1

"load few pending orders from DB and Cancel them based on few criteria, if these criteria don't match I should not cancel them"

I would go with a CancelOrderService similar to your first example.

As you say the benefit of this over Order.Cancel is that you can add random business logic services without changing existing code.

The one flaw in your example is I think, getting more than one order to cancel before looping through them.

You need to design your Worker programs to be able to run simultaneously with themselves and other status changing workers.

This means locking the orders they are working on, and you want to lock as few as possible at a time.

Answer 2

Hey whatever works for you. Here are a couple observations though.

Subject, verb, and object

A call like myOrder.Cancel() might be described in English as

An order is canceled.

I've learned that using the passive voice tends to introduce ambiguities, so let's switch to an active voice.

A user cancels an order.

Wait, wouldn't that be myUser.Cancel(order)?

Maybe it doesn't matter... oh wait, what about these cases:

A CSR cancels an order

or

The system automatically cancels an expired order.

or

The system automatically cancels an order due to out of stock.

It seems like this cancel operation may involve other objects and other information, such as reason.

So maybe a unary operation such as myOrder.Cancel() isn't sufficient to accommodate the kinds of requirements that I might expect to be common.

Based on this reason, I don't think that domain model objects ought to have operations of this kind. If they have unary operations, they should be operations that apply to their own state only.

Coordination across objects

Now it sounds like you have a clever event system whereby model objects can raise and subscribe to events, and you handle coordination that way.

Personally I prefer to have a different layer of abstraction deal with cross-object coordination. The upper layer will call to the lower layers to get state variables updated.

Why do I do this? Well, if you just have one layer where objects are free to call each other, you can end up with circular or redundant calls, and it isn't always clear who is supposed to do what. But if I have a separate layer, there is no risk of circular calls, because the lower layer cannot call the upper layer. Thus the sequence of calls looks like a tree instead of an undirected graph.

IMO domain objects should operate on themselves, and business or workflow objects should be used to coordinate business transactions, which often include more than one domain object.