2

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?

UPDATE

I have found a few interesting articles to read in the topic:

How to implement a FSM - Finite State Machine in Java

When you have more complex state handling

Five
  • 378
  • 2
  • 19

2 Answers2

1

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

StepUp
  • 36,391
  • 15
  • 88
  • 148
  • Thanks for the idea. I was thinking about doing the same (more or less). I am afraid that it will bring up code duplications. Like what if two statuses are doing the same for an action? How should I extract the behavior? With strategy pattern? – Five Mar 13 '22 at 22:15
  • @Five yeah, you are right! And behavior can already be extracted through choosing desirable implementation from ‘AccountStateByStatus’ of class ‘StatusToAccountState’ – StepUp Mar 14 '22 at 08:52
  • @Five if you will have code duplications, then you can move it to abstract class and reuse this behaviour in derived classes – StepUp Mar 14 '22 at 09:40
  • 1
    Thanks, I was working on it these days. Found it very hard to decide when these state-related classes are creating extra complexity and making the code harder to maintain. (Sometimes a few ifs looks like a better solution, mostly when I don't have much code.) – Five Mar 16 '22 at 22:58
0

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

Oli
  • 76
  • 8