凝縮度・結合度という尺度から関数のリファクタリングを行う

背景

☝️読んで感化されました。

感化された内容は☝️でもまとめられています。
この記事は上記の内容を読み解いていくだけです。

はじめに

より良いプロダクトにするために色々な設計論や尺度が存在しています。
知っておいて良いと思うのですが、ひきづられない方がよくて、
現状のプロダクトにとって 何が最善なのかはチームで決めていく ことが望ましいと思います。
（本の受け売り）

この記事では

• 凝縮度
• 結合度

の観点から関数のリファクタリングについて考えてみたいと思います。

なぜリファクタリングするのか

Since each refactoring is small, it's less likely to go wrong. The system is kept fully working after each refactoring, reducing the chances that a system can get seriously broken during the restructuring.
Refactoring lowers the cost of enhancements

プロダクトをグロースさせていくためにも日々の小さなメンテナンスは不可欠ですね。

凝縮度と結合度

凝集度と結合度という概念は、コンスタンチンとヨードンが、その共著である「構造化設計」において提案した関数の尺度です
保守作業に伴って品質の低下を招く危険は、構造化の言語であろうと、オブジェクト指向の言語であろうと同じです。

凝縮度

1から順に危険な凝集とされています。

1. 偶発的凝集
2. 論理的凝集
3. 一時的凝集
4. 手順的凝集
5. 通信的凝集
6. 逐次的凝集
7. 機能的凝集

偶発的凝集

関係性のない処理が含まれている

def calc_price(item, price)
  result_price = price * 0.1
  item.price = result_price
  price * 0.1
end

価格の計算と、item.priceに値を与える処理が混ざっている。

論理的凝集

引数によって内部の処理を選択する

def start_deal(item, user_type)
  if user_type == 'business_user'
    item.notify_deal_by_mail
    item.update(status: 'checking_deal')
  else
    item.update(status: 'dealing')
  end
end

1回の呼び出しでシーケンスに処理されるのではなく、選択された機能だけが実行される。
実行側が内部構造を意識してフラグを渡す必要がある。

一時的凝集

ある時点だけ実行する処理が含まれる

def start_deal(item, user)
  if user.first_deal
    PointBack.create(user_id: user.id, point: 1000)
    user.graduate_new_commer
  end

  item.update(status: 'dealing')
end

一度しか行われない処理が含まれると関数の再利用性がさがる

手順的凝集

実行順序に意味のある処理

def create_item(params)
  item = ItemBuilder.new(params).build
  return nil unless item.valid?

  record = ItemRepository.store(item)  
  record
end

処理順番に意味を持ってしまっているため、関数の再利用性が悪い

通信的凝集

あるデータを処理する機能が集まっている

def update_item(item)
  update_a_process(item)
  update_b_process(item)
  update_c_process(item)
end

逐次的凝集

関連性の高い機能がまとまって処理される

def greeting(user)
  user.say_hello
  user.shake_hands
end

機能的凝集

単一機能

def twice(num)
  2 * num
end

凝集度のまとめ

web db pressの記事を書いた sonatard さんは凝集度を以下のようにまとめている

偶発的凝集は避けるべき。
論理的凝集と時間的凝集は一概には言えないが避けるべき。
機能的凝集は理想的。

時間的凝集のリファクタリング

AS-IS

# 時間的凝集は具体的な処理を書くのではなく、極力機能的凝集の関数を実行することに徹するべき
module Tenant
  module V2
    module CreateDisplayUnit
      class ModelBuilder
        def initialize(params, business_user)
          @params = params
          @business_user = business_user
        end

        def build
          # ディスプレイのインスタンスを生成
          display_unit = DisplayUnit.new(name: @params[:display_unit_name],
                                         vendor_code: @params[:display_unit_vendor_code])
          display_unit.do_something

          # ディスプレイ商品のインスタンスを生成
          display_unit_item = DisplayItem.new(
              user_id: @business_user.user_id,
              origin_price: @params[:sell_price],
              size_id: Tenant::Item::DEFAULT_SIZE_ID,
              delivery_method: Fril::DeliveryMethod::DEFAULT_ID,
              open_flag: @params[:open_flag])
          display_unit_item.do_something
          [display_unit, display_unit_item]
        end
      end
    end
  end
