Lets Get Small: Why I Dislike the Module Include Pattern
Like many others in the Ruby community I’ve read and been inspired by Sandi Metz’s great book Practical Object-Oriented Design in Ruby. If you haven’t read it yet go out an buy it (right after you finish this article :) There is a lot of information in there but the simple mantra I’ve taken from it is: Smaller is better than Bigger.
- Small classes
- Small methods
- Small number of responsibilities (single responsibility principle)
- Small files
- Small number of dependencies
- Small everything
Let’s Get Small - I try to keep repeating this mantra as I code and find that it results in better, more testable and easier to understand code.
We’re going to look at an example that tries to DRY up some code by extracting it into a module that gets included in different classes. Even though it looks like this shrinks the classes we’ll see that it actually doesn’t and is a pattern to be avoided.
Let’s imagine we need to record certain events in an audit log. Our application is running on many different places and the audit log lives on a remote server and we post our events using an http api.
Simplest straw-man solution
The simplest solution is to just put the HTTP post code in each of our classes.
1 class MyClass 2 def do_something 3 HTTP.post 'http://api.example.com/audit/log', event: 'I did something' 4 5 # actually do something 6 end 7 end
Reading this you can see the audit trail code on line 3 but its not really clear that that is recording to an audit log. Additionally a real implementation with error handling and additional logic would almost certainly be more than a single line causing a lot of duplication once we copy-and-paste into another class.
We need an
AuditTrail thing to DRY up the code and make the dependency more explicit.
The module include anti-pattern
Our first refactor is to create an
AuditTrail module and include it in many different classes. I’ve seen this often (heck I’ve written it a lot) but my thinking has evolved and I now avoid it because it violates the keep it small rule.
1 class MyClass 2 include AuditTrail 3 4 def do_something 5 record_in_audit_trail 'I did something' 6 7 # actually do something 8 end 9 end
You can see on line 2 we include the module and on line 5 we
The included module look like this:
1 module AuditTrail 2 def record_in_audit_trail(message) 3 HTTP.post 'http://api.example.com/audit/log', event: message 4 end 5 end
The good things about this refactor are
- We now have an AuditTrail module that can be adapt as the implementation grows to more than a single line.
- Other classes besides
MyClasscan also use the
So what’s wrong with this design? Well, by including the AuditTrail:
- We increase the size of each of our classes. Each now responds to its methods and all the methods of the AuditTrail (I know its only 1 in this simple example).
- We had to adopt a naming convention
record_in_audit_trailthat hints at where the method is defined. This makes line 5 of
MyClasseasier to read but makes line 2 of
AuditTrailmore confusing and harder to read.
We’ve decreased the size of each class’ file and extracted common code (both good things) but we haven’t really made the surface area of the class smaller and the dependency on
AuditTrail is explicit on line 2 but hidden on line 5.
The next refactor solves these problems to keep the surface area of each class small and also make explicit that AuditTrail stuff is the responsibility of the AuditTrail?
Make AuditTrail a class
Since Ruby is all about classes and sending messages between objects so let’s use create an
AuditTrail class. Now our
MyClass looks like this
1 class MyClass 2 def do_something 3 AuditTrail.record 'I did something' 4 5 # actually do something 6 end 7 end
Its really easy to see on line 3 where it tells the AuditTrail to record the event and the rest of the code is 100% related to its responsibility (doing something).
AuditTrail side we simply change it from a module to a class and rename its method to
1 class AuditTrail 2 def self.record(message) 3 HTTP.post 'http://api.example.com/audit/log', event: message 4 end 5 end
Since it is a class we also have the opportunity to extend it as requirements change perhaps using resque or something else so we do our processing in the background.
1 class AuditTrail 2 attr_reader :message 3 def initialize(message) 4 @message = message 5 end 6 7 def self.record(message) 8 new(message).send 9 end 10 11 def send 12 HTTP.post url, event: message 13 end 14 15 def url 16 'http://api.example.com/audit/log' 17 end 18 end
What are the advantages of this approach?
- Each class clearly defines its single responsibility without any pollution from the AuditTrail responsibility.
- The classes are easier to read as their dependency on AuditTrail is explicit as
AuditTrail.add(I believe Sandi Metz would argue that this dependency should be extracted through dependency injection but I’m still struggling with that idea)
- The AuditTrail class also defines it’s responibility and is free to name its method
In summary by using a class instead of including a module we keep it small and smaller is better.