# Report with Subtotals — In Search Of An Abstraction

## High Level Explanation of Code's Purpose

I wrote a class to create sectioned reports, where each section has a subtotal. It allows these reports to be nested within each other, to create multiple levels of subtotals. An example will help clarify the idea before getting to the real code....

Consider this sample data:

rows = [
{person: 'john'  , team: 'red'  , height: 73, age: 30} ,
{person: 'carl'  , team: 'red'  , height: 65, age: 23} ,
{person: 'sam'   , team: 'red'  , height: 74, age: 42} ,
{person: 'joe'   , team: 'red'  , height: 67, age: 29} ,
{person: 'bob'   , team: 'blue' , height: 77, age: 45} ,
{person: 'james' , team: 'blue' , height: 71, age: 26} ,
{person: 'danny' , team: 'blue' , height: 70, age: 26} ,
{person: 'billy' , team: 'blue' , height: 67, age: 27} ,
]


Here's a report showing subtotals by team, along with a grand total:

We may want to drill down further, and create subtotals within the team totals based on tall people (> 72 inches) and normal people. It might look like this:

## Example Client Code

To make the first report pictured above, we'd use the class I wrote, ReportWithSections, like this:

report = ReportWithSections.new(
rows: rows,
totals_cols: [:height, :age],
total_label_proc: ->(totals, *) {totals.update(:person => 'Grand Total')}
)

by_team = ->(totals) {totals[:team]}

team_label = ->(totals, report) do
team = report.last_row[:team]
totals.update(:person => "Team #{team} Total")
end

report = report.add_subtotals(subsection_proc: by_team, total_label_proc: team_label)

puts html_table(report.rows_with_totals)


Note that html_table is just a simple "view" function for the purposes of this post, and not part of my question, but for completeness it looks like this:

def html_table(rows)
row = ->(x) { "<tr>#{x}</tr>\n" }
cells = ->(r) {r.values.map  {|x| "<td>#{x}</td>"}.join}
all_tds = rows.map {|r| row.(cells.(r))}.join
end


Finally, to create the 2nd report pictured above, which includes height subtotals, we'd continue as follows:

by_height = ->(totals) {totals[:height] > 72}

height_label = ->(totals, report) do
height = report.last_row[:height] > 72 ? 'Tall' : 'Normal'
totals.update(:person => "#{height} Total")
end

report = report.add_subtotals(subsection_proc: by_height, total_label_proc: height_label)

puts html_table(report.rows_with_totals)


So, we can specify which columns we want to total, and we can dynamically create the labels for the subtotals however we want, and put those labels in whatever column we want, although in this example we choose to place all subtotal labels in the first column.

## The Code Itself

The class below is my current implementation. I am fairly happy with it. The methods are short and clearly named I think, indendation is kept to one level for the most part, and the public API is small. There is one thing that's not sitting with well with me, yet I can't figure out a way around it. This is what I'd like suggestions on, although other suggestions are welcome too. I'll put my thoughts after the code, in case you want to form your opinion by reading the code without bias.

class ReportWithSections

attr_accessor :all_cols, :totals_cols, :totals

# rows can be [{}] or [ReportWithSections]
def initialize(rows:, totals_cols:, total_label_proc: nil)
@rows, @totals_cols, @total_label_proc = rows, totals_cols, total_label_proc
@all_cols = nested? ? @rows.first.all_cols : @rows.first.keys
@totals = make_totals_row
@totals = total_label_proc.(@totals, self) if total_label_proc
end

def rows_with_totals
flatten_with_totals + [@totals]
end

def rows_without_totals
flatten_without_totals
end

def last_row
rows_without_totals.last
end

send(method, subsection_proc: subsection_proc,
total_label_proc: total_label_proc)
end

private

def clone_with(rows:, total_label_proc: nil)
label_proc = total_label_proc || @total_label_proc
ReportWithSections.new(rows: rows, totals_cols: @totals_cols,
total_label_proc: label_proc)
end

subsections = @rows.group_by(&subsection_proc)
return self if subsections.size == 1

nested_reports = subsections.map do |chunk_elm|
clone_with(rows: chunk_elm.last, total_label_proc: total_label_proc)
end
clone_with(rows: nested_reports)
end

rows = @rows.map {|r| r.add_subtotals(subsection_proc: subsection_proc, total_label_proc: total_label_proc)}
clone_with(rows: rows)
end

def flatten_with_totals
return @rows unless nested?
@rows.map {|r| r.rows_with_totals}.flatten
end

