雑草SEの備忘録

[旧:東大卒SEの備忘録]東大を卒業し、SEとして働くことになった。備忘録的に作業を綴る。

Rubyのインスタンス変数の直接参照について

会社で議論になり、未だに私の中で決着がついていない問題を取り上げてみます。

Rubyインスタンス変数を直接参照することについてです。

例えば、コンストラクタでインスタンス変数を設定します。

そして、インスタンスメソッドでそのインスタンス変数を直接参照して処理をして値を返却するということをします。

良い例ではないかもしれませんが、こんな感じのイメージです。

class PriceCalculation
  def initialize(price, tax)
    @price = price
    @tax = tax
  end

  def tax_included_price
    @price * (1 + @tax)
  end
end

前提として、@priceや@taxはこのクラス内のみで使用され、外部からは参照されないものとします。

これに対し、同僚のドイツ人がインスタンス変数を直接参照するのは良くないから、attr_readerを使えと指摘してきました。 彼が言うにはこんな感じです。

class PriceCalculation
  attr_reader :price, :tax

  def initialize(price, tax)
    @price = price
    @tax = tax
  end

  def tax_included_price
    price * (1 + tax)
  end
end

しかし、私は、attr_readerを使ってしまうと、PriceCalculationの外部からpriceやtaxが参照される必要が無いにもかかわらず参照できてしまい、そちらのほうがよろしく無いのでと思いました。

インスタンス変数をそのまま参照する方法で、attr_readerを使わない場合は参照することはできません。

(もちろんrubyなので参照できないわけではないですが)

例1: この方法だと呼び出せない

[1] pry(main)> class PriceCalculation
[1] pry(main)*   def initialize(price, tax)
[1] pry(main)*     @price = price
[1] pry(main)*     @tax = tax
[1] pry(main)*   end
[1] pry(main)*
[1] pry(main)*   def tax_included_price
[1] pry(main)*     @price * (1 + @tax)
[1] pry(main)*   end
[1] pry(main)* end
=> :tax_included_price
[2] pry(main)> pc = PriceCalculation.new(200, 0.08)
=> #<PriceCalculation:0x007f9b41ae0ae0 @price=200, @tax=0.08>
[3] pry(main)> pc.price
NoMethodError: undefined method `price' for #<PriceCalculation:0x007f9b41ae0ae0 @price=200, @tax=0.08>
from (pry):12:in `__pry__'
[4] pry(main)> pc.tax
NoMethodError: undefined method `tax' for #<PriceCalculation:0x007f9b41ae0ae0 @price=200, @tax=0.08>
from (pry):13:in `__pry__'
[5] pry(main)> # rubyなので読み出す方法はもちろんあるが・・・。
[6] pry(main)> pc.instance_eval("@price")
=> 200

例1: この方法だと呼び出せる

[1] pry(main)> class PriceCalculation
[1] pry(main)*   attr_reader :price, :tax
[1] pry(main)*
[1] pry(main)*   def initialize(price, tax)
[1] pry(main)*     @price = price
[1] pry(main)*     @tax = tax
[1] pry(main)*   end
[1] pry(main)*
[1] pry(main)*   def tax_included_price
[1] pry(main)*     price * (1 + tax)
[1] pry(main)*   end
[1] pry(main)* end
=> :tax_included_price
[2] pry(main)> pc = PriceCalculation.new(200, 0.08)
=> #<PriceCalculation:0x007fa20222e830 @price=200, @tax=0.08>
[3] pry(main)> pc.price
=> 200
[4] pry(main)> pc.tax
=> 0.08

彼の言い分としては、これはパターンなのだと。attr_readerを使ったほうが変化に耐えうるソースコードなのだと、そう言っていました。そして、Sandi Metzの本を参照してきました。 GitHubから引用させていただきます。 poodr/chapter_2.rb at master · skmetz/poodr · GitHub

############## Page 24 ##############
class Gear
  def initialize(chainring, cog)
    @chainring = chainring
    @cog       = cog
  end

  def ratio
    @chainring / @cog.to_f      # <-- road to ruin
  end
end

############## Page 25 ##############
class Gear
  attr_reader :chainring, :cog  # <-------
  def initialize(chainring, cog)
    @chainring = chainring
    @cog       = cog
  end

  def ratio
    chainring / cog.to_f        # <-------
  end
end

road to ruinとは一直線に破滅に向かうという意味です。

