リファクタリング: Ruby エディション第7章 #Ruby

はじめに

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 を推奨していたりと、どちらが正しいというわけではなく、状況に合わせて適切だと思える方の実装に寄せるのが大切なのではないかと思いました。

状況を判断するときは人によって主観的な見解もあると思うので、チーム開発の場合はメンバーで認識を合わせた上でリファクタリングするべきだなと思いました。

リファクタリングの良さは、仕様やコードの特徴が変われば、それに合わせて適切な（メンテナンスしやすい）コードに変えていけるということだと思います。

だからこそ最も避けたいのは、修正のしづらさを分かっていながら何も対策を打たずに進み、最終的にどこを修正すれば良いかわからなくなり、開発スピードが低くなってしまうことなのかなと思いました。

リファクタリング: Ruby エディション 第7章