会社で議論になり、未だに私の中で決着がついていない問題を取り上げてみます。
例えば、コンストラクタでインスタンス変数を設定します。
そして、インスタンスメソッドでそのインスタンス変数を直接参照して処理をして値を返却するということをします。
良い例ではないかもしれませんが、こんな感じのイメージです。
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の本には、このパターンについて２つの問題点が挙げられていました。今手元に無いので詳細は覚えていないのですが、もしかしたら、そちらの問題点のほうが重要なのではないかとさえ思いました。
そこで、いくつか参考例を見てみようと思い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++の素養がゼロなのであまり理解できておりません・・・。
この辺りも参考になりそうです。 http://qiita.com/Yahagi_pg/items/1bf59fc75d7f17c3b731
正直rubyでどう書くのがベストなのか、結論づけられていません・・・。
なお、車内で日本人エンジニア４人ほどと議論したところ、皆インスタンス変数直接参照すれば良いという意見でした。
※念のため繰り返しますが、インスタンス変数が外部から参照される必要が無い場合です。外部から参照される場合は、素直にattr_readerを使えば良いです。
の共著者）は、あなたが言語の設計が完璧であるはずはないと思っていると言った。なぜだ？