I’ve occasionally come across advice to wrap instance variables in attr_reader
. There are two supposed benefits of this practice, which I’ll describe shortly. First I’ll provide a piece of example code.
Example of wrapping instance variables in attr_reader
Original version
Here’s a tiny class that has just one instance variable, @name
. You can see that @name
is used in the loud_name
method.
class User
def initialize(name)
@name = name
end
def loud_name
"#{@name.upcase}!!!"
end
end
attr_reader version
Here’s that same class with an attr_reader
added. Notice how loud_name
now references name
rather than @name
.
class User
attr_reader :name
def initialize(name)
@name = name
end
def loud_name
"#{name.upcase}!!!"
end
end
The purported benefits
Advocates of this technique seem to find two certain benefits in it. (I’ve also come across other supported benefits but I don’t find them strong enough to merit mentioning.)
It makes refactoring easier
Rationale: If you ever want to change @name
from being defined by an instance variable to being defined by a method, then you don’t have to go changing all the instances of @name
to name
.
This reasoning isn’t wrong but it is weak. First, it’s very rare that a value will start its life as an instance variable and then at some later point need to change to a method. This has happened to me so few times that I can’t recall a single instance of it happening.
Second, the refactoring work that the attr_reader
saves is only a trivial amount of work. The cost of skipping the attr_reader
is that you have to e.g. change a handful of instances of @name
, in one file, to name
. Considering that this is a tiny amount of work and that it needs to happen perhaps once every couple years per developer, this justification seems very weak.
It saves you from typo failures
Rationale: If you’re using instance variables and you accidentally type @nme
instead of @name
, @nme
will just return nil
rather than raising an error. If you’re using attr_reader
and you accidentally type nme
instead of name
, nme
will in fact raise an error.
This justification is also true, but also weak. If typo-ing an instance variable allows a bug to silently enter your application, then your application is not tested well enough.
I would be in favor of saving myself from the typo problem if the attr_reader
technique hardly cost anything to use, but as we’ll see shortly, the attr_reader
technique’s cost is too high to justify its benefit. Since the benefit is so tiny, the cost would have to be almost nothing, which it’s not.
Reasons why the attr_reader technique is a bad idea
Adding a public attr_reader throws away the benefits of encapsulation
Private instance variables are useful for the same reason as private methods: because you know they’re not depended on by outside clients.
If I have a class that has an instance variable called @price
, I know that I can rename that instance variable to @cost
or change it to @price_cents
(changing the whole meaning of the value) or even kill @price
altogether. What I want to do with @price
is 100% my business. This is great.
But if I add attr_reader :price
to my class, my class suddenly has responsibilities. I can no longer be sure that the class where @price
is defined is the only thing that depends on @price
. Other clients throughout my application may be referring to @price
. I’m no longer free to do away with @price
or change its meaning. This makes my code riskier and harder to change.
You can add a private attr_reader, but that’s unnatural
If you want to make use of the attr_reader
technique but you don’t want to throw away the benefits of encapsulation, you can add a private attr_reader
. Here’s what that would look like.
# attr_reader version
class User
def initialize(name)
@name = name
end
def loud_name
"#{name.upcase}!!!"
end
private
attr_reader :name
end
This solves the encapsulation problem, but what have we really gained on balance? In exchange for not having to change @name
to name
on the off chance that we change name
from an instance variable to a method, we have to pay the price of having this weird private attr_reader :name
thing at the bottom of our class.
And consider that we would have to do this on every single class that has at least one instance variable!
Don’t wrap instance variables in attr_reader
Wrapping your instance variables in a public attr_reader
changes your instance variables from private to public, increasing the public surface area of your class’s API and making your application a little bit harder to understand.
Wrapping your instance variables in a private attr_reader
adds an unnatural piece of boilerplate to all your Ruby classes.
Given the tiny and dubious benefits that the attr_reader
technique provides, this cost isn’t worth it.
Using attr_reader
is good and necessary for values that really need to be public. As a default policy, wrapping instance variables in attr_reader
is a bad idea.
In this example, the original code is fine (well… in this example, I wouldn’t even bother with a class), but if you have more complex classes, then the private `attr_reader` doesn’t require you to make the class 50% longer and you probably already have a private section, so it’s just a matter of where you put the `attr_reader`. As it gets more complex, there will be more vars, more methods, etc, and you’ll be doing any amount of renaming. Tests should still catch misspellings, but that doesn’t mean that the error will be right at the place that it occurs. Eg if you extract a bunch of logic into a class and pass the ivar to the class, but misspell it, then your tests will observe that something blows up down in the depths of that other class, and you’ll have to debug it back to that ivar being `nil` b/c of the misspelling.
Anyway, point being, for something this simple, yeah, go with ivars, but as you get more code / more vars / more complexity, start preferring private `attr_accessor` (then you can use the setter and catch misspellings when setting and when getting)
agreed – adding a private attr_reader doesn’t seem “unnatural” or any more effort than declaring it publicly
Hi Josh, these are all very fair points. My overall verdict is unchanged but I wouldn’t disagree with any of these particular things.
+1 for private attr_reader declarations
I appreciate the logic in your arguments and think that’s fair enough. It’s made me reflect on why I mostly use private attr_readers – I think its the encapsulation angle, and a habit I developed after a watching a talk by ( I _think_) Sandi Metz.
I’m interested in your thoughts on the barsoom/attrr_extras gem as a shorthand? I heard Ben Orenstein pick it on the Rogues podcast a few years back and I have been using it ever since. Its very concise – your example becomes this (which some will say is the work of devil but I really like):
class User
pattr_initialize :name
def loud_name
“#{name.upcase}!!!”
end
end
Interesting. I had not heard of attr_extras before now. On one hand I like what it does. On the other hand I feel weird about mixing in a third-party dependency into something so common and fundamental. I guess I’d say I 20% like it and 80% don’t like it.
I think you are forgetting about inheritance.
100% agree that public attr_readers are (as a rule of thumb) bad because they break encapsulation.
There are benefits to using them though.
Error handling of methods is better, since instance variables default to nil. When you call an undefined method a NameError is raised, making it much easier to figure out that you’ve made a mistake.
Another reason is during equality comparison. If you want to define == then fairly often you’ll want to compare that the encapsulated objects are equal, so being able to do something like name == other.name is nice.
For these reasons, I tend to make my attr_readers protected so that they’re callable by instances of the same class, but nothing else. I’ve written a bit more about this at https://www.garethrees.co.uk/2020/06/14/use-protected-attr-readers/
It seems like you are arguing more from an academic perspective and less a practical one. In more than a decade of writing Ruby and Rails code, I don’t think I’ve ever seen a case where public accessors have come back to bite someone in application code. For shared library code it might make sense to be more careful, but even then I’d probably err on the side of private attr accessors.
You mentioned the benefits, but you’re kind of dismissive of them.
> it’s very rare that a value will start its life as an instance variable and then at some later point need to change to a method
In my case it frequently happens in my code that something starts as an attr accessor or an instance variable and wants to become a method. These kinds of refactorings unnecessarily bloat the diff when switching from an instance variable.
> If typo-ing an instance variable allows a bug to silently enter your application, then your application is not tested well enough.
Maybe, but I’d rather have the extra layer of protection. Being human, it’s common for me not to get everything right the first time, including my tests. I try to use practices that reduce the cost of my human fallibility. When I typo an instance variable, the error can often be far from the source of the issue. Even when I’m writing the tests this can be frustrating and confusing. Having a mis-typed method name throws an error right at the moment it is called.
> You can add a private attr_reader, but that’s unnatural
This is a subjective assessment, and doesn’t provide any concrete rationale. Just like any other practice, if you get used to it, it won’t seem so unnatural anymore.
Quality code is often not a matter of huge wins, but small wins that add up over time. The more I can integrate small practices that help me avoid getting tripped up, the more I am able to get code out the door that works right the first time.
Thanks, there’s actually very little of this that I would disagree with.
I would argue though that I am in fact coming at it from a practical perspective. The specific cost of a public attr_reader is that when I see attributes that are made public by attr_reader, I can’t easily know if it’s safe for me to refactor those attributes. This isn’t an academic concern as it has happened to me. The fact that it has recently happened to me multiple times is what prompted me to write this post.
In that case I might throw your own argument back at you: “your application is not tested well enough.” Generally speaking, in the scenario you describe I would make the change I want to make and see what tests fail. With a well tested codebase, I’ve found it makes these sorts of refactorings super easy. We recently replaced an entire database table using this method. It’s a strategy I use frequently to rename things across the codebase (as well as some grep-fu to track down loose references in the views).
I thoroughly agree with Robert Fletcher’s arguments. I’d use attr_reader over using that instance variable directly every time. It’s much cleaner, and the misspelling issue is big for me and not one that can be lightly dismissed with – your tests should deal with that.
Good points, there’s also the performance argument, `attr_reader` adds a method call which has some overhead:
“`ruby
require “benchmark”
REP = 10_000_000
class Foo
attr_reader :bar
def initialize; @bar = 99 end
def get_var; @bar; @bar; @bar; @bar; @bar end
def get_acc; bar; bar; bar; bar; bar end
end
f = Foo.new
Benchmark.bm do |x|
x.report(“variable”) { REP.times { f.get_var } }
x.report(“accessor”) { REP.times { f.get_acc } }
end
# user system total real
# variable 0.352928 0.000520 0.353448 ( 0.353455)
# accessor 0.645047 0.000031 0.645078 ( 0.645110)
“`
If it’s in a hot path this could make a sizeable difference.
Thanks. I knew about the performance argument but chose not to include it because, frankly, I don’t think it’s that strong of an argument. I don’t disagree that it could theoretically become an issue in certain circumstances, it just seems unlikely to enter the picture in 99%+ of cases.
Thanks for sharing your interesting thoughts on this. Let me offer a slightly different take. I agree that instance variables are private in the sense that, by definition, only ‘self’ can refer to them (other than ivar_set and ivar_get but never mind those). The thing is, I don’t think that wrapping them in a reader method means that they are no longer “private” in the specific way that they were “private” to start with. They still cannot be referenced by a different ‘self’.
I see the attr_* methods simply as code-writing shortcuts. The question, then, is in my view not whether one should use attr_reader, but how one wants to handle per-object state. def x; @x; end is really just a special case of def x; [do something with @x, maybe memoize a calculation, whatever, log a message, etc….]; end. State is certainly a vexed topic in certain ways, but I don’t think attr_reader per se is a particular culprit.
One other note: you can always do this instead of putting everything at the bottom:
attr_reader :x, :y, :z
private :x, :y, :z
Hi David, thanks. I think I’m confused about one thing – is it not true that if you do e.g. attr_reader 😡 (no private) then x becomes a public attribute of that class?
The other stuff makes sense to me. I didn’t know you could do the private :x, :y, :z technique.
I also prefer the private attr_reader approach for many of the reasons above which I won’t echo for brevity.
I want to add another reason I like the use of attr_reader however and that is that when you use this approach you never end up using an instance variable literal in your code unless you are assigning a value to it.
As someone who reads a lot of code I appreciate that as it causes me to draw attention to these parts of the code as they are very important parts of the class and deserve the added attention.
The article is confused about object orientation.
OO is about messages between objects, i.e. the methods called, not instance variables. Objects are not structs. Other uses of a class, through inheritance, extension, refinement, etc., should not care if a value comes from a variable, a computation, a database query or a remote API request.
The article says that changing an instance variable happens “perhaps once every couple years.” But, say that the `User` class above needed a first name and last name:
“`
class User
def initialize(first_name, last_name)
@first_name = first_name
@last_name = last_name
end
end
“`
Too bad, it just happened again.
Without a `#name` accessor, we would need to know about all the class uses in the app: inheritance, mix-ins, etc. Search and replace if we owned the code or introduce a redundant `@name` instance variable.
The fact that the language designers made methods and local variables interchangeable and easy to type in Ruby, but used a “noise” prefix, for instance variables and globals, could make one think something else is missing.
My experience is the opposite. When I choose not to define an accessor, the few times are those not meant for an extension, if the requirement can’t change, where performance is an issue, etc. The code that comes to mind is not application code but test classes, request processing, view rendering, libraries, etc.
Calling arguments strong or weak based on preference alone can lead to wrong conclusions.
“Other uses of a class, through inheritance, extension, refinement, etc., should not care if a value comes from a variable, a computation, a database query or a remote API request.”
I assume by “other uses of a class” you mean clients of the class. If so I would totally agree. I’m not talking about clients though, I’m talking about the class itself.
Uses of a class are not limited to clients creating instances and calling class methods. Inheritance, extension, composition, delegation, decoration, etc., also depend on a class.
Take modules, for example:
“`
module TagHelper
def name_tag
“#{@name}”
end
end
class User
include TagHelper
end
“`
If we wanted to reuse that module for other objects with a “name,” we would need to contemplate “name” as a method or an instance variable.
Public and private interfaces are made of methods. Ivars, their names, format, type, etc., are an implementation detail that can, and will, change over time.
To your question:
>I assume by “other uses of a class” you mean clients of the class. If so I would totally agree. I’m not talking about clients though, I’m talking about the class itself.
The class itself shouldn’t care either unless the code is trivial. You don’t want to be caught using ivars in a 100-line class with inheritance and a few modules.
I do this:
“`
class User
private; attr_reader :name; public
end
“`
It works well. I’ve got a patch to Ruby to allow `private attr_*` methods but i stopped working on it as it would add an extra array allocation to each `attr_*` method call. https://github.com/ruby/ruby/compare/master…schneems:schneems/attr_array
That link got mangled. Here’s a shortened version https://bit.ly/3pIKCIe
> Adding a public attr_reader throws away the benefits of encapsulation
In Ruby 3.0+ you can:
“`
private attr_reader :name
“`
You don’t have to put it at the bottom