19

When defining accessors in Ruby, there can be a tension between brevity (which we all love) and best practice.

For example, if I wanted to expose a value on an instance but prohibit any external objects from updating it, I could do the following:

class Pancake
  attr_reader :has_sauce

  def initialize(toppings)
    sauces = [:maple, :butterscotch]
    @has_sauce = toppings.size != (toppings - sauces).size
...

But suddenly I'm using a raw instance variable, which makes me twitch. I mean, if I needed to process has_sauce before setting at a future date, I'd potentially need to do a lot more refactoring than just overriding the accessor. And come on, raw instance variables? Blech.

I could just ignore the issue and use attr_accessor. I mean, anyone can set the attribute if they really want to; this is, after all, Ruby. But then I lose the idea of data encapsulation, the object's interface is less well defined and the system is potentially that much more chaotic.

Another solution would be to define a pair of accessors under different access modifiers:

class Pancake
  attr_reader :has_sauce
  private
    attr_writer :has_sauce
  public

  def initialize(toppings)
    sauces = [:maple, :butterscotch]
    self.has_sauce = toppings.size != (toppings - sauces).size
  end
end

Which gets the job done, but that's a chunk of boilerplate for a simple accessor and quite frankly: ew.

So is there a better, more Ruby way?

A Fader Darkly
  • 3,516
  • 1
  • 22
  • 28
  • 1
    **Which gets the job done** Wrong! has_sauce inside your initialize() method is a local variable--not an instance variable. You didn't even test your code (which also has another error). **I mean, if I needed to process has_sauce before setting at a future date, I'd potentially need to do a lot more refactoring than just overriding the accessor.** Setting an instance variable by going through a setter is good practice, and using two access modifiers is the way to accomplish that for a read-only instance variable. – 7stud Aug 29 '14 at 17:45
  • Deepest apologies for the untested code, and well spotted. Fixed now, fwiw. Also, I'm glad you agree on this being good practice but this implementation feels slightly hacky. Probably just me. – A Fader Darkly Sep 02 '14 at 09:49

5 Answers5

13

private can take a symbol arg, so...

class Pancake
  attr_accessor :has_sauce
  private :has_sauce=
end

or

class Pancake
  attr_reader :has_sauce
  attr_writer :has_sauce; private :has_sauce=
end

etc...

But what's the matter with "raw" instance variables? They are internal to your instance; the only code that will call them by name is code inside pancake.rb which is all yours. The fact that they start with @, which I assume made you say "blech", is what makes them private. Think of @ as shorthand for private if you like.

As for processing, I think your instincts are good: do the processing in the constructor if you can, or in a custom accessor if you must.

AlexChaffee
  • 8,092
  • 2
  • 49
  • 55
  • 1
    I did explain what I don't like about using instance variables in this context. But to further explain it, a principle I use is: If you have an accessor, use the accessor rather than the raw variable or mechanism it's hiding. The raw variable is the implementation, the accessor is the interface. Should I ignore the interface because I'm internal to the instance? I wouldn't if it was an attr_accessor. – A Fader Darkly Aug 29 '14 at 16:25
  • 1
    Then `attr_accessor :bar; private :bar=;` is what you want. – AlexChaffee Sep 02 '14 at 14:41
  • 1
    The principle "if you have an accessor, use it" is more geared towards client objects rather than the object itself, in languages (like JavaScript and Java) where external objects *do* have direct access to instance vars. But sure, go ahead and enforce self-encapsulation if you like; it makes it easier to do memoization or whatever later on. – AlexChaffee Sep 02 '14 at 14:46
  • 2
    I've seen (and fixed) Ruby code that needed to be refactored for the client objects to use the accessor rather than the underlying mechanism, even though instance variables aren't directly visible. The underlying mechanism isn't always an instance variable - it can be delegations to or manipulations of a class you're hiding behind a facade, or a session store with a particular format, or all kinds. And it can change. 'Self-encapsulation' can help if you need to swap a technology, a library, an object specification, etc. – A Fader Darkly Sep 02 '14 at 14:58
  • 1
    I'm liking your use of private though - a fair compromise between ugly code and updating the Object class. – A Fader Darkly Sep 02 '14 at 15:00
6

attr_reader etc are just methods - there's no reason you can define variants for your own use (and I do share your sentiment) For example:

class << Object
  def private_accessor(*names)
    names.each do |name|
      attr_accessor name
      private "#{name}="
    end
  end
