論路的凝集との向き合い方

12290 ワード
リファクタリング 継承 凝集度 リファクタリング テキストリンク

概要

  • この記事は「論理的凝集」に着目して、開発生産性の高いコードを書くために気をつけるべきことのメモ
  • 良かれと思って処理の共通化を行い、コードの量こそ減らしたものの機能追加に弱く、開発生産性の低いコードを書いてしまうことを避けるためのヒント

凝集度

https://gihyo.jp/magazine/wdpress/archive/2022/vol127
https://fortee.jp/object-oriented-conference-2020/proposal/a826b6c6-167c-4c5c-bfc7-52bb8bc22ec1

  • 凝集度について載っているのでこちらで理解すると良い
  • 本を買って読ませていただいた内容を元にこの記事を書いています。本記事では凝集度の説明については割愛
  • この記事では 「関数」 に対する役割の尺度という意味で凝集度を扱う

論理的凝集

論理的凝集とは

  • 関連のない処理をフラグ等で切り替える関数
  • 良くないコードであるということに気づくことが難しい
    • 処理の共通化がされている

論理的凝集を使っている関数の例

def exec(usecase)
  a()
  b()

  if usercase == "usecase1"
    c() 
  end

  d()
end

引数の値の種類や、関数が増えると・・・

  • 仕様が追加されると処理が複雑化していく
  • 可読性が低く、保守が難しいコードになっていく
def exec
  a()
  b()

  if usercase == "usecase1"
    c()  
  end

  d()

  if usercase == "usecase1"
    e()
  end

  if usercase == "usecase3"
    f()
  end
end

改善方法

  • ユースケースごとに関数分けることで、単一責任の関数となる
    • フラグを削除し関数を分ける
    • 関数のユースケースが一つになり、シンプルになる
    • 一つの関数を変更した際に、他の関数への影響がない
    • 単一責任の原則 
# 分割した関数
def usercase1
  a()
  b()
  c()
  d()
end

def usercase2
  a()
  b()
  d()
end

関数増加、ユースケースの増加にも対応しやすい

  • シンプルで理解しやすい
  • 処理の変更の影響が該当の関数のみとなる
  • a() b()の重複があるが、共通化と論理的凝集の回避はトレードオフと考える
    • 論理的凝集を回避した方が良いことが多い
def usercase1
  a()
  b()
  c()
  d()
  e()
end

def usercase2
  a()
  b()
  d()
end 

def usercase3
  a()
  b()
  d()
  f()
end

継承による隠された論理的凝集

  • 親クラスにて子クラスの処理の共通化が行われている
  • 一見条件分岐されるようなフラグもなく、論理的凝集ではないように見えない

例

class Base
  def exec()
    a()
    b()
    usercase()
    d()
  end

  # オーバーライド用関数を定義
  def usercase
  end
end

class Usercase1 < Base
  # オーバーライド
  def usercase
    c()
  end
end

class Usercase2 < Base
  # オーバーライドしない
end

ユースケースや関数が増えていくと・・・

  • どんどん複雑になっていく・・・（無理やり複雑化させている節はあるが）
  • もし a の処理が usecase1 だけの不要になったら・・・
class Base
  def exec()
    a()
    b()
    usercase()
    d()
    usercase_additional()
  end

  # オーバーライド用関数を定義
  def usercase
  end

  # オーバーライド用関数２つ目を定義・・・
  def usercase_additional
  end
end

class Usercase1 < Base
  # オーバーライド
  def usercase
    c()
  end

  def usercase_additional
    e()
  end
end

class Usercase2 < Base
  # オーバーライドしない
end

class Usercase3 < Base
  # オーバーライド
  def usercase_additional
    f()
  end
end

改善方法

  • 継承をやめる。親クラスによる処理の共通化をしない
  • インターフェースもしくは抽象クラスを利用する程度にする（必須ではない）
    • 下記はrubyのため擬似的なインターフェースを設定している
class Interface
  def exec 
    raise NotImplementedError # 擬似的なインターフェース
  end
end

class Usercase1 < Interface
  def exec
    a()
    b()
    c()
    d()
  end
end

class Usercase2 < Interface
  def exec 
    a()
    b()
    d()
  end
end

仕様が追加された場合も・・・

  • 関数、ユースケースが増えた場合もシンプルな構造を保つことができる
class Interface
  def exec 
    raise NotImplementedError # 擬似的なインターフェース
  end
end

class Usercase1 < Interface
  def exec
    a()
    b()
    c()
    d()
    e()
  end
end

class Usercase2 < Interface
  def exec 
    a()
    b()
    d()
  end
end

class Usercase3 < Interface
  def exec 
    a()
    b()
    d()
    f()
  end
end