Feature #17314: Provide a way to declare visibility of attributes defined by attr* methods in a single expression

• Status: Closed
• Assignee: -
• Target version: -

[radarek (Radosław Bułat)]

Description

Many of us (me included) declare class attributes, even if they are private, on the top of class definition. When reading source code it's convinient to see what kind of attributes class has.

To declare private attributes we can:

• declare them with one of attr* methods and later change visiblity calling private
• call private without argument, then declare attributes and finally call (in most cases) public to keep defining public methods
• declare attribute on top of the class but make them private in private section later in a file

clsss Foo
  attr_accessor :foo
  private :foo, :foo= # we have to remember about :foo= too

  private

  attr_accessor :bar

  public

  # rest of the code
end

To simplify it and create other possibilites I propose to:

• change attr* methods so as they return array of defined methods names
• allow public/protected/private methods to receive array of methods names (single argument)

With requested feature we could write code like this:

class Foo
  private attr_accessor :foo, :bar
end

Additionaly you could use attr* with your own methods. Something like this:

class Module
  def traceable(names)
    # ...
    names
  end
end

class Foo
  traceable attr_accessor :foo
  # it can be mixed with public/protected/private too
  protected traceable attr_accessor :bar
end

Backward compatibility

• attr* methods currently return nil so there should be no problem with changing them
• public/protected/private methods receive multiple positional arguments and convert all non symbol/string objects to strings. I can imagine only one case where compatibility would be broken:

class Foo
  def foo; end
  def bar; end

  arr = [:foo]
  def arr.to_str
    'bar'
  end

  private arr
end

p [Foo.public_instance_methods(false), Foo.private_instance_methods(false)]

Currently [[:foo], [:bar]] would be displayed, [[:bar], [:foo]] after requested feature is implemented.

Implementation

You can view my implementation in this (draft) PR: https://github.com/ruby/ruby/pull/3757

[marcandre (Marc-Andre Lafortune)]

I think this is a reasonable request that could be helpful. I've been tempted a few times to write this.

[sawa (Tsuyoshi Sawada)]

Private attribute methods defeat the purpose of attribute methods. If you want to access them within the class, you can directly access the instance variables.

[I]t's convenient to see what kind of attributes [a] class has

For that purpose, you can write that as a comment in the code. Defining methods just for that purpose is waste of resource.

[marcandre (Marc-Andre Lafortune)]

Private attribute methods defeat the purpose of attribute methods

I should have mentioned it, but it is more useful for protected than private.

[zverok (Victor Shepelev)]

Private attribute methods defeat the purpose of attribute methods. If you want to access them within the class, you can directly access the instance variables.