def flatten_without_totals
return @rows unless nested?
@rows.map {|r| r.rows_without_totals}.flatten
end

def make_totals_row
nested? ? sum_rows(nested_totals) : sum_rows(@rows)
end

def sum_rows(rows)
rows.each_with_object(zeroed_totals) do |row, total|
@totals_cols.each {|c| total[c] += row[c]}
end
end

def nested_totals
@rows.map {|r| r.totals}
end

def totals_col?(col)
@totals_cols.include? col
end

def zeroed_totals
@all_cols.map {|c| [c, totals_col?(c) ? 0 : nil]}.to_h
end

def nested?
@rows.first.is_a? ReportWithSections
end

end


What is not sitting with well with me is that is_a? ReportWithSections inside of nested? -- those calls to nested? are really switch statements on type. And switching on type smells of an incorrect, or missing, abstraction. And yet, I see no way around it, since the rows of hashes are the base case of this recursive structure, and seem to need different treatment. What's worse, if I decided to provide access to the "rows" (of hashes or of ReportWithSections) and totals separately, to support views that, eg, wanted to put the totals row above the rows it sums, then this type switching would leak out into the view.

Is there a better approach to this kind of nested structure that avoids these problems?

• As you said, there seems to be an improper abstraction, you probably should have separate classes to generate report and to hold data, what would allow you to convert those hashes to something more usefull before further operation. Instead of using polimorphism you pass behaviour around in weird ways, what makes your code pretty hard to follow, even if it looks quite nice aprat from it. I'll add more detailed answer later. – Borsunho Oct 19 '15 at 9:33
• Thanks, just FYI using dependency injection rather than inheritance is a very conscious choice. I pretty much think inheritance is an anti-pattern but that is a whole other long subject not appropriate for here. however if you believe you have a good solution which uses it I am open to seeing it – Jonah Oct 19 '15 at 13:09

Code smell you noticed is called fake polimorphism, and - as name indicates - it's easily removed by introducing real polimorphism, thus making code compliant to tell, don't ask rule.

In your case, you could create an "unnested" report class, convert your rows of data to it, and split implementation of each method that needs to check for nested between it and ReportWithSection:

def SimpleReport
def initialize data # or 'rows'
@data = data
end

def flatten_with_totals
self # note that we do not return @data hash - this class abstracts the hash!
end

end

def totals
sum_rows(@rows)
end
# ... and so on ...
end


If you end up with inheritance that bothers you that way, refactor it into composition, but I don't see any sane reason to do so - Ruby modules are flexible enough to suffice in most cases, and, after all, those classes were one in your code, so adding SimpleReport loosens coupling anyways ;)

Passing @totals to total_label proc smells with exposing implementation details. Passing self smells with Python. It would look much nicer if you passed result of grouping proc (key that groups record) and returned appropiate label label:

by_height = ->(record) {record[:height] > 72}

height_label = ->(are_tall) do
are_tall ? 'Tall Total' : 'Normal Total'
# no update, your class does that
end


Now this is less flexible, but easier to refactor and produces nicer interface.

Finally, because your class holds both data and lambdas describing the report, it forces user to manually handle each step of generating the report, so interface is tiresome to use and - unless I missed something - there is no easy way to generate the same report for different sets of data. This doesn't look like client code at all. Consider adding a 'facade' class that would gather all those lambdas, and than use your class to actually generate report, i.e.

gen = ReportGenerator total_cols: [:height, :age],
total_label_proc: ->{ ... }


• Funny, I had a class SimpleReport at first, then did away with it, but I think you're right -- it will fix this. I also agree I should have a facade class to make client usage more friendly, but that's more of a cosmetic problem, easily fixable. Your suggestion for removing @totals from the label proc, while agree with it in spirit, damages key functionality. You can only use static labels that way. See "Team red total" and "Team blue total" in screens in my OP. Can you think of a cleaner solution that preserves this ability? – Jonah Oct 21 '15 at 21:33
• My solution preserves this ability, unless I misunderstood you, i.e. ->(team){"Team #{team} Total"}. – Borsunho Oct 22 '15 at 3:12
• I think I misunderstood you. You were suggesting that we simply always use the first column to hold the total, and thus that logic can live inside ReportWithSections and doesn't have to be passed in. Likewise we always pass in the "group by" parameter, and that's the only thing the label proc can use. I think it's a good idea that will work for 90% of cases, even thought it's a bit less flexible. – Jonah Oct 22 '15 at 3:26