1

I have an existing bank application classes as shown below. The banks account can be of SavingsBankAccount or FixedBankAccount. There is an operation called IssueLumpSumInterest. For FixedBankAccount, the balance need to be updated only if the owner of the account has no other account.

This demands the FixedBankAccount object to know about other accounts of the account owner. How to do this by following SOLID/DDD/GRASP/Information Expert pattern?

namespace ApplicationServiceForBank
{

public class BankAccountService
{
    RepositoryLayer.IRepository<RepositoryLayer.BankAccount> accountRepository;
    ApplicationServiceForBank.IBankAccountFactory bankFactory;

    public BankAccountService(RepositoryLayer.IRepository<RepositoryLayer.BankAccount> repo, IBankAccountFactory bankFact)
    {
        accountRepository = repo;
        bankFactory = bankFact;
    }

    public void IssueLumpSumInterest(int acccountID)
    {
        RepositoryLayer.BankAccount oneOfRepositroyAccounts = accountRepository.FindByID(p => p.BankAccountID == acccountID);

        int ownerID = (int) oneOfRepositroyAccounts.AccountOwnerID;
        IEnumerable<RepositoryLayer.BankAccount> accountsForUser = accountRepository.FindAll(p => p.BankUser.UserID == ownerID);

        DomainObjectsForBank.IBankAccount domainBankAccountObj = bankFactory.CreateAccount(oneOfRepositroyAccounts);

        if (domainBankAccountObj != null)
        {
            domainBankAccountObj.BankAccountID = oneOfRepositroyAccounts.BankAccountID;
            domainBankAccountObj.AddInterest();

            this.accountRepository.UpdateChangesByAttach(oneOfRepositroyAccounts);
            //oneOfRepositroyAccounts.Balance = domainBankAccountObj.Balance;
            this.accountRepository.SubmitChanges();
        }
    }
}

public interface IBankAccountFactory
{
    DomainObjectsForBank.IBankAccount CreateAccount(RepositoryLayer.BankAccount repositroyAccount);
}

public class MySimpleBankAccountFactory : IBankAccountFactory
{
    public DomainObjectsForBank.IBankAccount CreateAccount(RepositoryLayer.BankAccount repositroyAccount)
    {
        DomainObjectsForBank.IBankAccount acc = null;

        if (String.Equals(repositroyAccount.AccountType, "Fixed"))
        {
            acc = new DomainObjectsForBank.FixedBankAccount();
        }

        if (String.Equals(repositroyAccount.AccountType, "Savings"))
        {
            //acc = new DomainObjectsForBank.SavingsBankAccount();
        }

        return acc;
    }
}

}

namespace DomainObjectsForBank
{

public interface IBankAccount
{
    int BankAccountID { get; set; }
    double Balance { get; set; }
    string AccountStatus { get; set; }
    void FreezeAccount();
    void AddInterest();
}

public class FixedBankAccount : IBankAccount
{
    public int BankAccountID { get; set; }
    public string AccountStatus { get; set; }
    public double Balance { get; set; }

    public void FreezeAccount()
    {
        AccountStatus = "Frozen";
    }

    public void AddInterest()
    {
        //TO DO: Balance need to be updated only if the person has no other accounts.
        Balance = Balance + (Balance * 0.1);
    }
}

}

READING

  1. Issue in using Composition for “is – a “ relationship

  2. Implementing Business Logic (LINQ to SQL) http://msdn.microsoft.com/en-us/library/bb882671.aspx

  3. Architecting LINQ to SQL applications

  4. Exploring N-Tier Architecture with LINQ to SQL http://randolphcabral.wordpress.com/2008/05/08/exploring-n-tier-architecture-with-linq-to-sql-part-3-of-n/

  5. Confusion between DTOs (linq2sql) and Class objects!

  6. Domain Driven Design (Linq to SQL) - How do you delete parts of an aggregate?

Community
  • 1
  • 1
LCJ
  • 22,196
  • 67
  • 260
  • 418
  • you have frequently misspelled "repository" as "repositroy" – McKay Jun 28 '12 at 13:49
  • Hi @Lijo - you asked me to answer this question. SonOfPirate has done a good job and there is nothing more that I would add to his/her answer. Hope this helps... – RobertMS Jun 28 '12 at 13:53
  • 1
    Its weird that you have repository objects and domain objects - should your repository not just return the domain objects? – Chris Moutray Jun 28 '12 at 14:36
  • @mouters. I am new to repository pattern and LINQ to SQL. So, when LINQ to SQL generate the BankAccount entity, how should I make them as SavingsBankAccount and FixedBankAccount? Can you please help with code/ example/ reference? – LCJ Jun 29 '12 at 06:40

4 Answers4

2

The first thing I noticed was the improper use of the bank account factory. The factory, pretty much as you have it, should be used by the repository to create the instance based on the data retrieved from the data store. As such, your call to accountRepository.FindByID will return either a FixedBankAccount or SavingsBankAccount object depending on the AccountType returned from the data store.

If the interest only applies to FixedBankAccount instances, then you can perform a type check to ensure you are working with the correct account type.

public void IssueLumpSumInterest(int accountId)
{
    var account = _accountRepository.FindById(accountId) as FixedBankAccount;

    if (account == null)
    {
        throw new InvalidOperationException("Cannot add interest to Savings account.");
    }

    var ownerId = account.OwnerId;

    if (_accountRepository.Any(a => (a.BankUser.UserId == ownerId) && (a.AccountId != accountId)))
    {
        throw new InvalidOperationException("Cannot add interest when user own multiple accounts.");
    }

    account.AddInterest();

    // Persist the changes
}

