📑

【1日1zenn - day22】リファクタリングでやるべきこと

に公開
オブジェクト指向
リファクタリング
tech

諸々落ち着いてきたので執筆を再開します。

えげつない複雑度の処理をリファクタリングする必要が出てきました。

改めて現場で役立つシステム設計の原則を読んで、意識しようと思ったことをメモっていきます。

前提、課題感

ざっくり言うとECサイトみたいなサービスを開発しています。

商品には色やサイズなどのバリエーションがあったり、
商品に追加でつけられるオプション商品があったりします。
オプション商品に対して、もう一階層ネストしたオプション商品もあります。

これらの商品について、
カート追加時と注文ボタン押下時で、商品情報に変化がないかチェックしています。

在庫が切れて注文できなくなったり、セールが終わって価格が変更されていたり、商品の購入受付を停止されていたり、送料無料期間が終了していたり、様々な変更をチェックする必要があります。
しかもその中で、必須で付ける必要があるオプション商品が売り切れていたら親商品も売り切れ扱いにするなど、他階層の影響を伝播させる必要があったりもします。

商品種別が複数あり、階層も3階層あったりする中で、複数のチェックを行い、場合によっては影響を伝播させる。
エンジニアの方ならこのしんどさ、多分伝わりますよね。

しかもこれを結構フロントでやってるんです。バックエンドも相当な処理を行った上で値を返していますが、フロントだとエラーに応じてどんなUIを出すかによって処理を変えていたりして、UIとロジックが結合しているのも辛い。

商品種別が増えるたびに増築されたコードは、今や九龍城です。
どう一掃するか考えるヒントを集めていきます。

読書メモ帳

第1章 小さくまとめてわかりやすくする

狭い関心ごとに特化させたクラスにする

  • 送料クラスでは送料計算以外を行わない
  • 特定の関心ごとに特化した小さなクラスを作る
  • 割引率や特別価格の計算などは分割する
  • 業務の用語と直接対応するドメインオブジェクトを用意することがオブジェクト指向らしい設計
  • 業務の関心事の単位をそのままプログラミング単位としてクラスで表現する
  • 分析で発見した業務の構造とプログラムの構造が一致していれば、変更が楽で安全になる

感想

  • 商品種別ごとの分岐をどれくらい抽象化させるかが難しい
    • 抽象化させずに全て具体的に分割する場合、膨大すぎるコードに膨れ上がる
    • 商品種別粒度で分割した場合、「セール終了したか」など固有のチェックが様々な関数にまたがることになる
    • 逆にセール終了チェックなど関心ごと単位で分割すると、商品種別が凝縮した複雑なコードになる
    • これにどう折り合いをつけるか
    • この本的には「目的」に特化させるのを推奨していそう
      • 引数としてどんな商品種別でも受け入れられる最小限の型に抽象化する？

プログラムの基本はデータを使った判断、加工、計算の3つ

  • なんとなくメモ。こういう整理好き。

コレクション型を扱うコードの整理

  • コレクションオブジェクト
    • 「顧客一覧」の専用クラスを、独自に宣言
    • 顧客一覧を操作するロジックはすべて1つのクラスに集める
    • 使う側はコレクションオブジェクトのメソッドを呼ぶだけにする
    • 操作対象のデータを持つクラスにロジックを集めることがオブジェクト指向設計の基本

感想

  • となるとやっぱ商品種別ごとに似たような処理を用意する？
    • 商品種別ごとにコレクションオブジェクトを作りつつ、その中で「セール終了したか」の抽象的な関数を呼び出すとかがいいかも
    • itemIdとisSaleだけ受け取れるようにして、商品種別問わず対象のチェック処理を行えるようにするとか。

第2章 場合分けのロジックを整理する

前置き

  • 区分や分類は、対象ごとに異なる業務ルールを適用するために使われる
  • 複数な区分体系を組み合わせて判断する業務ロジックだと、if文の分岐がどんどん複雑になる
  • まじでそう。

判断や処理のロジックをメソッドに独立させる

  • 判断ロジックと分岐後のロジックをそれぞれメソッドに抽出する
  • if文は、メソッドを呼び出すだけにする
  • 処理の詳細をメソッドに隠すことで、条件分析の構造だけに集中して考える

