This site provides the following access keys:

Brandan Lennox's

Refactoring with (Maybe) Null Object

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.