その理由としては、インスタンス変数を編集してクラスで共通に使いたくなるときにメソッドだけ修正すればいいから変化に耐えうるということが挙げられていました。

############## Page 25 ##############
  # a simple reimplementation of cog
  def cog
    @cog * unanticipated_adjustment_factor
  end

############## Page 25 ##############
  # a more complex one
  def cog
    @cog * (foo? ? bar_adjustment : baz_adjustment)
  end

こんな風に@cogをそのまま使うのではなく、編集して使いたい時に、attr_readerを使ったほうが良いということでした。

ただ、私個人の意見としては、attr_readerで外から見えるようにすると、どこで使われているかすぐにはわからなくなり、保守性が下がってしまうこと、また、この例で言う@cogをPage 25の例のように何か処理をして使いたくなる場合は、ほとんどないと思うこと、あったとしてもすべてのcogを編集したcogで使いたいと思うケースはないと思うので、この利点についてはまったく利点と感じませんでした。むしろ、なにか処理をしたい場合はその処理を意味する名前を冠したメソッド名をつけておかないと何を意味するのかが皆目分からなくなってしまいます。

  def some_decorated_cog
    @cog * (foo? ? bar_adjustment : baz_adjustment)
  end

のように処理の意味を付け加えておかないと可読性が落ちます。

※コンストラクタで設定するということもできますが、これは良いのでしょうか、悪いのでしょうか・・・。

  def initialize(chainring, cog)
    @chainring = chainring
    @cog = cog * (foo? ? bar_adjustment : baz_adjustment)
  end

このあたり、私はSandi Metzの主張するパターンには同意しかねます。Sandi Metzの本には、このパターンについて2つの問題点が挙げられていました。今手元に無いので詳細は覚えていないのですが、もしかしたら、そちらの問題点のほうが重要なのではないかとさえ思いました。

そこで、いくつか参考例を見てみようと思いDraperのコードをあたってみました。

draper/factory.rb at master · drapergem/draper · GitHub

すると、attr_readerをprivateにするというやり方をしていました。

確かにこれだと外部から参照されないことが約束されるので、保守性が高まります。(黒魔術が使われない前提。)

[1] pry(main)> class PriceCalculation
[1] pry(main)*   def initialize(price, tax)
[1] pry(main)*     @price = price
[1] pry(main)*     @tax = tax
[1] pry(main)*   end
[1] pry(main)*
[1] pry(main)*   def tax_included_price
[1] pry(main)*     price * (1 + tax)
[1] pry(main)*   end
[1] pry(main)*
[1] pry(main)*   private
[1] pry(main)*
[1] pry(main)*   attr_reader :price, :tax
[1] pry(main)* end
=> nil
[2] pry(main)> pc = PriceCalculation.new(200, 0.08)
=> #<PriceCalculation:0x007fb6739cc3c8 @price=200, @tax=0.08>
[3] pry(main)> pc.price
NoMethodError: private method `price' called for #<PriceCalculation:0x007fb6739cc3c8 @price=200, @tax=0.08>
from (pry):16:in `__pry__'
[4] pry(main)> pc.tax
NoMethodError: private method `tax' called for #<PriceCalculation:0x007fb6739cc3c8 @price=200, @tax=0.08>
from (pry):17:in `__pry__'
[5] pry(main)> pc.tax_included_price
=> 216.0

正直、私はインスタンス変数になにかの処理をして同名のゲッターメソッドでそれを呼び出すことに大きな抵抗を感じるので、インスタンス変数を直接参照することにくらべてどのくらいの利点があるのだろうと思いました。 ゲッターメソッド経由で参照されるので処理も少し遅くなります。

このあたりの議論をrubyではないですが、C++での議論がありました。コメントでの議論も含めて考えさせられましたが、私はC++の素養がゼロなのであまり理解できておりません・・・。

メンバ変数を隠蔽する理由 - Qiita

この辺りも参考になりそうです。 http://qiita.com/Yahagi_pg/items/1bf59fc75d7f17c3b731

正直rubyでどう書くのがベストなのか、結論づけられていません・・・。

なお、車内で日本人エンジニア4人ほどと議論したところ、皆インスタンス変数直接参照すれば良いという意見でした。

※念のため繰り返しますが、インスタンス変数が外部から参照される必要が無い場合です。外部から参照される場合は、素直にattr_readerを使えば良いです。