In todays thoughtbot Boston developer discussion we discussed the merits of extracting modules vs extracting classes. Below are my notes from our lively discussion. Chime in with clarification or views of your own.
What Makes A Good Mixin?
- Repeatable, self contained functionality
- A clear contract with a minimal interface for that contract
Examples of a good mixin module are
Comparable. They both have single method contracts (
<=> respectively) and are repeatable and applicable across a wide number of objects. As a developer, you’d have no logical reason to reach into those modules and override one of the methods. Contrast this with the
ActiveModel::Model mixin. The room full of rails developers couldn’t identify what the contract for
ActiveModel::Model is. When you include AM:M, you still often find yourself reaching into it to override various methods.
** Mixins vs Object Composition in Rails **
It was agreed that mixins, as used in Rails, tend to break the rules for good mixins far more often than they follow them. Rails, unfortunately, encourages a proliferation of mixins because object composition is so difficult in, for instance ActionController or ActiveRecord.
We reviewed an example of an application that had several types of resources (
Article) that could be commented on. Each of these had their own controller that defined a single private method (
commentable). The actual functionality (
create actions) were defined in an included module.
It would be more dry (in both controllers and in your routes file) if you could use composition to do this instead, but doing so would be to fight with Rails conventions. Even if you found a solution you were happy with, you’re now outside of the typical rails convention and other Rails developers may find your code obfuscated by the new conventions you’re trying to put in place.
** Conclusion **
The conclusion we wanted to give was to not use mixins. Unfortunately, as pragmatic developers we admit that sometimes the best thing to do is to follow the conventions. Even in these situations you should consider if busting out of the rails ActionController/ActiveRecord sandbox would present an easier solution. Can you use a plain old Ruby object that could do the work just as well and offer you an opportunity to use composition instead?
Joe drew us a very helpful flow chart
The contract for
ActiveModel::Model is I want to do everything. We should probably refactor this mixin to be defined in terms of the
everything method to gain the nice interface you mentioned for others.
Have you heard that
nil is not an activemodel-compatible object?
+1 from me.
My biggest issue is having random methods called from outside my class and not having a sense of what they do. The implicitness of modules really trip me up. Testing them always feels really brittle as well, mixing in an instance of a dummy class, etc.
The disagreements I’ve had with others about module use are usually centered around “exceptions” like view logic (I’d recommend a presenter class) and repeated behavior across classes (I’d use composition to accomplish DRY).
I believe module mixins to be a form of inheritance, based on the inclusion of behavior from another object, where we prefer composition instead.
Maybe this is good Rails-specific advice, but defining your own business objects using modules or classes should be very natural. I recommend listening to Ruby Rogues #22 for a good discussion of modules and classes.
@derekprior I really appreciate your notes. Keep them posting! Thanks
thanks @derekprior! finally I know I’m not the only one who’s always in internal debates with myself regarding mixins(“use or not to use, that is the question” )
+1 @derekprior for publishing these dev discussions on the forum. Super useful.
Out of curiosity - with regard to extracting classes, do you guys consider giving names to the types of objects when you’re creating them?
I see in the Learn repo there is a services directory and you’ve touched on value objects in another discussion as well as form objects & decorators in other blog posts.
It seems a lot of people in the Rails community are trying to formulate a ‘conventional’ non-Railsy way to structure their application logic (i.e. app/services, app/policy etc), whereas you guys [at thoughtbot] seem to care about sticking to the Rails way but using POROs (in most instances) to clear things up? Is that a fair statement?
I just watched @benorenstein’s refactoring talk which I highly recommend where he’s quite liberal in creating new classes. It’s an eye-opener for anyone else who’s interested in this topic.
I wouldn’t say we have a convention as to the types of objects we extract or where we put them. I think that’s less important than actually extracting them.
The project I’m working on right now has these folders in app:
- admin - active_admin stuff
- builders - factory objects (named builders because we didn’t want spec/factories and spec/factories.rb)
- delegates - I don’t really know what the unifying principle is here…
- facades - View models, perhaps. We use them to present a single object to the view rather than many ivars.
- inputs - custom simple_form inputs
- models - not just AR:Base stuff.
- presenters - adding view-specific stuff to models
- processes - basically Command pattern stuff. Objects that encapsulate a business process
- validators - custom validators
On this project we were pretty liberal about creating new folders, as you can see. I’m not happy with all of them and think we were inconsistent with how we named things, but the extractions themselves definitely made for better code. Most of those things are PORO’s. I don’t use things like Draper.
I’d like to see some convention over the tyopes of objects we frequently extract and where we put them, but I think that’s still emerging.
That’s really interesting. Thanks for sharing @derekprior.
Seeing a /facades directory reminded me of this blog post. I had been researching decorators & presenters over the last couple of days to try and figure out how to pass Sandi Metz’s fourth rule and couldn’t remember that post! With facades, you probably want to concentrate on refactoring all of your other objects first to ensure SRP before lumping them in a facade?
In the /processes directory are you referring to things like form objects? (e.g. RegistrationProcess etc)
It would be cool to see/learn more about the /builders directory and what you’re doing there exactly?
Thanks for the insight. I think I’m going to try and write a blog post on what folders I might have in my application and what may be inside them. Perhaps I’ll post it here to gather some feedback
This is the project that post came from.
No. We typically think of those as models. “Registration”, for instance. The process directory has objects such as
NotifySubscribers (kicks off notification emails on content that has been updated),
AddUserToGroup ( takes a membership request, creates the membership, sends associated emails).
The idea here was that these objects exist solely to create/build other objects in the system (single or multiple). For instance, we have
ActivityFactory which takes a list of things that can appear in an activity feed and either creates an activity or updates the existing activity.
I think there’s actually a lot of overlap in the types of objects that exist in these various folders. I’m not too worried about it. I’d probably try to consolidate some of them on my next go round.
Really insightful. I see what you mean now by using the Command pattern for processes and thanks for clarifying the /builders directory with the
ActivityFactory example. I think a lot of people would perhaps lump that kind of behavior into a service object without being explicit about it using an abstract factory pattern.
Definitely going to try and apply some of these ideas into my next project.