Collection calculating too much?

Question

Let say I've abstracted away a data structure such as a list; should this new object perform all the calculations or just provide minimal calculation functions and then allow a new object such as ReportGenerator to perform more large-scale calculations? Example below:

THIS?

class WrappedCollection
{
  public double CalculateByYear(int year) {...}

  public double CalculateCumulativeByYear(int year) {...}

  public double CalculateTotal() {...}

  private IList<double> collection = new List<double>();
}

OR THIS?

class WrappedCalculation
{
  public double CalculateByYear(int year) {...}

  public double CalculateTotal() {...}

  private IList<double> collection = new List<double>();
}

class ReportGenerator
{
  public ReportGenerator(WrappedCollection collection) {...}

  public double TotalByYear(int year) {...}
  public double CumulativeTotalByYear(int year) {...}
  public double Total() {...}

  private WrappedCollection collection;
}

This is a greatly simplified version; however, I'm starting to notice that in my WrappedCollection I am needing more methods and I feel as if I'm violating the SRP. The WrappedCollection class should just manage what I want (e.g. someone's retirement portfolio), and the reporting should left to another class that does all the calculations.

I seem to remember an example from Uncle Bob Martin or Martin Fowler that showed pushing all the data gathering further down the hierarchy into the collections themselves. This seems to generate more than 1 responsibility.

The project is comparing retirement portfolios; so I don't believe someone's 401k should give me all the metrics like cumulative contributions, etc. It should just give me it's current total, and maybe a contribution for a given year based on type (e.g. employer vs employee contribution). Another class can them compile a list of the cumulative contributions. Yes?

Answer 1

Your portfolio class violates the Single Responsibility Principle because it has methods for performing tasks from two largely orthogonal groups - namely, maintaining portfolio's content, and computing reporting metrics based on that content.

Separating out the two the way you did provides an improvement, but you could go even further by giving WrappedCalculation an interface, say, IWrappedCalculation, and coding up ReportGenerator in terms of that interface. This way you would be able to reuse the report generator that you wrote for retirement portfolios to produce reports for portfolios of other kind - say, non-retirement portfolios, or combinations of several portfolios.

Answer 2

Yet another developer torpedoed by the SOLID principles.

Responsibility doesn't mean "do only one thing." It means "have only one reason to change," or more specifically, "This is the place to go to make modifications for this area of concern." Having more methods on your class doesn't necessarily mean you're violating SRP.

A repository doesn't have four responsibilities because it has Create, Read, Update and Delete methods. It has only one: data access.

The SOLID principles exist to suggest ways to improve your software's maintainability. It is maintainability you should be striving for, not slavish adherence to arbitrary principles.