6

I have a bank account domain as listed below. There can be SavingsAccount, LoanAccount, FixedAccount and so on. One user can have multiple accounts. I need to add a new functionality – get all accounts for a user. Where should be the function written and how?

It would be great if the solution follows SOLID principles( Open-Closed principle,…) and DDD.

Any refactoring that would make the code better is welcome.

Note: The AccountManipulator will be used by a website client over a web service.

namespace BankAccountBL
{
public class AccountManipulator
{
    //Whether it should beprivate or public?
    private IAccount acc;

    public AccountManipulator(int accountNumber)
    {
        acc = AccountFactory.GetAccount(accountNumber);
    }

    public void FreezeAccount()
    {
        acc.Freeze();
    }

}

public interface IAccount
{
    void Freeze();
}

public class AccountFactory
{
    public static IAccount GetAccount(int accountNumber)
    {
        return new SavingsAccount(accountNumber);
    }
}

public class SavingsAccount : IAccount
{
    public SavingsAccount(int accountNumber)
    {

    }

    public void Freeze()
    {

    }
}
}

READING:

  1. When to use the CQRS design pattern?

  2. In domain-driven design, would it be a violation of DDD to put calls to other objects' repostiories in a domain object?

  3. Refactoring domain logic that accesses repositories in a legacy system

  4. Which of these examples represent correct use of DDD?

  5. Good Domain Driven Design samples

  6. Advantage of creating a generic repository vs. specific repository for each object?

Community
  • 1
  • 1
LCJ
  • 22,196
  • 67
  • 260
  • 418
  • Where are the aggregate boundaries? – Dennis Traub Jun 19 '12 at 06:43
  • 1
    Scanning over the proposed code, it looks very *nouns*-heavy, as most traditional OO code. In his DDDD draft paper, Greg Young clearly explains why the stereotypical OO approach doesn't work with DDD: http://abdullin.com/storage/uploads/2010/04/2010-04-16_DDDD_Drafts_by_Greg_Young.pdf – Mark Seemann Jun 19 '12 at 12:10
  • 1
    Once you start thinking about DDD in terms of CQRS, you'll often find that domain objects aren't really 'the nouns of the system', but rather the Commands and Events that occur in the system. – Mark Seemann Jun 19 '12 at 12:11

4 Answers4

4

if your AccountManipulator is a Façade to your domain, I wouldn't put the account number in the constructor. I would refactor it this way:

public class AccountManipulator
{
    private  AccountFactory _factory;
    private UserRepository _users;

    public AccountManipulator(AccountFactory factory, UserRepository users)
    {
       _factory = factory;
       _users = users;
    }

    public void FreezeAccount(int accountNumber)
    {
       var acc = _factory.GetAccount(accountNumber);
       acc.Freeze();
    }

    public IEnumerable<IAccount> GetAccountsOf(User user) {
       return _users.GetAccountIds(user).Select(_factory.GetAccount);
    }
}

public interface UserRepository {
    IEnumerable<int> GetAccountIds(User user);
}

In order to state if your domain is SOLID, you should analyze it with the principle:

  • Single Responsibility: every object has is own responsibility (and only that one):
    • AccountFactory: creates IAccounts
    • SavingsAccount: implementation of IAccount that reads from/writes to (a database? a web service?)
    • AccountManipulator: provide a minimal and simple set of operations to do with domain objects.
  • Open/Closed: are you classes are open to extensions and closed to changes?
    • AccountFactory: well, no. If you write a new implementation of IAccount, in order to use it you have to change AccountFactory. Solution: abstract factory
    • SavingsAccount? It depends if it will use external dependencies. Need more code to say.
    • AccountManipulator: yes. If you need to do another operation with your domain objects, you can use directly the other services without change AccountManipulator. Or you can inherit from it
  • Liskov substitution: can you substitute any class with another implementation? Need more code to say. You have no other implementations of IAccount or IAccountFactory now
  • Dependency Inversion:
    • AccountManipulator should depend on abstractions: AccountFactory and UserRepository should be interfaces.