end

TO-BE

module Tenant
  module V2
    module CreateDisplayUnit
      class ModelBuilder
        def initialize(params, business_user)
          @params = params
          @business_user = business_user
        end

        def build
          # インスタンス生成のタイミングに処理がまとまっている
          # 順序に意味はない 
          display_unit = build_display_unit
          display_unit_item =build_display_unit_item
          [display_unit, display_unit_item]
        end

      private
     
        def build_display_unit
          display_unit = DisplayUnit.new(name: @params[:display_unit_name], vendor_code: @params[:display_unit_vendor_code])
          display_unit.do_something
          display_unit
        end

        def build_display_unit_item
          display_unit_item = DisplayItem.new(
              user_id: @business_user.user_id,
              origin_price: @params[:sell_price],
              size_id: Tenant::Item::DEFAULT_SIZE_ID,
              delivery_method: Fril::DeliveryMethod::DEFAULT_ID,
              open_flag: @params[:open_flag])
          display_unit_item.do_something
          display_unit_item
        end
      end
    end
  end
end

論理的凝集のリファクタリング

AS-IS

def build_item(params, is_display_item)
  item = Item.new(params)
  item.do_something_a
  item.do_something_b
  item.do_something_c if is_display_item
  item
end

display_item = build_item(params, true)
stock_keeping_unit_item = build_item(params, false)

TO-BE

def build_display_item(params)
  item = Item.new(params)
  item.do_something_a
  item.do_something_b
  item.do_something_c if is_display_item # ディスプレイ商品は必ず「サイズなし」になるようにする
  item
end

def build_stock_keeping_unit_item(params)
  item = Item.new(params)
  item.do_something_a
  item.do_something_b
  item
end

# 呼び出し元が内部実装を知らなくても良い
display_item = build_display_item(params)
stock_keeping_unit_item = build_stock_keeping_unit_item(params)

論理的凝集に関してはDRYの原則に反してもおり、ケースバイケースであるが、
今回のような順序に意味がaあるような場合　sonatard　さんは経験則上では

• 順序が変わる場合はほとんどない
• 順序が変わっても機能しなくなることで検知できる
• 同じようなユースケースが発生した場合にどの内部処理が必要なのかが明確になる

という理由で、関数を分けることを勧めている

この内容はClean Architectureにも登場しており、

反射的に重複を排除するのではなくて、その重複が本物かどうかを見極めるべき

とあるようです。
とは言え、重複する関数を複製するのも管理が大変なので、状況に応じて適宜判断するのが良さそうです。

結合度

1から順に危険な凝集とされています。

1. 内部結合
2. 共通結合
3. 外部結合
4. 制御結合
5. スタンプ結合
6. データ結合

結合はイメージがつきやすいと思うので説明は簡略に行います。

内部結合

他の関数と外部宣言されていない値で結合されている

共通結合

共通のグローバル変数で結合されている

外部結合

public変数で結合

制御結合

呼び出し側が関数の内部の処理に関連するフラグを知った上で、指示することで結合している。（論理的凝集）

スタンプ結合

構造体やクラスの受け渡しで結合

データ結合

スカラー型の受け渡しで結合している

メッセージ結合

関数の実行で結合している。値の受け渡しは存在しない

結合度まとめ

web db pressの記事を書いた sonatard さんは凝集度を以下のようにまとめている

凝集からみる間違ったDRY

（web db pressでこの辺りが熱く書かれているので、気になった方は購入をお勧めします。）

DRYにすることで一時的にコードがスッキリしますが、
機能変更のたびに不要な修正が入ってしまうようなDRYは時間的凝集もしくは論理的凝集がうまく行えていない可能性があります。

(論理的凝集が行われている。)

大きい皿を取り出して、上に乗っている小さい皿が割れない方が安全そうにみえる。
そういう話なのかなと個人的に思いました。

最後に

開発中に仕様が変わったり、機能が拡張されたり。
とソフトウェアは日を追うごとに当初の状態からは陳腐化していくと思います。

現状で最善のプロダクトを目指して、小さな改善を続けることで機能拡張に備え、サービスのグロースに寄与していきたいですね。