end

Then use private_accessor as you would attr_accessor (I think you need a better name than private_accessor though)

Frederick Cheung
  • 83,189
  • 8
  • 152
  • 174
  • That's more like it! I was considering doing exactly this, but I wanted to explore all other options before changing Object... I tend to try not to interfere with the fundamentals of the language unless it would be very hacky and/or inefficient not to do so... Or unless I really really want to. ;) – A Fader Darkly Sep 02 '14 at 09:53
  • Also, I couldn't think of a good name for the accessor either! Maybe that's why it's not in the language. I did find this in my travels: https://github.com/dbrady/scrapbin/blob/master/scraps/scoped_attr_accessor.rb which has the advantage of a large test suite, but the disadvantage of requiring two accessors to make up the required result. – A Fader Darkly Sep 02 '14 at 09:57
  • **and I do share your sentiment** Really? So to your sensibility writing: `private attr_writer :has_sauce` is like fingernails on a chalkboard, but writing `class << Object def private_accessor(*names) names.each do |name| attr_accessor name private "#{name}=" end end end private_accessor :has_sauce` feels smooth as silk? – 7stud Sep 02 '14 at 19:59
  • 1
    One of the consequences (although arguably not the primary motivation) of DRY is that you tend to end up with chunks of complex code expressed once, with simpler code referencing it throughout the codebase. I can't speak for anyone else, but I consider it a win if I can reduce repetition and tuck it away in some framework or initialisation code. Having a single accessor definition for a commonly used accessor makes me happy - and the new Object class code can be tested to hell and back. The upshot is more beautiful, readable code. Or would be if I could think of a good name for it... – A Fader Darkly Sep 03 '14 at 09:28
5

You can put attr_reader in a private scope, like this:

class School
  def initialize(students)
    @students = students
  end

  def size
    students.size
  end

  private

  attr_reader :students
end

School.new([1, 2, 3]).students

This will raise an error as expected:

private method `students' called for #<School:0x00007fcc56932d60 @students=[1, 2, 3]> (NoMethodError)
Dorian
  • 7,749
  • 4
  • 38
  • 57
3

Ruby 3.0 made access modifiers and attr_* work with each other, so you can just write

private attr_reader :has_sauce
Dave Schweisguth
  • 36,475
  • 10
  • 98
  • 121
2

There's nothing wrong with referencing instance variables directly within your class. attr_accessor is just doing that indirectly anyway, whether you make those methods public or private.

In this particular example, it may help to recognize that toppings are likely an attribute you want to save for other purposes, and has_sauce is a "virtual attribute", a characteristic of the model that's dependent on the underlying toppings attribute.

Something like this might feel cleaner:

class Pancake
  def initialize(toppings)
    @toppings = toppings
  end

  def has_sauce?
    sauces = [:maple, :butterscotch]
    (@toppings & sauces).any?
  end
end

Up to you whether or not to expose attr_accessor :toppings as well. If you're just throwing the toppings away, your class is less of a Pancake and more of a PancakeToppingDetector ;)

devpuppy
  • 822
  • 9
  • 8
  • I see a 'virtual attribute' as something we're forced to implement when using frameworks, ORMs and the like. Something that lets us inject our code into the path of whatever metaprogramming has been put in place for us. In a simple PORO like this, I don't see how it has meaning; it's just a method. :) – A Fader Darkly Apr 08 '16 at 15:27
  • With the whole 'using instance variables throughout the code' thing, I'll point you at this: https://github.com/dbrady/scoped_attr_accessor where someone's done the job for me. :) – A Fader Darkly Apr 08 '16 at 15:28
  • 2
    Also, Sandi Metz mentions this in POODR. As I recall, she also advocates wrapping bare instance variables in methods, even when they're only used internally. It helps avoid mad refactoring later. – A Fader Darkly Apr 08 '16 at 15:30
  • Yeah, ["virtual attribute"](http://docs.ruby-doc.com/docs/ProgrammingRuby/html/tut_classes.html) seems like dated terminology to me, so conceptually just a method. Totally possible to just add `private; attr_accessor :toppings` to the code above. – devpuppy Apr 08 '16 at 21:25
  • Indeed, see OP! To be fair, since I posted this a year and a half ago, I haven't been using any of the weird and wonderful systems I found for creating private accessors. I have been wrapping instance variables in accessor methods whenever I can though. – A Fader Darkly Apr 11 '16 at 08:45