【96】【Rails】リファクタリングトレーニング_OpenStruct

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

改善の余地があるコード

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

class Bicycle
 attr_reader :size, :parts

 def initialize(args = {})
   @size = args[:size]
   @parts = args[:parts]
 end
 def spares
   parts.spares
 end
end

class Parts
 attr_reader :parts

 def initialize(parts)
   @parts = parts
 end

 def spares
   parts.select{|part| part.needs_spare}
 end
end

class Part
 attr_reader :name, :description, :needs_spare

 def initialize(args)
   @name = args[:name]
   @description = args[:description]
   @needs_spare = args.fetch(:needs_spare,true)
 end
end

module PartsFactory
 def self.build(config,part_class = Part,parts_class = Parts)
   parts_class.new(
       config.collect{ |part_config|
         part_class.new(
             name: part_config[0],
             description: part_config[1],
             needs_spare: part_config.fetch(2,true)
         )
       }
   )
 end
end

テスト

require_relative '5_4_1_before_refactoring'


RSpec.describe "Bicycleモデル" do
 mountain_config =
     [
         ["chain","10-speed"],
         ["tire_size","Manitou",false ],
         ["rear_shock","Fox"]
     ]
 mountain_bike =
     Bicycle.new(
         size: "L",
         parts: PartsFactory.build(mountain_config)
     )


 spares = mountain_bike.spares.map do |part|
   {"#{part.name}".to_sym =>"#{part.description}"}
 end

 it '#spares' do
   expect(spares).to eq [{:chain=>"10-speed"}, {:rear_shock=>"Fox"}]
 end
 it '#parts.size' do
   expect{mountain_bike.parts.size}
       .to raise_error(NoMethodError)
 end
end

改善したい点

 def self.build(config,part_class = Part,parts_class = Parts)
   parts_class.new(
       config.collect{ |part_config|
         part_class.new(
             name: part_config[0],
             description: part_config[1],
             needs_spare: part_config.fetch(2,true)
         )
       }
   )
 end

上記のメソッドは入れ子になっていて、Partsインスタンスの作成とPartインスタンスの作成を行いっています。
buildメソッドの責任が複数になっています。
PartsインスタンスとPartインスタンスの作成を別にさせ、buildメソッドの責任を単一にしたいですね。。

module PartsFactory
 def self.build(config,parts_class = Parts,part_class = Part)
   parts_class.new(
       config.collect do |part_config|
         build_part(part_config,part_class)
       end
   )
 end
 def self.build_part(part_config,part_class)
     part_class.new(
         name: part_config[0],
         description: part_config[1],
         needs_spare: part_config.fetch(2,true)
     )
 end
end

メソッドを分離させました。
Partクラスを引数で受けとって、build_partクラスに渡すという点が少し気になります。
OpenStructを使うと劇的に改善できます。

リファクタリング

class Bicycle
 attr_reader :size, :parts

 def initialize(args = {})
   @size = args[:size]
   @parts = args[:parts]
 end
 def spares
   parts.spares
 end
end

class Parts
 attr_reader :parts

 def initialize(parts)
   @parts = parts
 end

 def spares
   parts.select{|part| part.needs_spare}
 end
end

require 'ostruct'
module PartsFactory
 def self.build(config,parts_class = Parts)
   parts_class.new(
       config.collect{|part_config|
         create_part(part_config)
       }
   )
 end
 def self.create_part(part_config)
   OpenStruct.new(
       name: part_config[0],
       description: part_config[1],
       needs_spare: part_config.fetch(2,true)
   )
 end
end

テスト

require_relative '5_4_2_refactoring'


RSpec.describe "Bicycleモデル" do
 mountain_config =
     [
         ["chain","10-speed"],
         ["tire_size","Manitou",false ],
         ["rear_shock","Fox"]
     ]
 mountain_bike =
     Bicycle.new(
         size: "L",
         parts: PartsFactory.build(mountain_config)
     )


 spares = mountain_bike.spares.map do |part|
   {"#{part.name}".to_sym =>"#{part.description}"}
 end

 it '#spares' do
   expect(spares).to eq [{:chain=>"10-speed"}, {:rear_shock=>"Fox"}]
 end
 it '#parts.size' do
   expect{mountain_bike.parts.size}
       .to raise_error(NoMethodError)
 end
end

 ポイント

OpenStructを使いました。OpenStructとは要素を動的に追加・削除できる構造体を提供するクラスです。
partクラスの生成はこのOpenStructを使って行うことにしました。

def self.create_part(part_config)
   OpenStruct.new(
       name: part_config[0],
       description: part_config[1],
       needs_spare: part_config.fetch(2,true)
   )
 end

class Part
 attr_reader :name, :description, :needs_spare

 def initialize(args)
   @name = args[:name]
   @description = args[:description]
   @needs_spare = args.fetch(:needs_spare,true)
 end
end

Partクラスと完全に重複しています。
よって、Partクラスをまるごと削除することができました。

注意点！

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

 関連する記事

参考文献

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

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

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

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