早期リターンさせる

これはそう。基本やってるけど、やれてないとこもあったなぁ。

複文は単文に分ける

  • elseをなくして、if(isChild()) return childFeeみたいな短文を列挙するだけにする
  • if文同士を疎結合にして、それぞれ順番を入れ替えてもうまく動作するようにする

区分ごとのロジックを別クラスに分ける

  • これはデメリットとして、区分ごとにクラスを分けるとクラスを使う側が様々な箇所でif文による判定を入れることになる
    • それを避けるために、インターフェースを使って異なるクラスを同じ方として扱うことで、そもそもどのような区分が存在するのか知らなくていいようにする
    • そのようなポリモーフィズムを使うと、区分の一覧がわかりにくいと言う問題が発生する
      • どういう区分体系であるか一覧性に欠けると、コードの見通しが悪くなり、変更がやりにくくなる
  • 区分ごとのロジックをenumを使って表現することで回避できる

GPTに出させたコード

以下みたいに分割&抽象化するのは確かにちょっとマシになりそう。
ただ区分多いから結局複雑な気がするのがなんとも言えない。。

型定義

enum ProductKind {
  NORMAL = "NORMAL",
  SUBSCRIPTION = "SUBSCRIPTION",
  TICKET = "TICKET",
  DIGITAL = "DIGITAL",
  RESERVATION = "RESERVATION",
  COLD = "COLD",
  STORE_PICKUP = "STORE_PICKUP",
}

type Product = {
  kind: ProductKind;
  basePrice: number;

  saleEndAt?: Date;
  salePrice?: number;

  firstTimePrice?: number;
  stock?: number;
  eventDate?: Date;
  destination?: "JP" | "OVERSEAS";
  reservedAt?: Date;
  temperature?: "CHILLED" | "FROZEN";
  pickupStartAt?: Date;
  pickupEndAt?: Date;
};

整合性チェック

type Rule = {
  validate: (p: Product, ctx: { now: Date }) => { message: string }[];
  price: (p: Product, ctx: { now: Date }) => number;
};

const rules: Record<ProductKind, Rule> = {
  [ProductKind.NORMAL]: {
    validate: (p, ctx) => {
      if (!p.saleEndAt || !p.salePrice) return [];
      if (p.saleEndAt <= ctx.now) return [{ message: "セール終了" }];
      return [];
    },
    price: (p, ctx) =>
      p.saleEndAt && p.salePrice && p.saleEndAt > ctx.now
        ? p.salePrice
        : p.basePrice,
  },

  [ProductKind.SUBSCRIPTION]: {
    validate: () => [],
    price: (p) => p.basePrice,
  },

  [ProductKind.TICKET]: {
    validate: (p, ctx) => {
      if (p.stock != null && p.stock <= 0) return [{ message: "在庫なし" }];
      if (p.eventDate && p.eventDate <= ctx.now) return [{ message: "イベント終了" }];
      return [];
    },
    price: (p) => p.basePrice,
  },

  [ProductKind.DIGITAL]: {
    validate: (p) =>
      p.destination === "OVERSEAS" ? [{ message: "海外販売不可" }] : [],
    price: (p) => p.basePrice,
  },

  [ProductKind.RESERVATION]: {
    validate: (p, ctx) =>
      p.reservedAt && p.reservedAt <= ctx.now
        ? [{ message: "予約日時が過去" }]
        : [],
    price: (p) => p.basePrice,
  },

  [ProductKind.COLD]: {
    validate: () => [],
    price: (p) => (p.temperature === "FROZEN" ? p.basePrice + 200 : p.basePrice),
  },

  [ProductKind.STORE_PICKUP]: {
    validate: (p, ctx) => {
      if (p.pickupStartAt && p.pickupEndAt) {
        if (ctx.now < p.pickupStartAt || ctx.now > p.pickupEndAt) {
          return [{ message: "受取期間外" }];
        }
      }
      return [];
    },
    price: (p) => p.basePrice,
  },
};

呼び出し側

function validateAndPrice(p: Product, now: Date) {
  const rule = rules[p.kind];     // enumなので安全
  const errors = rule.validate(p, { now });
  if (errors.length) return { ok: false as const, errors };

  return { ok: true as const, price: rule.price(p, { now }) };
}

