Zenn
💡

合流・分岐パターンをやめよう

2024/05/02に公開
プログラミング
tech

私が勝手に「合流・分岐パターン」と呼んでいる、プログラミングのアンチパターンがあります。一見キレイなコードに見えるため、リファクタリング等で追加されることすらあります。プログラミング言語に関わらず存在します。

合流・分岐パターン

このアンチパターンは次のように言うことができます。

  1. 処理が発生するきっかけが異なる複数の場所からある一つの関数を呼び出している (合流)
  2. そのきっかけを識別するために関数に引数を渡している
  3. 関数内で引数に応じて異なる処理を行っている (分岐)

サンプルコード

このアンチパターンは次のようなコードです。異なる２つのボタンを押したときに、onTapButton が呼ばれ、それぞれのボタンに応じた処理を行います。

ここで、onTapButton が 合流 で、switch (type) が 分岐 です。

function onTapButton(type) {
  switch (type) {
    case "delete":
      deleteItem()
      break
    case "add":
      addItem()
      break
  }
}

return <>
  <button onClick={() => onTapButton("delete")>Delete</button>
  <button onClick={() => onTapButton("add")>Add</button>
</>

修正後のコード

合流と分岐をなくしましょう。今回のコードは単純なので、単に switch-case で呼んでいた各関数を直接呼び出すだけでよいです。

return <>
  <button onClick={() => deleteItem()>Delete</button>
  <button onClick={() => addItem()>Add</button>
</>

合流・分岐パターンの何が悪いか

  • その後の変更の影響範囲が広くなる
  • もし合流したコードが複雑な場合、コードを理解しない人がそれを使うのを避けたコードを書く

このまま開発を続けるとどうなるか

例: 機能追加

さて、このコードの未来を見てみましょう。まず、Delete, Add ボタンを押したときにローディングのグルグルを表示したいとします。これは簡単です。onTapButton に統一しているため、一箇所に書くだけで済みます。むしろ、あなたはこのコードの共通化をしたくて onTapButton を追加したのではありませんか？

async function onTapButton(type) {
  showLoading(true)

  switch (type) {
    case "delete":
      await deleteItem()
      break
    case "add":
      await addItem()
      break
  }

  showLoading(false)
}

さぁ、どんどんボタンを追加していこうという気持ちになります。悪いことなどなさそうです。

半年後

あなたは様々な理由で、このプロジェクトから半年ほど離れていました。ピカピカのコードがいまどうなっているか見てみましょう。

async function onTapButton(type) {
  // update と back-navigation は画面遷移するのでローディングを表示しない
  showLoading(type !== "update" && type !== "back-navigation")

  switch (type) {
    case "delete":
      await deleteItem()
      break
    case "add":
      await addItem()
      break
    case "rename":
      await renameItem()
      break
    case "update":
      await udpateItem()
      break
    case "back-navigation":
      await backNavigation()
      break
  }

  // 戻るボタンを押したときはイベントを送信しない
  if (type !== "back-navigation") {
    analytics.sendEvent("tap-item-button", type)
  }
  showLoading(false)
}

新たに３つのボタンが追加されたようです。また、ボタンを押したときにはアクセス解析にイベントを送るようになったようです。しかし、ボタンによっては、ローディングやイベント送信が必要ないため、type を見て動作を変えているようです。

問題点

ボタンを押したときに何が起こるのか一目では分からなくなってしまいました。ボタンによっては必要のない関数があり、もともとの処理の共通化という目的が破綻しています。

なぜこうなったのか

あなたの後任のエンジニアは、共通化された onTapButton を見て、何か共通の処理を行う事情があったのだと察しました。当然、既存のボタンの動作にバグがでていはいけないので、onTapButton を削除することはできません。その結果、既存のボタンの動作を変えないように、注意深くそれをバイパスするコードを書くことになりました。

改善

理想のコードはどんな見た目でしょうか？onTapButton を削除し、ボタンは直接ボタンごとの関数を呼び出すようにします。showLoading などは下記のように各関数に書きます。

async function deleteItem() {
  showLoading(true)
  ...
  showLoading(false)
  analytics.sendEvent("tap-item-button", "delete")
}

async function addItem() {
  showLoading(true)
  ...
  showLoading(false)
  analytics.sendEvent("tap-item-button", "add")
}

async function renameItem() {
  showLoading(true)
  ...
  showLoading(false)
  analytics.sendEvent("tap-item-button", "rename")
}

async function updateItem() {
  ...
  analytics.sendEvent("tap-item-button", "update")
}

async function backNavigation() {
  ...
}

なぜこれでよいのか

キレイには感じないですか？これでいいんです。「同じようなコードが繰り返し書かれている」「リファクタリングしなければ」といった心の声は無視しなければいけません。

なぜなら、それぞれが「異なるボタン」だという前提があるからです。「異なるボタン」に「異なる動作」があることは、プログラミング言語やコードの綺麗さとはまったく関係ありません。ユーザーやビジネス、上司やデザイナーの意見が関係しています。現実の複雑さを反映している部分ということになります。

誰かが「このボタンを押したときだけはローディングのグルグルは違うデザインにしたい」と言い出すのは、明日かもしれません。

合流・分岐パターンをやめ、それぞれの関数内で必要な処理をすべて行うように書き直したことで、一つのボタンの処理の見通しはよくなり、特定のボタンの動作を変更することも簡単になりました。

現実では

しかし、実際のプロジェクトにおいて、このようなコードを変更することはバグを生み出すリスクが高く、既存の動作も確認しないといけないのでなかなか手を付けることができません。一個づつ切り出していくにも、エイヤでいっぺんに書き直すにも、余計な手間がかかります。

ではどうすべきか？最初から合流・分岐パターンになっていないか気をつけることです。このパターンはうっかり生み出すこともあります。次のようなことに気をつけましょう。

アドバイス

「関係のないものは、関係のないままにしておく」

  • 処理が似てても、発生するきっかけが別ならそれを分けておこう
  • 一見繰り返しに見えるコードを恐れないこと
  • 共通の処理がある場合は、共通の処理を関数化し、それを利用できるようにしよう

ハイレベルアドバイス

どうしても繰り返しが気になる場合は、クロージャを受け取る関数で共通化することもできます。例えば先程のコードでは、showLoading の繰り返しが気になるので、次のように書くかもしれません。しかし、これぐらい簡単なコードであれば、直接 showLoading を呼び出すことをおすすめします。withLoading が新たな合流・分岐パターンにならないように気をつけましょう。このようなコードが役に立つのは、必ず後始末をしないといけない処理がある場合などでしょう。言語によっては RAII (Resource Acquisition Is Initialization) や C# の using を使っての解決も検討しましょう。

async function withLoading(fn) {
  showLoading(true)
  await fn()
  showLoading(false)
}

async function deleteItem() {
  await withLoading(async () => ...)
  analytics.sendEvent("tap-item-button", "delete")
}

async function addItem() {
  await withLoading(async () => ...)
  analytics.sendEvent("tap-item-button", "add")
}

Redux はアンチパターンか？

ここまで読んで、おやおやと思った人もいるでしょう。Redux の Action がこのパターンに該当しますね。Redux のような状態管理ライブラリでは、合流・分岐パターンを活用しています。Redux を使うべきではないのでしょうか？

Redux を使って構築された偉大なソフトウェアは数多くあり、非常に洗練されたコードベースを持ちます。しかし、非常に恐れ多いですが、私としては Action, Reducer は合流・分岐パターンの一種であり、アンチパターンであると考えます。しかし、これは Redux のデザイン上の選択であり、この設計によって状態管理に関する様々な強力な機能を持つことができます。その長所・短所をひとくくりにしてアンチパターンとは言えません。長所を活用できる人であれば、やめる必要はないでしょう。しかしどうしても、ステートを一つにまとめる以上、関心の分離が難しくなります。その分、注意深くなる必要があります。私としては、「関係のないものは、関係のないままにしておく」を原則とし、それを実践しやすいライブラリの選定をおすすめします。

私個人としては、必要な分だけ共通化されたデータストアを持ち、Derived State を活用するという方向性がよいのではと考えています。React では Recoil や Jotai を使うことができます。もしくは、React Query のような、状態管理の問題をキャッシュ戦略の問題と捉えるようなパラダイムが良いのではないでしょうか。それぞれの関数内で必要なデータフェッチなどの処理を呼び出すが、その裏で処理の最適化を行うというような考え方です。このようなコードはハイコンテキストになりづらく、Copilot などの AI による予測もしやすいと思われます。賢く短いコードよりも、愚直なコードを書けることでより効率的になります。

おわりに

脱線しまくりましたが、「合流・分岐パターン」の紹介でした。なにかの本にこういうことが書いてあったという話も聞くので、より広く知られた名称があれば教えてください。

Discussion