7

I am modeling an aggregate root, which has several actions that perform operations against other entities, as you'd expect. The aggregate, however, has a state, and several of these operations can only be performed when the aggregate is in a particular state.

I created an implementation of the state pattern, so that the aggregate would simply delegate the action to the state concrete object. However, now that I have implemented it, I found meself with the following concerns:

  • There are operations that can be invoked in more than one state, thus I ended up repeating implementations.
  • There are operations that generate domain events, so I had to pass the root's event collection so they can add the events properly.
  • Some operations require access to the private members of the aggregate root, so I ended up either declaring them as internal (C#) or creating internal methods that modify the private members.

So now I'm wondering whether the implementation was worth it, or if the state object should only have CanPerformOperation1 properties, and let the aggregate root check this property and if false, throw an InvalidOperationException.

The following code is a summary of what I'm trying to attempt.

interface IState {
    void Register(DomainName domain, CustomerCode code);
    void Activate(ActivationManifest manifest);
    void Lock();
    void Unlock();
    void EnsureConsistency();
}
class NewState : IState {
    // can only call Register method, transitions to RegisteredState
}
class RegisteredState : IState {
    // can only call Activate method, transitions to ActiveState
}
class ActiveState : IState {
    // can call Lock or EnsureConsistency
    // Lock transitions to locked state
    // EnsureConsistency can transition to RestrictedState or ActiveState
}
class LockedState : IState {
    // can only call Unlock, transitions to ActiveState
}
class RestrictedState : IState {
    // can only call EnsureConsistency, which can transition
    // to ActiveState or RestrictedState
}

class Tenant {
    private IState _state = new NewState(this);
    private readonly UserAccountCollection _accounts;
    private readonly LicenseCollection _licenses;
    private readonly ApplicationCollection _applications;

    // had to make these internal accesors to be used by
    // EnsureConsistency in ActiveState and RestrictedState
    internal UserAccountCollection _accounts => _accounts;

    internal Application RegisterApplication(AppKey key, UserAccount admin){
        // this method is called by the RegisteredState.Activate method
        // so what's the point of delegating?
    }
    internal License RegisterLicense(LicenseKey key) {
        // this method is also called by the RegisteredState.Activate
        // method, just like the one above.
    }
    // etc
}

Now, this will only increase in complexity, as the customer requires me to add more methods that depend on state. So I was just wondering whether I should just add properties like CanRegisterApplication, CanRegisterLicense, etc., and then the states will only be acting as a flag switch.

What would be a proper way to implement what I'm trying to achieve? Or maybe I'm getting the state pattern wrong?

  • It seems that you break the Open-close principle. Every new state forces you to modify all the other state classes. – Constantin Galbenu Jan 30 '18 at 6:58
  • @ConstantinGalbenu what would you suggest? – Fernando Gómez Jan 31 '18 at 0:00
3

IMHO, "customer requires me to add more methods" is the exact reason why we need the state pattern in the first place. Without state pattern, it's likely that you need to repeat the switch or if/else in the new added methods.

From the implementation point view, abstract class is much easier than interface in state pattern.

Regarding your access modifier, you can make the each individual state as internal class of the client (Tenant class in your case, see nested types). By this way, you actually have better encapsulation as the external world doesn't need to know the actual state detail.

abstract class State {
    void Register(DomainName domain, CustomerCode code){};
    void Activate(ActivationManifest manifest){};
    void Lock(){};
    void Unlock(){};
    void EnsureConsistency(){};
}


class Tenant {
    class NewState : State {
        // can only call Register method, transitions to RegisteredState
    }
    class RegisteredState : State {
        // can only call Activate method, transitions to ActiveState
    }
    class ActiveState : State {
        // can call Lock or EnsureConsistency
        // Lock transitions to locked state
        // EnsureConsistency can transition to RestrictedState or ActiveState
    }
    class LockedState : State {
        // can only call Unlock, transitions to ActiveState
    }
    class RestrictedState : State {
        // can only call EnsureConsistency, which can transition
        // to ActiveState or RestrictedState
    }
    private State _state = new NewState(this);
    private readonly UserAccountCollection _accounts;
    private readonly LicenseCollection _licenses;
    private readonly ApplicationCollection _applications;

    // Should this go to abstract class State 
    private UserAccountCollection _accounts => _accounts;

    private Application RegisterApplication(AppKey key, UserAccount admin){
        // Should this go to abstract class State 
    }
    private License RegisterLicense(LicenseKey key) {
        // Should this go to abstract class State 
    }
    // etc
}

P.S: In java, people usually use enum to implement state pattern. However, .net doesn't allow polymorphism in enum.

  • Thank you. Yes I use an interface and then an abstract class, but I omitted them in the example to make it simpler. So basically you say to use nested types. Interesting. It might make lengthy implementation of the class, but it seems it would solve the problem. After all, what I'm trying to achieve is what you stated about encapsulation. Thanks, I'll give it a go! – Fernando Gómez Jan 30 '18 at 23:54
  • I guess that to avoid a big lengthy cs file, I could use a partial class declaration and put the nested types in a different file. This could work. I'll give it a go. Thanks again! – Fernando Gómez Jan 31 '18 at 0:00
  • I ended up using your suggestion. I used nested types for implementing each state, thus gaining access to the internal properties of the aggregate root, and thus eliminating the internal fields and members. Furthermore, I declared my Tenant class as partial, and created each type in its own cs file, improving the readability of the aggregate. – Fernando Gómez Feb 1 '18 at 0:39
5

I often encounter the same problem: I have some business-processes that require my aggregate to pass through several states, with some behavior specific to each state:

Saga

What I often do is split this aggregate with all its state classes into multiple separate classes, each one representing an aggregate in its specific state. For example, instead of one FinancialTransaction aggregate with states Registered, CheckedForFraud, Authorized, etc, I have RegisteredTransaction, CheckedForFraudTransaction, AuthorizedTransaction, etc.

Just to make it crystal clear, that's how it works. First, the command is issued by the client. It could be smth like Register transaction. Command gets validated, RegisteredTransaction is created and saved to db. Alongside Transaction is registered event is published with newly created transaction_id. It is consumed by some listener who wants to check whether this transaction is fraudulent. Say, it requires requesting some 3rd party service, and if it's ok, then CheckedForFraudTransaction is created. Under the hood it operates upon the same db table row (though it's not necessary) with the same transaction_id.

I like this approach, it doesn't make me to modify State's interface each time new behavior is added. Besides, each step is well isolated, could be mocked while integration testing, and the whole business-process is more clear.

  • Thanks for the response. A question, for several states I have functionality that is repeated, that is, states that can invoke the same method. With your proposal, how would you avoid repeating yourself? – Fernando Gómez Jan 30 '18 at 23:56
  • 1
    I would carefully consider whether those implementation are the same by coincidence or by design. Are they the same because they really model the same behavior, or they just happen to coincide in a certain moment of time? Very often I find that code duplication is not really a duplication, but a coincidence, that some pieces of code look the same. But If you're quite sure about duplication, well, generally, you have two options: inheritance and composition. – Zapadlo Jan 31 '18 at 8:38
  • +1 this also has the benefit of type safety on operations (instead of throwing InvalidOperationExceptions) – guillaume31 Feb 1 '18 at 11:04
  • Do you use a repository for each aggregate type (CheckedForFraudTransactionRepo, RegisteredTransactionRepo, etc) or just 1 repository that handles all aggreate types? – F21 May 23 '18 at 3:40
  • Now I use a repository for each aggregate type. – Zapadlo May 23 '18 at 8:08
0

Your implementation breaks the Open/closed principle because every new state implementation forces you to modify all the other implementations. I'm proposing an alternate approach, based on the Strategy pattern.

The idea is that your Tenant at any given moment is in a State and every State knows only the states that it may transition to. This frees your Tenant of the state transition algorithm. However, the Tenant is the one that commands the state transition, it is the one in control.

On the other hand, States should not know about the Tenant as they have only a single responsibility: to permit or reject transition to another State.

Following this pattern, the Tenant becomes very clean.

Here is an example of implementation (in PHP):

<?php

interface IState
{
    /**
     * @param IState $newState
     * @throws \Exception
     */
    public function tryTransition(IState $newState): void;
}

class NewState implements IState
{
    public function tryTransition(IState $newState): void
    {
        if (!($newState instanceof RegisteredState)) {
            throw new \Exception("Can only register");
        }
    }
}

class RegisteredState implements IState
{
    public function tryTransition(IState $newState): void
    {
        if (!($newState instanceof ActiveState)) {
            throw new \Exception("Can only activate");
        }
    }
}

class ActiveState implements IState
{
    public function tryTransition(IState $newState): void
    {
        if (!($newState instanceof RestrictedState) && !($newState instanceof ActiveState)) {
            throw new \Exception("Can only restrict or activate");
        }
    }
}

class LockedState implements IState
{
    public function tryTransition(IState $newState): void
    {
        if (!($newState instanceof ActiveState)) {
            throw new \Exception("Can only activate");
        }
    }
}

class RestrictedState implements IState
{
    public function tryTransition(IState $newState): void
    {
        if (!($newState instanceof RestrictedState) && !($newState instanceof ActiveState)) {
            throw new \Exception("Can only restrict or activate");
        }
    }
}

class Tenant
{
    /** @var IState */
    private $state = 0;

    /**
     * Other private data
     */
    private $_accounts;
    private $_licenses;
    private $_applications;

    public function __construct()
    {
        $this->state = new NewState();
    }

    public function RegisterApplication(AppKey $key, UserAccount $admin)
    {
        $this->tryTransitionOrThrowException(new RegisteredState());

        $this->_applications[] = new Application($key, $admin);
    }

    /**
     * @param IState $nextState
     * @throws Exception
     */
    private function tryTransitionOrThrowException(IState $nextState)
    {
        $this->state->tryTransition($nextState);
        $this->state = $nextState;
    }

    public function Activate(LicenseKey $key)
    {
        $this->tryTransitionOrThrowException(new ActiveState());

        $this->_licenses[] = new License($key);
    }

    public function Lock()
    {
        $this->tryTransitionOrThrowException(new LockedState());
    }

    public function UnLock()
    {
        $this->tryTransitionOrThrowException(new ActiveState());
    }
    // etc
}
  • Thanks for your input! Let me check it (I'm not php-fluent) and see how it works. A question though, doesn't the GoF state design pattern say that it is the concrete states' responsibility to transition to the next state? If I read this correctly, if I need a new state have to modify the aggregate root to transition to said states, instead of just modifying the IState implementations. Wouldn't this lead to a violation of said O/C principle? – Fernando Gómez Feb 1 '18 at 0:36
  • @FernandoGómez mine is not the State design pattern but Strategy. And you don't break the O/C principle if you add new IState implementation and if you modify the Aggregate. Modifying the Aggregate if the business change is normal. – Constantin Galbenu Feb 1 '18 at 5:29
  • @FernandoGómez also, in an Aggregate you use other classes to move some low level code outside of it. An Aggregate should have code that is at the right level of abstraction. – Constantin Galbenu Feb 1 '18 at 6:33
0

What you are trying to build is State Machine and akka .Net Framework provides this functionality very neatly.

Read this link and patterns there will answer your questions

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.