I’m not sure exactly which design pattern this is, but I found it useful today.
I had a class that performed a query and returned the results in JSON format for a charting library. The query grouped by one column (column), calculated the average of another column ('metric'), counted the number of records in each group, and did a little formatting of the values before returning the JSON. This is an approximation of what it looked like (lots of other logic omitted for brevity):
ChartData = Struct.new(:scope, :column) do
def as_json(options = nil)
values = results.map do |record|
{ x: "#{record.name} (#{record.count})", y: record.score }
end
[{ key: column, values: values }]
end
private
def results
scope
.group(group)
.select(group.as('name'), count, score)
end
def group
scope.arel_table[column]
end
def count
Arel::Nodes::NamedFunction.new('count', [Arel.sql('*')]).as('count')
end
def score
Arel::Nodes::NamedFunction.new('avg', ['metric']).as('score')
end
end
This worked great. Pretty easy to test, and flexible enough to work with any table that had the appropriate columns.
Then we got a new requirement. We needed to be able to perform the “count” portion of the query on a table that didn’t have the appropriate columns (i.e., it lacked a “metric”), and use the count rather than the score as the Y value of the chart.
At first, it seemed like a matter of reformatting the JSON, but that wasn’t sufficient. The query itself had to change, and that was buried in a private method. It was a big enough change to be painful, but small enough that it didn’t feel like a whole new class.
I first tried passing a new include_scores boolean to the class:
ChartData = Struct.new(:scope, :column, :include_scores) do
def as_json(options = nil)
values = results.map do |record|
if include_scores
{ x: "#{record.name} (#{record.count})", y: record.score }
else
{ x: record.name, y: record.count }
end
end
[{ key: column, values: values }]
end
private
def results
if include_scores
scope
.group(group)
.select(group.as('name'), count, score)
else
scope
.group(group)
.select(group.as('name'), count)
end
end
# ...
end
That worked, but it was ugly. The real query was more complex than this, and I didn’t like duplicating all that code. Likewise, the JSON wasn’t two slightly different representations of the same data. It was really two different datas being represented as JSON.
After some more refactoring, I ended up with this:
ChartData = Struct.new(:scope, :column) do
def scores
data_points_with_score(average_score) do |record|
{ x: "#{record.name} (#{record.count})", y: record.score }
end
end
def counts
data_points_with_score(null_score) do |record|
{ x: record.name, y: record.count }
end
end
private
def data_points_with_score(score)
values = scope
.group(group)
.select(group.as('name'), count, score)
.map do |record|
yield record
end
[{ key: column, values: values }]
end
def average_score
Arel::Nodes::NamedFunction.new('avg', ['metric']).as('score')
end
def null_score
Arel.sql('NULL').as('score')
end
# ...
end
I replaced the single as_json method with two methods, one for each JSON format that I needed to support. I also replaced the single score method with average_score (the actual score calculation) and null_score (effectively a no-op). Since the caller was now responsible for doing the “switching” — i.e., either calling scores or counts — I could declare the scoring method and the JSON format in the same place, and nothing else in the class needed to know about it.
I can see an argument that this is still violating Single Responsibility Principle. Maybe if there were one more case to handle, I would split this into multiple classes. For now, this remains easy to test, and the exceptional behavior is grouped together, so I call it a win.
