How to separate responsibilities when requirements evolve in this way?

Question

First my requirement was

"We can create an account an put money on it, when we buy an item we decrease the account"

So my AccountController looked like

class AccountController
{
    private IAccountDataSource _accountDataSource;

    Create(Account anAccount)
    {
        _accountDataSource.Insert(anAccount);
         Render(anAccount.Id);
     }
}

But then there is a new requirement "Some people can have a free account (all the items will be free), but if we create a real account then we remove the free account"

So my controller.Create became

Create(Account anAccount)
{
    _accountDataSource.Insert(anAccount);
    RemoveFreeAccount(anAccount.Customer);
    Render(anAccount.Id);
}

RemoveFreeAccount(Customer aCustomer)
{
    _accountDataSource.Remove(new AccountFilter() { Type='Free', CustomerId=aCustomer.Id });
}

But for me it feels like I should put this RemoveFreeAccount somewhere else, but I don't know where because IAccountDataSource is just suppose to handle the data storing.

Answer 1

The problem shows that you are breaking SRP. Your controller should not contain business logic. Using a repository directly in the controller forces you do put all logic in it and therefore getting two responsibilities (being a bridge between the M in MVC + handling the business logic).

The first refactoring part should be to move the business logic into the model (in MVC not to be confused with an entity model or a view model)

this giving your original code the following structure:

public class AccountService
{
    void CreateAccount(string accountName)
    {
       var account = new Account(accountName);
        _dataSource.Create(account);
        DomainEvents.Publish(new AccountCreated(account));
    }
}

public class AccountController
{
    private AccountService _service;

    Create(AccountViewModel model)
    {
        var account = _accountDataSource.Create(model.Name);
        Render(account.Id);
     }
}

The change might look minor, but is important:

1. The controller now has only one reason to change (mapping between the view and the model)
2. Any change of business requirements will not force changes to the UI layer.
3. Changes in requirements are only made in one place

To add support for free accounts I would use an event driven model:

public class FreeAccountService : ISubscriberOf<UserCreated>, ISubscriberOf<AccountCreated>
{
    public FreeAccountService(AccountService)
    {
    }

    public void HandleEvent(UserCreated domainEvent)
    {
        accountService.Create(new FreeAccount());
    }

    public void HandleEvent(AccountCreated domainEvent)
    {
        var freeAccount = dbSource.GetFreeAccount();
        if (freeAccount != null)
            accountService.Delete(freeAccount)
    }
}

Since it requires no changes in the other account service.