onof
  • 17,167
  • 7
  • 49
  • 85
  • 1
    Don't make the AccountManipulator to be a web service, rather make the web service to use it. You should decouple the web service signature from the facade. – onof Jun 19 '12 at 07:17
  • 1
    The web service could use a DependencyInjection container to build up AccountManipulator. The client of AccountManipulator is the web service. – onof Jun 19 '12 at 07:32
  • Thanks. Could you please provide some good and easy article reference for "DependencyInjection container to build up AccountManipulator"? – LCJ Jun 19 '12 at 08:31
  • 1
    If your web service is made with WCF you can use Castle and the WCF facility, you can start reading here: http://stackoverflow.com/questions/10795365/castle-wcf-facility-container-configuration – onof Jun 19 '12 at 09:14
3

Firstly, to really answer your question it's important to know why you need to get all user accounts? Are you:

  1. Fetching a list of accounts to display on screen for the user to then perform a command/transaction against a single account?
  2. Performing a single command/transaction on all of the users accounts - such as 'Freeze All User Accounts'?

The reason I ask is because you only need to consider the DDD aspect if it's the latter. If the reason for this 'functionality' is the former (and after reading your question I suspect it is) - I really recommend just creating a thin query service layer that gets the user's account data you need for the screen. You don't need to add the 'restrictions' of DDD for this; there are no transactions or model state changes involved. Providing this functionality doesn't have to involve the domain model at all. Just define some simple POCO DTO's and use Entity Framework to get the data and pass it back to the UI.

This is what CQRS is about; you don't need repositories, factories or aggregates to give the UI a list of accounts for the user to choose from - you would be over complicating it and making A LOT more work for yourself.

If there is a scenario that requires a single transaction over all of the user's accounts then I'd do something like:

public class AccountService : IAccountService
{
    private IAccountRepository _accountRespository;

    public void FreezeAllAccountsForUser(Guid userId)
    {
        IEnumerable<IAccount> accounts = _accountRespository.GetAccountsByUserId(userId);

        using (IUnitOfWork unitOfWork = UnitOfWorkFactory.Create())
        {
            foreach (IAccount account in _accounts)
            {
                account.Freeze();
                _accountRespository.Save(account);
            }
        }
    }
}

Where AccountService is a webservice, i.e. the Application Layer.

In summary, my advice is: Only consider DDD in the context of commands that require transactions. For fetching lists of data; create a simple query service that the UI can consume.

P.S. I've noticed the misuse of the Factory pattern in your question and some of the answers. Factories are designed to provide an object's CREATION strategy, given particular data. There shouldn't be a 'GetAccount(accountId)' method that calls the database; repositories call the database then pass data to a factory to create the object.

David Masters
  • 8,069
  • 2
  • 44
  • 75
  • No, the account service is an Application Service (see http://weblogs.asp.net/pgielens/archive/2006/05/31/Applying-the-Application-Layer-in-Domain-Driven-Design.aspx). If your entities do not have methods on them, then you are **definitely not using DDD** at all; and no, the save should not be part of the Freeze method. To be honest, reading your comment above I can't help but feel you haven't researched DDD enough; have you read the blue book? – David Masters Jun 27 '12 at 08:09
  • 2
    Blue book -> http://www.amazon.com/Domain-Driven-Design-Tackling-Complexity-Software/dp/0321125215 – Chris Moutray Jun 27 '12 at 08:14
1

First of all why do you need AccountManipulator? It does absolutely nothing, but makes the code more complicated.

As for getting all accounts of a User, the most logical place to put this method would be in the User class. You could pass an account factory to that method, further implementation would probably depend on how you store accounts.

Jevgenij Evll
  • 1,892
  • 18
  • 21
0

I'd rename 'AccountFactory' to AccountRepository and put an extra method in it GetAccountsForUser( int userId ) which retrieves all Accounts for a specific user.

If AccountManipulator is a webservice, then this class will use the AccountRepository, like this:

public class AccountManipulator
{

   public void FreezeAccount( int accountNr )
   {
        var repository = new AccountRepository();
        var account = repository.GetAccount(accountNr);
        account.Freeze();
        repository.Save(account);        
   }

   public ICollection<Account> GetAccountsForUser( int userId )
   {
        var repository = new AccountRepository();
        return repository.GetAccountsForUser (userId);
   }

}
Frederik Gheysels
  • 56,135
  • 11
  • 101
  • 154