Sometimes it’s nice to see how people are doing things in real codebases. So here is another snippet from a real application I’m working on.
I had a class that generated a PDF report with the PRAWN gem that looked at data over a 5 year period. I was asked to create another report that was similar in design but only looked at a single year of data.
How I Went About This
I first reviewed the original class to see what would have to change to create the new report. I was trying to see if I could make the changes within the class or at least how to reuse the majority of the code as the reports are so similar.
Quickly I knew a new class would be needed for the new report (separate responsibilities and all). I also found that the original PDF class would need to be refactored, it was 773 lines and did way too much without letting me easily reuse anything.
I figured I could start breaking out the original PDF class into smaller classes that would be easier to maintain, test and reuse.
Below is one of the changes I made. This one method from the original PDF class seemed to do one thing, group some data into a specific structure for the report. It was ideal for extracting into it’s own class. It also had many areas within it that were ideal for extracting smaller methods. And, when done, this new class could be called from the new report.
This is one method from a 773 line class (including comments). Note: comments have been trimmed to keep the post shorter and I have changed various naming/terms.
Method as a Class
The new class does everything the big weird method (above) was doing.
I first extracted the method to a class with an initialize(project) and everything else became a single method called get_todos_grouped_by_year(project)
I then extracted new methods from the large single method
Areas with multiple levels of nesting I tried to break out into new methods
Areas that were hard to understand what was going on I broke out
I made sure the user could instantiate the class and only invoke call to get the results (e.g. todos = ToDosGroupedByYear.new(@project).call)
I Updated names of methods to try and make them easier to understand and reduce commenting
Why This is Better
I’ll keep improving this as I go but here are the main benefits:
The original PDF class does 1 less thing and closer to focusing on only generating the PDF
The new class ToDosGroupedByYear does exactly what it’s called
Smaller classes all-around
The new class is testable
Smaller methods in the new class which are easier to read and maintain
Enables/encrouages appropriate reuse of the class
The smaller chunks make it easier to see where additional refactoring is needed or wonky stuff is happening