Design pattern for state-based entity handling-CodePudding

My question is about what should be the most OOP solution and the right design pattern for my situation. We have a user entity and multiple account entities belong to the user. Account entities can have multiple states and we can execute multiple operations on accounts. The outcome of these operations is based on the account entity's state.

I have the following code which is based mostly on switch (sometimes it looks like a few "if"). I would like to change it but cannot find the right design pattern.

enum Status {
    ACTIVE, INACTIVE, DELETED; 
}

@Entity
class Account {
    private long id;
    private long userid;
    private Status status;

    //...
}

class AccountService{

    Account delete(long id) {
        //...
        
        if (accountInfo.getSatus() == DELETED) {
            throw new IllegalOperationException();
        }
        if (accountInfo.getStatus() == ACTIVE || accountInfo.getStatus()) {
            accountInfo.setStatus(DELETED);
            accountInfoRepository.save(accountInfo);
        }
    }

    Account create (Account account) {
        // various operations based on state
    }
}

I really want to refactor these codes, I fear that as soon as our service grows it will contain more "magic" and will be hard to maintain. And if we would like to introduce a new state it will be nearly impossible.

My junior mind thought that I should have state objects which would implement all the operations, in pseudo-code style:

class AccountService {
    private StateFactory stateFactory;
    private AccountRepository accountRepository;

    Account delete(long id) {
        final Optional<Account> account = accountRepository.findById(id);
        Account deletedAccount = account.map(stateFactory::getByState)
                                        .map(accountState -> accountState.delete(account))
                                        .orElseThrow(() -> new IllegalOperationException());
        return accountRepository.save(deletedAccount); 
    }

    Account create (Account account) {
        // various operation based on state
    }
}

and:

class ActiveState extends AccountState {

    @Override
    public Account delete(Account account) {
        //implementation
    }

    @Override
    public Account activate(AccountInfo) {
        // implementation
    }
}

and:

interface AccountState {
    Account activate(AccountInfo);
    Account delete(AccountInfo);
}

I know there must be a better implementation for this problem. Which other design patterns are suitable for this setup?

CodePudding user response：

If I understood question correctly, then it is necessary to apply some action by its state. If it is true, then we can use Factory pattern to get desired object which can execute some action. Mapping between state and action can be putted into HashTable.

So let's see an example of code. I will write via C#, but this code can be easily translated to Java because languages have many syntax similarities.

So we will have enum of statuses:

public enum Status 
{ 
    Active,
    Deleted,
    Foo
}

and states of AccountState

public abstract class AccountState
{
    public abstract void ExecSomeLogic();
}

public class ActiveState : AccountState // "extends" instead of ":" in Java
{
    public override void ExecSomeLogic()
    {
    }
}

public class DeletedState : AccountState // "extends" instead of ":" in Java
{
    public override void ExecSomeLogic()
    {
    }
}

public class FooState : AccountState // "extends" instead of ":" in Java
{
    public override void ExecSomeLogic()
    {
    }
}

Then we need mapper class of Status to their AccountState:

public class StatusToAccountState 
{
    public Dictionary<Status, AccountState> AccountStateByStatus { get; set; } = 
        new Dictionary<Status, AccountState>() // HashMap in Java
    {
        { Status.Active, new ActiveState() },
        { Status.Deleted, new DeletedState() },
        { Status.Foo, new FooState() },
    };
}

And then in your service you can use it like this:

void Delete(long id, Status status)
{
    StatusToAccountState statusToAccountState = new StatusToAccountState();
    AccountState accountState = statusToAccountState.AccountStateByStatus[status];
    accountState.ExecSomeLogic();
}

If there are many logic to figure out what Status of object is, then you can create some class which will have just one responisibility of figuring out what state of object is:

public class StatusManager
{
    public Status Get() 
    {
        return Status.Active; // or something another based on logic
    }
}

After doing this, your classes will correspond to the single responsibility principle of SOLID. Read more about single responsibility principle of SOLID here

CodePudding user response：

Too many switch-/if-Statements indicate the code smell "Tool Abusers" (see M. Fowler "Refactoring"). Use the polymorphism mechanics to solve this. https://refactoring.guru/smells/switch-statements