第3章 業務ロジックをわかりやすく整理する

前置き

  • データとロジックを別のクラスに分けることがわかりにくさを生む
    • プレゼンテーション層、アプリケーション層、データ層を分ける三層アーキテクチャを採用しても、データクラスと機能クラスを分ける手続き型の設計のままでは、業務ロジックが入り組んだときに複雑になる。
    • 同じ業務ロジックがあちこちに重複して書かれたり、どこに業務ロジックが書いてあるか見通しが悪くなったりする
  • そもそも、データクラスと機能クラスに分ける設計は、クラス本来の使い方ではない
    • クラスは、データとロジックを1つのプログラミング単位にまとめる仕組み
    • データをインスタンス変数として持ち、そのインスタンス変数を使った判断、加工、計算のロジックをメソッドに書くのが、クラスの本来の使い方

業務ロジックをわかりやすく整理する基本のアプローチ

方針は、以下の2つ

  • データとロジックを一体にして、業務ロジックを整理する
  • 三層のそれぞれの関心事と業務ロジックの分離を徹底する

データとロジックを一体にして、業務ロジックを整理する

クラス設計で大切な事は、使う側のクラスのコードがシンプルになるように設計すること

  • データを持つクラスに業務ロジックを集める
  • 使う側のクラスにロジックを書き始めたら、設計を見直す
  • クラスが肥大化したら小さく分ける
  • 長いメソッドを短いメソッドとして独立させ、データを持つクラスに移動させる
  • クラスが肥大化し始めたら、関連性の強いいデータとロジックを抜き出して、新しいクラスに分けて凝縮度を上げる
    • 切っても切れない関係のデータとロジックを1つのクラスにまとめる
  • 長いパッケージ名をつけたくなったら、パッケージを階層にして、一つ一つのパッケージ名を短くする

三層のそれぞれの関心事と業務ロジックの分離を徹底する

  • 注文という大きなドメインについて、「商品」「数量」「金額」「納期」「届け先」「請求先」等の単位で分けながらドメインオブジェクトを作っていく
  • 業務アプリケーションでは、参照関係は基本的に時間軸に沿った関係になる
  • 時間軸に沿ってどの内容を知っていても良いか、逆に知っていてはいけないかを整理して改善を続ける

第4章 ドメインモデルの考え方で設計する

  • 「どういう場合に具体的にどうするか」という決め事が、複雑に絡み合ったものが業務ロジック
  • 業務の関心事をヒト、モノ、コトの3つに分類する
  • 特にコトに着目すると、ヒト、モノの関係を時間軸に沿って発散せず整理しやすい
  • コトを軸に、ヒトやモノをドメインとして分割する
  • 手続き型の場合は、if文の入れ子構造によって必要な条件判断と分析を実行する
  • オブジェクト指向の場合は、判断のもとになるデータとロジックごとにオブジェクトを生成し、判断結果によって何をすべきかの情報も持つ
    • 数量を持ったオブジェクトに、その数量が妥当かどうかの判断も、妥当でなかった場合に何を返すかという業務知識も持たせる

締め

ここから先はさらに抽象度高く、ゼロイチでプロダクトを組み上げる粒度の解説になってきたので一旦締めます。

やろうと思ってること

  • 整合性チェックを"コト"粒度に分割し、時間軸に沿って整理する
  • 商品種別ごとに分割し、それぞれの種別の中で整合性チェックを行う
  • 同様の理由でしか変更されない粒度で、整合性チェックを1つの関数に共通化する
  • 呼び出し側は呼び出すだけで済むようにし、それぞれ何を行っているかわかりやすい命名にする
  • ポリモーフィズム×enumで整理できそうなら整理する

感想メモ

1年前？半年前？とかにこの本を読んだときは何もしっくり来なかったけど、課題に向き合いながら読むと、具体的に「あのコードをこうするのがいいのかな」とか思い浮かんで消化しやすくなってた。やっぱ読書はタイミングだ。
オブジェクト指向も、FE中心なこともあってざっくり実務の中で得た肌感の側面が強かったが、これを読んでもう少し体系的に整理できた気がする。
続きの章はまだ今じゃない感を抱いたけど、またタイミングが来たら読もうと思う。インデックスは張っておく。

Discussion