Or: I’m getting too old for magic tricks
So much of what we try to do is get to a point where the solution seems inevitable: you know, you think “of course it’s that way, why would it be any other way?” It looks so obvious, but that sense of inevitability in the solution is really hard to achieve.
~ Jony Ive, July 2003
I’ve been doing Rails for nearly a decade. I’ve seen bits of magic come and go, I’ve written too-fancy abstractions that leak like sieves, and mostly I’ve worked both solo and on teams. I’ve come to like boring code. Code with little to no magic, that looks “enterprisey,” that has too many classes and objects, and uses boring old things like inheritence instead of composition.
Boring code is easy to read, and easy to debug. When you don’t define methods and classes dynamically, you can actually use the stacktrace. When you don’t use mixins, modules and concerns, you never have to wonder where a method is defined. You can grep your codebase. When you separate domain logic from the underlying technology, it’s very clear what is happening where.
That’s very helpful for working on teams. Everyone should be able to read and understand your code. The ability for someone else to understand and work with your code has an inverse, exponential correlation with the number of files, objects, and messages between input and output. Layers of indirection and metaprogrammed magic make the curve even steeper.
I want to make it really hard for the most annoying, stupid member of my team to screw it up: future me. Me in three months, when I’ve lost context and forgotten why I wrote any of this, or how. I want him to pick it up. Maybe he’ll say “man, this code is stodgy,” but he’ll understand it immediately.
Let’s get to work.
No side effects in model code
Code in your models should not change any other models, send emails, call APIs, or write to anything other than the primary data store. Especially in callbacks.
Callbacks are great for setting and verifying internal state. A callback to normalize a url, email, or url slug is great. You’re just ensuring the model’s data is consistent. A callback to send an email is total bullshit. There will be times, probably many of them, when you do not want to send that email. Data migrations, actions from admins, a hundred other cases. Put those actions in another class, or make a method that is never called automatically. Force yourself to be explicit about when that is happening in your controllers, background workers, etc.
Of course there are exceptions. touch: true is generally fine, as long as the touched model has no side effects on update.
No observers
Observers were removed in Rails 4 for a reason. They are invisible logic that no one knows to anticipate. Use explicit calls in controllers or workers.
No default scopes
When you write an ActiveRecord query, you should see exactly what it does. No one should have to wonder why they are getting unexpected ordering, joins or n+1 queries.
No state machines for models
Everyone thinks this state machines for your models are a great idea, and I’ve no idea why. Look at all these state machines. These put your business logic inside your models. That’s great, right? I mean, it gets them out of the controller. But models are not your junk drawer for business logic.
Models will get to invalid states, as inevitably as the fucking tides. The business logic will change. You will deploy bugs. Then you have to do some ugly hack like update_columns status: 'fml' to herd them back into line. You have to do a ton of setup in tests. State machines define tons of magic methods. Guard methods, state-specific methods, and transitions will fail.
State machines are for in-line processing. Regular Expressions are a great example. They are not for asynchronous changes over time that sync to an external service like a database.
Just use a bloody string field, or better yet an ActiveRecord Enum. You can use conditional validations, but really you should put your business logic elsewhere.
Avoid instance variables in views & helpers
I write partials like this:
# app/views/blog_posts/_byline.html.erb
<%
post = local_assigns[:post] || @post
%>
<div class="byline">
<span class="author-avatar"><%= fetch_author_avatar(post.author) %></span>
<span class="author-name"><%= post.author.name.titleize %></span>
<span class="post-date"><%= localize post.updated_at %></span>
</div>
# app/helpers/blog_posts_helper.rb
module BlogPostsHelper
def fetch_author_avatar(author)
CDNFetcher.generate_url(author.avatar_url)
end
end
Even that’s not great, since post.author may be an n+1 query, but that’s manageable with the Bullet gem.
Explicitly declaring variables and passing dependencies downward makes it crystal clear where everything is coming from. When you want to render this partial in some other view, and you inevitably will, you won’t have to dig through the whole chain and figure out what to set in the controller. Instance variables are effectively global variables for the view scope, and nobody likes globals.
Locals are excellent for making sure your partial doesn’t depend on instance variables, but they’re bloody annoying when it isn’t clear where they’re coming from. The local_assigns hash prevents cryptic undefined method errors, makes the partial’s dependencies explicit, and allows you to override them when you’re using the instance variable for something else. I even pull a local out of this hash for the partial-name-variable passed in with render partial: 'my_partial', object: obj - byline in this case. This allows for defensive coding, sensible defaults, and makes it an explicit dependency.
Helpers that depend on instance variables are less clear and less reusable than helpers with arguments. They compound the problem of instance variables in views or partials, since they’re not immediately visible when looking at the view code.
No view helpers in models or controllers
“Convention over configuration” is one of the huge benefits of Ruby on Rails. You don’t wonder where to put this or that bit of code, and other devs don’t wonder where to find it. If you have a method on a model that formats a name so it can be used in a view, you’ve made it harder for anyone else to find. Same thing if you define a helper in a controller that is used in the view.
Use additional conventions
Some really smart people in the Rails community have invented more specialized objects for parts of a Rails app, and they had some good reasons. Form Objects, Service Objects, Presenters, and other conventions exist to help you keep your code clean and DRY.
Don’t always or dogmatically use these things - a form to update a string in a model doesn’t need a form object. A controller that saves one model and makes an API call doesn’t need a service object. But when code gets re-used or specialized, these can be super helpful. Having more conventions for your team helps keep it obvious where any given piece of code is or should be.
Don’t go too far, either - I think Trailblazer or Hexagonal Architecture make it harder for Rails devs to understand where things are, and tempt you into using more magic to wire everything up.
Remember that abstractions hurt
All abstractions leak, and these are some of the most aggravating bugs to deal with. You end up pouring through someone else’s source code trying to figure out what the hell is going on. Not to pick on Trailblazer (it really does look interesting), but when I saw the contract / validation DSL I immediately shook my head. Knowing when something is invoked and how is pretty important. The more of that you have to keep in your head, the less working memory you have for actually writing your code.
To justify an abstraction, it has to have 10x easier than operating without it. Not using the abstraction has to be so painful that you’re actively losing hair over it.
For example, this is my main issue with HAML. It’s a big abstraction - it takes you very far away from the actual HTML you want to render - and the only value it provides is “it’s pretty.” And it’s not even pretty, for non-trivial apps. If you use BEM notation, any amount of data attributes, conditional classes, or I18n, you end up with perl-like punctuation soup. You can’t even add arbitraty white space to make it more readable.
Sass (in its scss form) is a great counter-example. Lacking variables, comprehensions, and clear inheritence is a massive pain when writing css. Sass keeps you pretty close to the generated css, and provides 100x the power.
DSLs, Concerns, transpiled languages, and syntax sugar gems are all suspect. Be mindful about when and how you introduce new layers of abstraction.
Don’t monkey patch
Duh. Use Decorators to make it explicit where your methods are coming from.
These are all very general guidelines. Rules are meant to be broken, and you totally should if it makes your code 10x easier. I’ll add more if I can think of anything else.
