【90】【Rails】リファクタリングトレーニング_継承を使う

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

問題のあるコード

下記のコードを見て、危険なコードだ！とすぐに分かりましたか？
また、どのような点が危険かをすぐに説明することが出来ますか？
そして、どのようにリファクタリングしますか？

class Bicycle
 attr_reader :style,:size,:tape_color,:front_shock,:rear_shock

 def initialize(args)
   @style = args[:style]
   @size = args[:size]
   @tape_color = args[:tape_color]
   @front_shock = args[:front_shock]
   @rear_shock = args[:rear_shock]
 end

 def spares
   if style == :road
     {
         chain: "10-speed",
         tire_size: "23",
         tape_color: tape_color
     }
   else
     {
         chain: "10-speed",
         tire_size: "2.1",
         rear_shock: rear_shock
     }
   end
 end
end

テスト

require_relative '4_1_1_before_refactoring'


RSpec.describe "Bicycleモデル" do
 bike = Bicycle.new(
     style: :mountain,
     size: "s",
     front_shock: "Manitou",
     rear_shock: "Fox"
 )

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

改善したい点

 def spares
   if style == :road
     {
         chain: "10-speed",
         tire_size: "23",
         tape_color: tape_color
     }
   else
     {
         chain: "10-speed",
         tire_size: "2.1",
         rear_shock: rear_shock
     }
   end

非常に問題山積みのコードですね。
コードが長くなる
styleが増えれば増えるほど、このコードに追加していく必要があります。
styleが何十、何百となったら？
何十、何百行と長いコードになっていきます。
else句の危険性
また、else以下の箇所も非常に問題があります。
もし、タイポしたらどうなるでしょうか？

    {
         chain: "10-speed",
         tire_size: "2.1",
         rear_shock: rear_shock
     }

という結果が返りますが、これは意図した結果ではありません。このような箇所はいつかはバグの元となります。

リファクタリング

class Bicycle
 attr_reader :size
 def initialize(args = {})
   @size = args[:size]
 end
end
class RoadBike < Bicycle
 attr_reader :tape_color

 def initialize(args)
   @tape_color = args[:tape_color]
   super(args)
 end

 def spares
   {
       chain: "10-speed",
       tire_size: "23",
       tape_color: tape_color
   }
 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
   {
       chain: "10-speed",
       tire_size: "2.1",
       rear_shock: rear_shock
   }
 end
end
​

テスト

require_relative '4_1_2_refactoring'


RSpec.describe "Bicycleモデル" do
 mountain_bike = MountainBike.new(
     size: "s",
     front_shock: "Manitou",
     rear_shock: "Fox"
 )

 it '#spares' do
   expect(mountain_bike.spares)
       .to eq({:chain=>"10-speed", :rear_shock=>"Fox", :tire_size=>"2.1"})
 end
end

 ポイント

クラスを分割し、継承を行うことにしました。
それぞれのクラスに単一の責任が保たれています。
継承する際は、

super(args)

の記入漏れがないようにしましょう。
この点は重要なポイントになります。
詳しくはこの記事で。

継承以外にもコンポジションという選択もあります。

この記事では継承を選択しました。

コンポジションについてはこの記事で

注意点！

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

 関連する記事

参考文献

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

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

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

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