【92】【Rails】リファクタリングトレーニング_フックメッセージを使う

この記事を読むと問題のあるコードや改善の余地があるコードを学び、リファクタリングする能力を養うことができます。

改善の余地があるコード

問題があるコードというわけではありません。
しかし、まだ改善の余地があります。
どの点を改善すべきか説明できますか？
そして、どのようにリファクタリングしますか？

class Bicycle
 attr_reader :size,:chain,:tire_size
 def initialize(args)
   @size = args[:size]
   @chain = args[:chain] || default_chain
   @tire_size = args[:tire_size] || default_tire_size
 end
 def spares
   {
       tire_size: tire_size,
       chain: chain
   }
 end
 def default_chain
   "10-speed"
 end
 def default_tire_size
   raise NotImplementedError,
         "This #{self.class} cannot respond do to:"
 end
end
class RoadBike < Bicycle
 attr_reader :tape_color

 def initialize(args)
   @tape_color = args[:tape_color]
   super(args)
 end
 def spares
   super.merge({tape_color: tape_color})
 end

 def default_tire_size
   "23"
 end
end
class MountainBike < Bicycle
 attr_reader :front_shock,:rear_shock

 def initialize(args)
   @front_shock = args[:front_shock]
   @rear_shock = args[:rear_shock]
   super(args)
 end
 def spares
   super.merge({rear_shock: rear_shock})
 end
 def default_tire_size
   "2.1"
 end
end

テスト

require_relative '4_3_1_before_refactoring'

RSpec.describe "Bicycleモデル" do
 bike = RoadBike.new(size: "L",tape_color: "red")

 it '#spares' do
   expect(bike.spares)
       .to eq({:chain=>"10-speed", :tape_color=>"red", :tire_size=>"23"})
 end
end


RSpec.describe "Bicycleを継承したテスト用のサブクラス" do
 class Foo < Bicycle
   attr_reader :tape_color

   def initialize(args)
     @tape_color = args[:tape_color]
     super args
   end
   def spares
     super.merge({tape_color: tape_color})
   end
   def default_tire_size
     "23"
   end
 end

 it '#spares' do
   bike = Foo.new(size: "L",tape_color: "red")
   expect(bike.spares)
       .to eq({:chain=>"10-speed", :tape_color=>"red", :tire_size=>"23"})
 end

end

​

 改善点したい点

改善点を解説するために、リファクタリング前のテストを修正します。
どの点が修正されたか分かりますか？注意して見ないと分からない修正です。
こういった時テストって便利ですよね。
まず、下記のテストをやってみましょう。

require_relative '4_3_1_before_refactoring'

RSpec.describe "Bicycleモデル" do
 bike = RoadBike.new(size: "L",tape_color: "red")

 it '#spares' do
   expect(bike.spares)
       .to eq({:chain=>"10-speed", :tape_color=>"red", :tire_size=>"23"})
 end
end


RSpec.describe "Bicycleを継承したテスト用のサブクラス" do
 class Foo < Bicycle
   attr_reader :tape_color

   def initialize(args)
     @tape_color = args[:tape_color]
   end
   def spares
     super.merge({tape_color: tape_color})
   end
   def default_tire_size
     "23"
   end
 end


 it '#spares' do
   bike = Foo.new(size: "L",tape_color: "red")
   expect(bike.spares)
       .to eq({:chain=>"10-speed", :tape_color=>"red", :tire_size=>"23"})
 end

end

テストが通りません！！
なぜでしょうか？

got: {:chain=>nil, :tape_color=>"red", :tire_size=>nil}

nilを返しているから、テストが通らないようです。
なぜnilを返すのか？？

super(args)の記入漏れ
   def initialize(args)
     @tape_color = args[:tape_color]
   end

super(args)の記入漏れが原因でした。
この記入漏れをすぐ気づきましたか？
今回はテストで意図的に再現しましたが、実際に起こり得る可能性は十分にあります。このコードの改善点が見つかりましたね。

リファクタリング

class Bicycle
 attr_reader :size,:chain,:tire_size
 def initialize(args)
   @size = args[:size]
   @chain = args[:chain] || default_chain
   @tire_size = args[:tire_size] || default_tire_size
   post_initialize(args)
 end
 def post_initialize(args)
   nil
 end
 def spares
   {
       tire_size: tire_size,
       chain: chain
   }.merge(local_spares)
 end
 def local_spares
   {}
 end
 def default_chain
   "10-speed"
 end
 def default_tire_size
   raise NotImplementedError,
         "This #{self.class} cannot respond do to:"
 end
end
class RoadBike < Bicycle
 attr_reader :tape_color

 def post_initialize(args)
   @tape_color = args[:tape_color]
 end

 def local_spares
   {tape_color: tape_color}
 end

 def default_tire_size
   "23"
 end
end
class MountainBike < Bicycle
 attr_reader :front_shock,:rear_shock

 def post_initialize(args)
   @front_shock = args[:front_shock]
   @rear_shock = args[:rear_shock]
 end
 def local_spares
   {rear_shock: rear_shock}
 end
 def default_tire_size
   "2.1"
 end
end

テスト

require_relative '4_3_2_refactoring'

RSpec.describe "Bicycleモデル" do
 bike = RoadBike.new(size: "L",tape_color: "red")

 it '#spares' do
   expect(bike.spares)
       .to eq({:chain=>"10-speed", :tape_color=>"red", :tire_size=>"23"})
 end
end


RSpec.describe "Bicycleを継承したテスト用のサブクラス" do
 class Foo < Bicycle
   attr_reader :tape_color

   def post_initialize(args)
     @tape_color = args[:tape_color]
   end
   def local_spares
     {tape_color: tape_color}
   end

   def default_tire_size
     "23"
   end
 end

 it 'default_tire_sizeを定義しないと、NotImplementedErrorが発生する' do
   expect{bike = Foo.new(size: "L",tape_color: "red")}
       .not_to raise_error(NotImplementedError)
 end
 it '#spares' do
   bike = Foo.new(size: "L",tape_color: "red")
   expect(bike.spares)
       .to eq({:chain=>"10-speed", :tape_color=>"red", :tire_size=>"23"})
 end

end

ポイント

では、superを使わないような書き方はどのようにすればいいのでしょうか？
各クラスに共通したメソッドを用意し、そのメソッドをフックメッセージの役割を果たせばよいわけです。
フックメッセージを使うとsuperを使わずに情報を送り、スーパークラスとサブクラスをより疎結合にすることができます。
この例では下記のメソッドがフックメッセージとして役割を果たしました。

 def local_spares
  {}
end

 def post_initialize(args)
  nil
end

注意点！

リファクタリング後のコードが完全な形になっていっているわけではありません。前よりかは良くなったと捉えてください。

 関連する記事

参考文献

オブジェクト指向設計実践ガイド
～Rubyでわかる 進化しつづける柔軟なアプリケーションの育て方

最後に
私がブログを書く目的は、素晴らしい本や、素晴らしい方々の技術記事を知って頂きたいからです。ぜひ、上記の参考文献を見て下さい。（noteなので広告とかは一切ありません。）

現在、株式会社grabssに行くために最後の悪あがきをしています！！
現在の進行状況
この記事は92件目の投稿。目標達成！！！！
目標再設定
20日までに100件目指す！！（92件目）
20日超えたが、寝るまでは20日営業日！！（まだ寝てない、ちょっと寝##峠を超え目が覚めた！）
気持ちは20日！

よろしければ、スキボタン及びサポートお願いします。勉強の励みになります。