The idea behind private attribute methods (which I've seen used in a lot of codebases, and use myself) is uniformity of the call-sequence of different values. I believe Avdi Grimm had a nice article on this point of view (will try to find it).

Basically, if in some "service" (command, operation) class we have a code like this:

class MyCommand
  def initialize(name)
    @name = name
  end

  def call
    path = File.join(BASE_PATH, @name)

    # a lot of processing done with path
    data = File.read(path)
  end

  private

  # some helper methods
end

Now, there are several ways this code may evolve:

For example (1), path calculation more complicated and extracted to its own method

def path
  File.join(BASE_PATH, @name)
end

def call
  # a lot of processing done with path
  data = File.read(path)
end

...or, if the calculation is heavy, it is also memoized (2)

def path
  @path ||= File.join(BASE_PATH, @name)
end

def call
  # a lot of processing done with path
  data = File.read(path)
end

Or -- to the point to this ticket -- suddenly we have a refactoring when the whole path is passed by class' client (3):

class MyCommand
  def initialize(path)
    @path = path
  end

  def call
    # a lot of processing done with path
    data = File.read(path)
  end

  private

  attr_reader :path

  # some helper methods
end

The important point here is: in either case, code working with just path, continues to work. There is no point in rewriting all relevant statements with @path in case (3).

More realistically, we might start with instance variable, and then calculation of it will become more complicated -- but due to the same reasons, it is reasonable to immediately start using private attr_reader, not "naked" instance var.

[radarek (Radosław Bułat)]

sawa (Tsuyoshi Sawada) wrote in #note-2:

Private attribute methods defeat the purpose of attribute methods. If you want to access them within the class, you can directly access the instance variables.

[I]t's convenient to see what kind of attributes [a] class has

For that purpose, you can write that as a comment in the code. Defining methods just for that purpose is waste of resource.

What kind of resource, cpu? But there are other resources involved which is more importand - humans. Using private attributes gives you more flexibility and it's easier to refactor.

If you use accessor methods you can later refactor them without checking where instance variables are used.

class Foo
  attr_accessor :bar
  private :bar, :bar=
end

Later you decided to change setter so it validates value

class Foo
  attr_reader :bar
  private :bar

  private def bar=(value)
    raise ArgumentError if if value < 0
    @bar = value
  end
end

It also works with subclassing. If you access or change your state through accessors methods then it's much easier to refactor you base class without worrying about other places.

zverok (Victor Shepelev) wrote in #note-4:

The idea behind private attribute methods (which I've seen used in a lot of codebases, and use myself) is uniformity of the call-sequence of different values. I believe Avdi Grimm had a nice article on this point of view (will try to find it).

I guess you are talking about this Ruby Tapas episode: https://www.rubytapas.com/2018/06/05/barewords/

[zverok (Victor Shepelev)]

The idea behind private attribute methods (which I've seen used in a lot of codebases, and use myself) is uniformity of the call-sequence of different values. I believe Avdi Grimm had a nice article on this point of view (will try to find it).

I guess you are talking about this Ruby Tapas episode: https://www.rubytapas.com/2018/06/05/barewords/

Indeed! Thanks.

[Dan0042 (Daniel DeLorme)]

+1

Ever since ruby 2.1 when def was improved to return a Symbol, I've always wondered why the same was not done for attr_* methods.

[marcandre (Marc-Andre Lafortune)]

I would also like alias_method x, y to return x so we could write protected alias_method :foo, :bar

[phluid61 (Matthew Kerwin)]

Dan0042 (Daniel DeLorme) wrote in #note-7:

+1

Ever since ruby 2.1 when def was improved to return a Symbol, I've always wondered why the same was not done for attr_* methods.

attr_accessor :x, :y  #=> ?

[radarek (Radosław Bułat)]

marcandre (Marc-Andre Lafortune) wrote in #note-8:

I would also like alias_method x, y to return x so we could write protected alias_method :foo, :bar

Additionaly both activesupport's delegate and ruby's def_delegators methods already return arrays of defined methods, which would allow you to define private/protected delegators like this:

class User < ActiveRecord::Base
  has_one :profile

  private delegate :date_of_birth, to: :profile
  private def_delegators :profile, :date_of_birth
end

[radarek (Radosław Bułat)]

phluid61 (Matthew Kerwin) wrote in #note-9:

attr_accessor :x, :y  #=> ?

This is exactly why I created this feature request. Improved attr_accessor returns array of defined methods and private/protected/public receives single array as argument.

[marcandre (Marc-Andre Lafortune)]

A Ruby summary:

class Foo
  protected [:x, :y]
  # same as:
  protected :x, :y

  attr_accessor :foo, :bar # => [:foo, :foo=, :bar, :bar=] instead of `nil`
  attr_reader :foo, :bar # => [:foo, :bar] instead of `nil`
  attr_writer :foo, :bar # => [:foo=, :bar=] instead of `nil`

  alias_method :new_alias, :existing_method # => :new_alias instead of `Foo`
end

[matz (Yukihiro Matsumoto)]

I have a small concern about compatibility. But basically agree. Go ahead.

Matz.

[nobu (Nobuyoshi Nakada)]

• Status changed from Open to Closed

Applied in changeset git|34f06062174882a98ebef998c50ad8d4f7fc0f2e.

---

Added tests for [Feature #17314]

[mame (Yusuke Endoh)]

Hi, I've just noticed that this changeset allows not only Module#private but also Module#private_class_method to accept an array of symbols. Module#public_class_method too.

class Foo
  def self.foo; end
  def self.bar; end  
  private_class_method [:foo, :bar] # No error
end

And toplevel Kernel#private and Kernel#public do too.

def foo = nil
def bar = nil

private [:foo, :bar]
public [:foo, :bar]

I cannot think of a case where we want to pass an array of symbols to public_class_method and toplevel public. (Note that there is no toplevel attr_reader.) So I believe these changes are not intentional. But it may be more consistent for the methods to allow a symbol array. I have no opinion whether the additional methods should allow or deny an array. What do you think?

Anyway, I've added a rdoc, tests, and NEWS for the extra methods at 3a81daaf8dc037057d96b8e8cdc6ab1691e7e9d9.

[marcandre (Marc-Andre Lafortune)]

It wasn't intentional but I think it is better this way:

• It is consistent
• Allows users to define methods to be chained (class_attr_reader? class_cache?) that could return array of symbols.
• no downside that I can see.

[zverok (Victor Shepelev)]

I cannot think of a case where we want to pass an array of symbols to public_class_method

Generally, there could be other DSLs that will define methods. Rails have mattr_{reader,writer,accesor} (for module attributes), which currently does not return the "proper" value to pass to public_class_method, but they can be change to do so. Then, this would make sense:

class A
  private_class_method mattr_reader :some, :internals
end

[k0kubun (Takashi Kokubun)]

• Related to Feature #6470: Make attr_accessor return the list of generated method added