Refactoring Demo - Counting posts

I found a question somewhere and decided to expand in a bit longer fashion on it. Note: The code is largely working pseudocode. I won't check / run this code and parts may be missing to just illustrate the example.

In the first refactoring demo I found this Reddit link and question (screenshot below):

The other answers are okay but they expand on the user model which is often the biggest God model in the application. It's also not a very extensible solution. What if I want comments in the last 30 days or total number of comments for a post. Typically I see methods and scopes pile up on the models for each permutation the app needs. But, I think there is a better way.

Make a class

The number of comments over a duration is a distinct thing. It is separate than a user, or comment and thus deserves it's own class. Let's begin by creating the rough class template.

class DailyCommentCount
  self.for(user)
    new(user).count
  end
  
  def initialize(user)
    @user = user
  end
  
  def count
    user.comments.where('created_at >= ?', 24.hours.ago)
  end
  
  private 
  
  attr_reader :user
end

This looks nice. Now we can use it anywhere.

<%= DailyCommentCount.for(current_user) %>

Why is this better?

You gain flexibility. By putting the code on the user model how does it work for other items? And we know how reality is. You are not only going to need a number of comments in some places. You are going to want to change the duration period, the display format etc. Let's expand on this a little bit.

class CommentCount
  self.for(object)
    new(object: object).count
  end
  
  def initialize(object:, timeframe: 24.hours.ago)
    raise StandardError.new("") if [Comment, User].exclude?(object.class)
    @timeframe = timeframe
    @object = object
  end
  
  def count
    object.comments.where('created_at >= ?', timeframe)
  end
  
  private 
  
  attr_reader :object, :timeframe
end

With a minute or two of fiddling around, I expanded the capabilities of this class without breaking old functionality. I did rename it to reflect the broader scope but that is not required. This class now:

• Works for any object that has comments
• Can specify the timeframe

But.. alas, now we have some funky requirements and we need to format the output better.

class CommentCount
  self.for(object)
    new(object: object).count
  end
  
  def initialize(object:, timeframe: 24.hours.ago)
    raise StandardError.new("") if [Comment, User].exclude?(object.class)
    @timeframe = timeframe
    @object = object
  end
  
  def count
    object.comments.where('created_at >= ?', timeframe)
  end
  
  def written_count_long
    I18n.t(
      "#{object.class.name.downcase}.written_count",
      number_in_words: number_in_words,
      count: count
    )
  end
  
  def written_count
    I18n.t(
      "#{object.class.name.downcase}.written_count", 
      count: count
    )
  end
  
  private 
  
  attr_reader :object, :timeframe
  
  def number_in_words
     count.to_words #pseudo ish... hopefully there is a class for this!
  end
end

Testing is a joy

This is now super easy to test. With the other way you need to render your views most likely to get a valid test of all the requirements. But now that it's in a tiny encapsulated class it becomes dead simple. I put the test outline below for rspec.

require "rails_helper";

describe CommentCount do 
  describe ".for" do
    it "should instantiate a new class and call count" do
      comment_count = CommentCount.new(object: mocked_user)
      expect(comment_count).to receive(:count)
      expect(CommentCount).to receive(:for).and_return(comment_count)
      
      CommentCount.for(mocked_user)
    end
  end
  
  describe ".new" do
    it "should raise an error if not a valid model" do
    end
  end
  
  describe "#count" do
    it "should default to the count in the last day" do
    end
    
    it "should be able to adjust the timeframe" do
    end
    
    it "should work with models that respond to comments" do
    end
  end
  
  describe "#written_count" do
  	it "should return the count as an integer and 'comments'" do
    end
  end
  
  describe "written_count_long" do
	it "should return the count as text and 'comments'" do
    end
  end
end

Getting nit-picky

I think this code has a bug after looking at what I just wrote. The code is fine. It works. The tests pass but still something is a bit off. When writing the test I wrote the requirement that "it should work with models that respond to comments". However, the code doesn't check that. It checks to ensure the model is a comment or a user.

This is a case where the described functionality of the class does not do what we say it does. In the future this could cause bugs. The team may operate under the assumption that it works with any object that responds to comments. The code however does not work that way.

Happy coding! Until next time.