いいコード悪いコードまとめ５章低凝集

凝集度とは

凝集度とはモジュール内における、データとロジックの関係性の強さを表す指標。これはクラス、パッケージ、レイヤーなどの場合も同じである。

この凝集度が高ければ変更に強い、望ましい構造と言える。

例えば、以下は低凝集なコードである。
一見、注文クラスの中だから、金額計算や注文管理のメソッドが混ざっていてもよさそうに思えるが、これはよろしくない。

なぜなら、コードが一つの関心事に集中していないため、理解しやすく保守しにくいからだ。このコードでは、金額の計算と注文の管理の両方の機能が混ざっている。

そのため、このコードを理解するには、金額の計算と注文の管理の両方の知識が必要になる。
また、このコードを保守するには、金額の計算と注文の管理の両方のコードを変更する必要もある。

このコードを改善するには、金額の計算と注文の管理の機能を分離すればよい。

例えば、金額の計算はOrderCalculatorクラスに、注文の管理はOrderManagerクラスに移譲することができる。これにより、コードが一つの関心事に集中し、理解しやすく保守しやすくなるわけだ。

class Order
  def initialize(amount, order_id)
    @amount = amount
    @order_id = order_id
  end

  def calculate_amount
    @amount * 1.1
  end

  def manage_order
    # 注文を管理する処理
  end
end

改善したコードは以下の通り。

class Order
  attr_reader :amount, :order_id

  def initialize(amount, order_id)
    @amount = amount
    @order_id = order_id
  end

  def calculate_amount
    order_calculator.calculate_amount(amount)
  end

  private

  def order_calculator
    @order_calculator ||= OrderCalculator.new
  end
end

class OrderCalculator
  def initialize
    @tax_rate = 0.1
  end

  def calculate_amount(amount)
    amount * @tax_rate
  end
end

class OrderManager
  def initialize
    @orders = []
  end

  def add_order(order)
    @orders << order
  end

  def manage_orders
    # 注文を管理する処理
  end
end

CommonやUtilのような共通処理クラスを作っていないか

ログ出力やエラー検出処理などの横断的関心事を共通処理にまとめているか

結果を返すために引数を使用していないか

引数は多すぎないか

メソッドチェインしすぎていないか

仕様変更が生じた場合、呼び出しているすべての箇所の影響を調べる必要が出てくる

呼び出し側は尋ねずに命令しているか

これはifなどで条件分岐するのではなく呼び出す側は命じるだけ、メソッドを実行するだけにしようということ
悪い例

<% if current_user.admin? %>
  <%= current_user.admin_welcome_message %>
<% else %>
  <%= current_user.user_welcome_message %>
<% end %>

良い例

# 良いメソッド名が思いつかなかったが要は呼び出す側で条件分岐とかはしないようにしようということ。
<%= admin_or_regular_user_welcome_message(user) %>

def admin_or_regular_user_welcome_message(user)
  return 'hello, admin' if user.admin?
  return 'hello, regular user' unless user.admin?
end