Just Say No to Instance Variables
Or: How I Learned to Stop Worrying and Love bare_words
Note: This is part of the Production vs Tutorial Code Series.
Why the hate?
Hating on instance variables seems like a weird place to start this series, no? After all, how would you write object oriented code without them?
I don’t hate instance variables, I just try to limit their use.
My general rule is that there are two places instance variables belong:
- In an initializer, with
attr_reader
s for later referenceclass Note attr_reader :content def initialize(author:, content:) @author = author @content = content end private attr_reader :author # they can be private if it makes sense! end
- In memoized methods
class Note # returns an empty array vs `nil`, otherwise you would need code like in `posted?` below... def comments @comments ||= fetch_comments end # may return false, so you need fancier memoization code def posted? return @posted if defined?(@posted) @posted = check_if_posted end end
This may seem excessive, but I have found that reading instance variables in any other setting almost always makes code more brittle and hard to understand and change.
Using methods to reference data instead of instance variables (aka “bare words”) has another advantage: you can change the method definition.
Today it might be a humble attr_reader
, but maybe tomorrow it is a memoized method with logic.
Maybe next week it will get extracted out to a shared Concern
.
You can also switch between using a method call and local variables or method arguments seamlessly, allowing for a more functional style of code.
What About Rails Controllers & Views?
A common pushback I get is that instance variables are how data is passed to views in Rails. While this is certainly a way to get data into views there are other (better) ways.
Take a common example controller from a tutorial:
class NotesController < ApplicationController
def new
if current_user.can?(:write_rich_text)
@new_note = RichTextNote.new
else
@new_note = Note.new
end
end
end
For one thing, please don’t assign the same variable in each code path…
class NotesController < ApplicationController
def new
@new_note =
if current_user.can?(:write_rich_text)
RichTextNote.new
else
Note.new
end
end
end
Now let’s modify this to use a method called new_note
instead of @new_note
.
We will then use the provided rails controller macro helper_method
to export this method to the view context.
class NotesController < ApplicationController
helper_method :new_note
def new; end
protected
def new_note
@new_note ||=
if current_user.can?(:write_rich_text)
RichTextNote.new
else
Note.new
end
end
end
Note that the new
action now has no logic in it.
We like boring controller actions, because it means that action needs fewer (and sometimes zero) tests.
FAQ: Why is this method protected? Why not private like other non-action controller methods?
This is a convention to indicate we are sharing it with the view (vs just the controller in the case of a private method).
Note that it is only a convention. You could have your helper methods be private or even public if you wanted to.
Protected (or private) methods are harder to test (you can’t stub this method, and need to call controller.__send__(:new_note)
to invoke it in a test), but I think it is worth the pain.
If you really need to test this logic, you could extract a single purpose class or module to do the work!
class NotesController < ApplicationController
helper_method :new_note
def new; end
protected
def new_note
@new_note ||= NoteFactory.for_user(current_user)
end
end
module NoteFactory
def self.for_user(user)
if user.can?(:write_rich_text)
RichTextNote.new
else
Note.new
end
end
end
Now this controller doesn’t need any tests, which is my favorite kind of controller.
Additional Reading
-
Avdi Grimm’s Ruby Tapas episode on “barewords”
Note: I highly recommend Avdi’s Ruby Tapas screencase series for the growing ruby developer (and we should always be growing, right?)