NOTE: FindById should only accept the ID parameter and not a lambda/Func. You've indicated by the name "FindById" how the search will be performed. The fact that the 'accountId' value is compared to the BankAccountId property is an implementation detail hidden within the method. Name the method "FindBy" if you want a generic approach that uses a lambda.

I would also NOT put AddInterest on the IBankAccount interface if all implementations do not support that behavior. Consider a separate IInterestEarningBankAccount interface that exposes the AddInterest method. I would also consider using that interface instead of FixedBankAccount in the above code to make the code easier to maintain and extend should you add another account type in the future that supports this behavior.

SonOfPirate
  • 5,642
  • 3
  • 41
  • 97
  • Thanks... Question regarding if (_accountRepository.Any(a => (a.BankUser.UserId == ownerId) && (a.AccountId != accountId))). When we do this, the BankAccountService object does the business logic handling. Is there any other method to overcome this? – LCJ Jun 28 '12 at 15:52
  • 1
    Personally, I think this is appropriate given that your service is conducting multiple accounts (in this case). This is exactly what a Domain Service should do. Your domain object should not contain this logic because it is a higher-level requirement that requires knowledge of multiple domain objects. Plus, the only relationship the account in question has with the others is the owner which may lead you to think about adding an Owner aggregate root but I don't think that would be appropriate in this context as the Owner really has nothing to do with interest rules. This is how I'd do it. – SonOfPirate Jun 28 '12 at 16:45
  • Thanks. I understand the rationale for keeping the logic in Application Service considering the Owner aggregate root. However, can you please give a reference where adding an aggregate root will be suitable and how to implement it? If you can update the answer with code, there is nothing like that. – LCJ Jun 29 '12 at 06:17
  • 1
    The rationale behind an aggregate root is when you have objects that don't/can't/shouldn't exist outside the context of the root. In the classic example of an Order aggregate with OrderLine child objects, it doesn't make sense to have an OrderLine outside the context of the parent Order. That's not the case in your scenario. However, you may find it makes sense to model a Transaction as a child object of the BankAccount root. – SonOfPirate Jun 29 '12 at 11:35
2

From reading your requirement, here is how I would do it:

//Application Service - consumed by UI
public class AccountService : IAccountService
{
    private readonly IAccountRepository _accountRepository;
    private readonly ICustomerRepository _customerRepository;

    public ApplicationService(IAccountRepository accountRepository, ICustomerRepository customerRepository)
    {
        _accountRepository = accountRepository;
        _customerRepository = customerRepository;
    }

    public void IssueLumpSumInterestToAccount(Guid accountId)
    {
        using (IUnitOfWork unitOfWork = UnitOfWorkFactory.Create())
        {
            Account account = _accountRepository.GetById(accountId);
            Customer customer = _customerRepository.GetById(account.CustomerId);

            account.IssueLumpSumOfInterest(customer);

            _accountRepository.Save(account);
        }
    }
}

public class Customer
{
    private List<Guid> _accountIds;

    public IEnumerable<Guid> AccountIds
    {
        get { return _accountIds.AsReadOnly();}
    }
}

public abstract class Account
{
    public abstract void IssueLumpSumOfInterest(Customer customer);
}

public class FixedAccount : Account
{
    public override void  IssueLumpSumOfInterest(Customer customer)
    {
        if (customer.AccountIds.Any(id => id != this._accountId))
            throw new Exception("Lump Sum cannot be issued to fixed accounts where the customer has other accounts");

        //Code to issue interest here
    }
}   

public class SavingsAccount : Account
{
    public override void  IssueLumpSumOfInterest(Customer customer)
    {
        //Code to issue interest here
    }
}
  1. The IssueLumpSumOfInterest method on the Account aggregate requires the Customer aggregate to help decide whether interest should be issued.
  2. The customer aggregate contains a list of account IDs - NOT a list of account aggregates.
  3. The base class 'Account' has a polymorphic method - the FixedAccount checks that the customer doesn't have any other accounts - the SavingsAccount doesn't do this check.
David Masters
  • 8,069
  • 2
  • 44
  • 75
1

2 min scan answer..

  • Not sure why there is a need for 2 representations of a BankAccount RepositoryLayer.BankAccount and DomainObjectsForBank.IBankAccount. Hide the persistence layer coupled one.. deal with just the domain object in the service.
  • Do not pass/return Nulls - I think is good advice.
  • The finder methods look like the LINQ methods which select items from a list of collection. Your methods look like they want to get the first match and exit..in which case your parameters can be simple primitives (Ids) vs lambdas.

The general idea seems right. The service encapsulates the logic for this transaction - not the domain objects. If this changes, only one place to update.

public void IssueLumpSumInterest(int acccountID)
{
    var customerId = accountRepository.GetAccount(accountId).CustomerId;

    var accounts = accountRepository.GetAccountsForCustomer(customerId);
    if ((accounts.First() is FixedAccount) && accounts.Count() == 1)
    {
       // update interest    
    }
}
Gishu
  • 134,492
  • 47
  • 225
  • 308
0

Things that strike me as weird:

  • Your IBankAccount has a method FreezeAccount, but I presume that all accounts would have quite similar behavior? Perhaps a BankAccount class is warranted that implements some of the interface?
  • AccountStatus should probably be an enum? What should happen if an account is "Forzen"?
McKay
  • 12,334
  • 7
  • 53
  • 76