はじめに
Amazon: https://www.amazon.co.jp/dp/4048678841
第6章から第11章まではリファクタリングの具体的なテクニックがまとめられています。
今回は リファクタリング: Rubyエディション
の 第7章 オブジェクト間でのメンバの移動
で紹介されているいくつかのテクニックについて、まとめました。
※ 以下のサンプルコードは書籍で紹介されているコードを参考に、自分で作成したコードを載せています。(著作権を侵害しないように)
テクニック一覧
テクニック詳細
Move Method
メソッドが、他クラスの機能を多く使っていたり、他クラスに何度も利用されたりしていたら、最もよく使われているクラスにメソッドを移動するテクニック
# リファクタリング前
class Employee
def base_salary
if @employee_type.manager?
return 200000
else
return 100000
end
end
end
# リファクタリング後
class EmployeeType
def base_salary
if manager?
return 200000
else
return 100000
end
end
end
メソッドを最もよく使われているクラスに定義することで、コードを読む人間にとって、以下のようなメリットがあります。
- 他クラスを参照しなくて済むので、コードが少なくなる
- 他クラスの実装を見にいかなくてよくなる
コードを追うときに別のクラスを行ったり来たりすると、それだけで大変ですよね。私はクラスを行ったり来たりしている間に、何をしていたのかわからなくなることが多々あります(笑)。そんな私にとっては、とても納得できる内容でした。
Move Field
フィールドが、他クラスでよく使われていたら、最もよく使われているクラスにフィールドを移動するテクニック
# リファクタリング前
class Employee
def initialize(bonus_rate)
@bonus_rate = bonus_rate
end
def bonus
@bonus_rate * base_salary
end
end
# リファクタリング後
class Employee
def initialize(employee_type)
@employee_type = employee_type
end
def bonus(base_salary)
@employee_type.bonus_rate * base_salary
end
end
class EmployeeType
attr_accessor :bonus_rate
def initialize(bonus_rate)
@bonus_rate = bonus_rate
end
end
※ 注意: 上記サンプルは、 bonus_rate
フィールドが Employee
クラスではなく EmployeeType
クラスでよく使われるようになっていることを前提としています
フィールドに関しても、最もよく使われているクラスに定義することで、コードを読む人間にとって、以下のようなメリットがあります。
- 他クラスを参照しなくて済むので、コードが少なくなる
- 他クラスの実装を見にいかなくてよくなる
コード量が少なくなれば、それだけ人間が読むコードの量も減りますし、クラスを行ったり来たりしなくてすむようになるので、結果的に効率的な開発に繋がるのだろうと思いました。
Extract Class
1つのクラスが多くの役割を担ってしまっているとき、クラスの役割を分割し、新しく別のクラスとして切り出すテクニック
# リファクタリング前
class Employee
attr_reader :name
attr_accessor :health_insurance_fee, :employment_insurance_fee
def sum_all_insurance_fee
health_insurance_fee + employment_insurance_fee
end
end
# リファクタリング後
class Employee
attr_reader :name
attr_accessor :insurance
def initialize
@insurance = Insurance.new
end
end
class Insurance
attr_accessor :health_insurance_fee, :employment_insurance_fee
def sum_all_insurance_fee
health_insurance_fee + employment_insurance_fee
end
end
私はクラスを分けるほどでもないという思いから、既存のクラスにどんどん機能を追加していってしまうことがあります。結果、クラスが担う役割が多くなってしまい、どんどん複雑な構造になってしまいます。
複雑なコードを読むのはそれだけで負荷になるので、効率よく開発するためにはクラスをシンプルな構造に保つ必要があります。だからこそ、クラスの役割を分割して新たなクラスとして切り出す必要があることに納得しました。
Inline Class
クラスがあまり役割を担っていないとき、すべての機能を他のクラスに移動して、クラスを削除するテクニック
# リファクタリング前
class Employee
attr_reader :name
attr_accessor :telephone
def initialize
@telephone = Telephone.new
end
end
class Telephone
attr_accessor :area_code, :city_code, :association_number
def telephone_number
area_code + city_code + association_number
end
end
# リファクタリング後
class Employee
attr_reader :name
attr_accessor :area_code, :city_code, :association_number
def telephone_number
area_code + city_code + association_number
end
end
Inline Class は Extract Class と全く逆のことをしています。
クラスがほとんど役割を果たしていないにも関わらず、そのクラスを残したままにしてしまっていると、コードを読むときにクラスを行ったり来たりしないといけなくなります。
コードを読むときに、役割を果たしていないクラスを、行ったり来たりしてしまうのはもったいないように思えます。そのときはクラスを削除して、適切なクラスにフィールドやメソッドを移動するということに納得がいきました。
Hide Delegate
クライアントがオブジェクト内の委譲クラスを呼び出しているとき、委譲クラスをカプセル化するテクニック
# リファクタリング前
class Employee
attr_accessor :department
def initialize(department)
@department = department
end
end
class Department
attr_reader :boss
def initialize(boss)
@boss = boss
end
end
department = Department.new("Bob")
person = Person.new(department)
person.department.boss # => "Bob"
# リファクタリング後
require 'forwardable'
class Employee
extend Forwardable
def_delegator :@department, :manager
def initialize(department)
@department = department
end
end
class Department
attr_reader :boss
def initialize(boss)
@boss = boss
end
end
department = Department.new("Bob")
person = Person.new(department)
person.boss # => "Bob"
オブジェクト指向三大要素のひとつであるカプセル化を実現するためのテクニックです。
リファクタリング前の実装だと、 person.dapartment.boss
という呼び出しになり、 Person
クラスが Department
クラスに manager
フィールドがあることを知ってしまっていることになります。
これでは Person
クラスと Department
クラスが密結合になってしまいます。密結合になってしまうと、変更を加える際に多くの箇所を修正しなければならなくなる可能性が高まります。結果、修正を加えにくくなり、効率よく開発ができなくなることにつながりかねません。
だから、委譲クラスは Ruby 標準ライブラリに含まれている Forwardable
モジュールなどを使ってカプセル化しましょうということなのだと思いました。
Remove Middle Man
クラスが単純な委譲しかやっていないとき、クライアントに委譲クラスを直接呼び出させるようにするテクニック
# リファクタリング前
require 'forwardable'
class Employee
extend Forwardable
def_delegator :@department, :manager
def initialize(department)
@department = department
end
end
class Department
attr_reader :boss
def initialize(boss)
@boss = boss
end
end
department = Department.new("Bob")
person = Person.new(department)
person.boss # => "Bob"
# リファクタリング後
class Employee
attr_accessor :department
def initialize(department)
@department = department
end
end
class Department
attr_reader :boss
def initialize(boss)
@boss = boss
end
end
department = Department.new("Bob")
person = Person.new(department)
person.department.boss # => "Bob"
Remove Middle Man は Hide Delegate とはまったく逆のことをしています。
Hide Delegate にはメリットだけでなく、委譲クラスにメンバが増える度に委譲メソッドを追加しなければならないという代償があります。
委譲クラスにメンバが増えすぎると、委譲メソッドを追加するのが苦痛になってきます。
もしクラスに単なる委譲メソッドしかない場合には、クライアントから直接委譲クラスを呼び出した方が、開発者の負担が少なくなって、開発スピードを維持できるのだろうと思いました。
考察
Extract Class を推奨する一方で Inline Class を推奨していたり、 Hide Delegate を推奨する一方で Remove Middle Man を推奨していたりと、どちらが正しいというわけではなく、状況に合わせて適切だと思える方の実装に寄せるのが大切なのではないかと思いました。
状況を判断するときは人によって主観的な見解もあると思うので、チーム開発の場合はメンバーで認識を合わせた上でリファクタリングするべきだなと思いました。
リファクタリングの良さは、仕様やコードの特徴が変われば、それに合わせて適切な(メンテナンスしやすい)コードに変えていけるということだと思います。
だからこそ最も避けたいのは、修正のしづらさを分かっていながら何も対策を打たずに進み、最終的にどこを修正すれば良いかわからなくなり、開発スピードが低くなってしまうことなのかなと思いました。