2

I'm implementing a feature in legacy system right now. Instead of putting all my logic in some dummy application services and having anemic models, I thought I will try to do some OOP this time.

I'm not using DDD, but trying to follow DDD-like-style a little bit.

So imagine a situation where you have:

public class CustomerAccount { private CustomerAccountId customerAccountId; Collection<AccountEntry> accountHistory; (or maybe some AccountHistory class) }

now, our Account has some domain logic. It should be able to calculate total debit, credit and averageBalance within some time period.

So it would be like following

public Money calculateTotalDebit(Date start, Date end) // or maybe some TimeInterval, it doesn't matter public Money calculateTotalCredit(Date start, Date end) public Money calculateAverageBalance(Date start, Date end, FancyCalculationStrategy strategy)

It also has some constraint that should be preserved while invoking other operations like addAccountEntry(AccountEntry entry)

So I can assume, that my Account is something like DDD's aggregate root. Obviously, it's not real aggregate root, because I'm not applying DDD, but let's just say I see some similarity.

And now my Account class grows - there's quite a lot logic in each of those methods. How is this related to Single Responsibility Principle? A class should have only one reason to change, but in this case, there are more. Or maybe not? Somehow I feel all this logic should belong to one place.

Yeah, I can create AccountHistory class, which encapsulates the behaviour, but this doesn't solve the problem - in that case, AccountHistory class will start to grow.

What I can do, is just to create some, let's say

AverageBalanceCalculator

and delegate the behaviour.

But isn't it going towards anemic model again? This calculator will probably have no state and it will behave just like a service.

Of course it doesn't have to be visible to end-user - from the implementation point of view, it could be a package private class.

How is SRP related to aggregates root and other DDD's tactical patterns in general?

I'm little confused, would appreciate some answers.

Improve this question
1
  • You don't need to force behavior inside an object to keep it from being anemic. You keep a class healthy (as in, non-anemic) by giving it the methods it needs. If it doesn't need any, good! Some data containers are just that, data containers. Forcing stuff inside a class for it to have a meaty behavior is as bad practice as putting that needed behavior somewhere unrelated. Commented Jun 2, 2017 at 19:55

2 Answers 2

Reset to default
2

While there is some logic in having the methods that do some operations with bill history in the CustomerAccount (or Bill) class, one might argue that having them there breaks the SRP. You could say that CustomerAccount class should be responsible only for maintaining the account data (adding, deleting or modifying that data). Reporting could be seen as a different cup of tea there. So, what you could do is resort to, say, a command pattern. You could do something like this:

interface IAccountReport { public Money CalculateReport(Collection<AccountEntry> accountHistory); } class CalculateAverageBalance : IAccountReport { private DateTime start; private DateTime end; public CalculateAverageBalance(DateTime start, DateTime end) : base() { this.start = start; this.end = end; } public Money CalculateReport(Collection<AccountEntry> accountHistory) { //Implement logic for calculating average balance using the start //and end date, as well as accountHistory return new Money(); } } class CustomerAccount { private Collection<AccountEntry> accountHistory; public void ProvideReport (IAccountReport report) { report.CalculateReport(accountHistory); } }

So, for each new type of report, you would implement a new class, et voilà, you have separated responsibilities. Of course, if the privacy of the account history is mandatory, then you could introduce some permission handling in terms of which reports can be run against which accounts and by whom, but that can all be fitted in this kind of a design.

Improve this answer
2
  • And this is different from an anemic data model how? Commented Jul 4, 2016 at 12:43
  • @Cerad It is not, really, but it was not the point of my answer to make it different. As a matter of fact, I just advocated that approach, which I understood was the dilemma. Bottom line is, if you are shooting for a non-anemic model, you can kiss SRP good-bye. Now, that is not necessarily a bad thing. However, if you have a problem with a class growing too much, then you have to refactor, which would in all likeliness move you towards anemic model. Commented Jul 4, 2016 at 13:13
1

You're overthinking this. If your class is:

public class Bill { private BillId billId; Collection<BillEntry> billHistory; }

Then your Average Balance method properly belongs in the Bill class.

public class Bill { private BillId billId; Collection<BillEntry> billHistory; public decimal GetAverageBalance() { return billHistory.Sum(x => x.Balance); } }

However, you should think carefully about how you are modeling this. Normally, average balances are calculated on a Customer Account object, not a bill.

Improve this answer
9
  • This is how I've implemented it so far. But as I wrote in my question, the Bill/CustomerAccount/WhateverYouCallIt started to grow. Commented Dec 15, 2014 at 16:57
  • What do you mean by "started to grow?" Commented Dec 15, 2014 at 16:57
  • 2
    I think you should trust your own judgement. If the class is becoming difficult to maintain, then find a way to conceptualize some of its behavior into a single theme, and put it in a new class. But 200 lines of code is not a lot, unless you're stuffing functionality into Bill that doesn't need to be there. Commented Dec 15, 2014 at 17:20
  • 1
    @RobertHarvey Assuming GetAverageBalance doesn't need to access private state, why does it "properly belong" to the Bill class? Adopting that point of view leads to the Bill class being modified every time you find a thing you want to do with it. Imagine if it were used by two different projects - you'd end up with both projects' helpers. To me that's anti-modular and a violation of the Open/Closed Principle - you shouldn't have to modify an existing module every time you find a new way to use it. Commented Dec 15, 2014 at 19:07
  • 1
    @Doval: Did you see my code example? It accesses private state, but it does so read-only. This isn't a helper object; it's the actual business object. The GetAverageBalance calculation isn't likely to change from project to project. Commented Dec 15, 2014 at 19:21

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.