以下の内容はhttps://tech-blog.rakus.co.jp/entry/20260319/software-design-principles-for-beginnersより取得しました。

新卒エンジニアがコードレビューの「設計指摘」を理解するまで

プログラミング Java リファクタリング

目次

0. はじめに

はじめまして。新卒一年目の楽楽販売開発エンジニアです。
ほとんど開発経験がない状態で入社し、数ヶ月の研修を経て実務に入りましたが、最初はコードレビューで受けた指摘の意図や直し方が分からず戸惑うことばかりでした。

例えば、コードレビューでこんな指摘を受けたことはないでしょうか。

  • 「このクラス、責務が多くないですか？」
  • 「将来 if が増えそうですね」
  • 「共通の処理が他にもあります」

私はまさにこのようなコードの設計に関する指摘をたくさん受けてきました。
ただ当時は「これらはなぜ必要なのか？結局どう直せばいいのか？」と思っていました。
私が書いたコードは要件通りに動くし、これらの指摘はどこか抽象的で、実装とどう結びついているのかが分からなかったからです。

それでもレビューで指摘を受けては実装を直し、学習を重ねるうちに、次第に

  • なぜその指摘が出てくるのか
  • どこを直せばよいのか

が、以前よりもはっきりしてきました。そこから実装やレビューの時に意識するべきポイントを見出すことができました。

この記事ではコード設計に関する指摘に焦点を当て、これまでの私の変化と学びを元に実装やレビューの時に気をつけると良いことについて述べます。 ここで紹介されるポイントを理解し実践することで、実装やレビューの質を上げていってほしいです。

コーディング経験が浅い人やこれからレビューを受ける人、これからレビューを始める人の参考になれば嬉しいです。

1. レビューされる側だった頃の問題点

最初の頃の私は、個人でコードを書いている感覚のまま、チーム開発をしていました。

  • とりあえず要件を満たして動けばOK
  • 保守性・可読性は「なんとなく」意識する程度

それに加えて重要な設計原則を知らず、気にしていなかったです。

このような状態だったので、レビューで多くの指摘を受けていました。 今だからこそ言えることですが、以下のような具体的な観点を実装時に持っていなかったことが原因の1つでした。

  • どこまでを1つの責務と考えるのか
  • 将来どんな変更が入りそうか
  • 他の人が読んだときに意図が伝わるか

いつも一貫した考えのもとで実装をしていなかったため、「指摘されてはその場で修正する」の繰り返しでした。
つまり、再現性のある判断基準を持てていなかったのです。 なお、上記のような観点の詳細やその重要性については 3 節以降で述べます。

2. レビューと設計の関係性

2.1 なぜレビューが必要なのか

ここで「レビューの必要性」について少し述べておきます。当初の私は、「レビューはバグを見つけるためのもの」だと考えていましたが、実際は違いました。
一般に、バグを見つけるのはテストでできるからこそ、中長期的な品質や保守性を担保するためにレビューを行います。

チーム開発には、

  • 自分以外の人がコードを読む
  • 複数人で継続的に機能追加や修正を行う

という特徴があるため、できるだけ早い段階で複雑さが増えにくい形にコードを整えておくことが重要になります。 これを実現するためにレビューが必要なのです。
つまりレビューは、「今の正しさ」だけでなく「将来も扱いやすいか」を確認するためにあると言えます。

また、結果としてそれはお客様に価値を早く、継続的に届けるための投資になります。

2.2 なぜ「設計」が関係するのか

レビューが「将来の扱いやすさ」を見る場だとすると、「では、その扱いやすさは何で決まるのか？」と疑問に感じるでしょう。

そこで出てくるのが「コード設計」です。

  • 責務の分け方
  • 変更理由の整理
  • 依存関係の持ち方 etc.

レビューでコード設計の話が出てくるのは、それが中長期的な保守性を大きく左右するからです。

3. コードレビュー指摘の傾向から学んだこと

ここではレビューで多かった指摘を振り返り、設計原則との関係を見ていきます。 また「コード設計に関する指摘」をもう少し具体的にします。

当時よくもらっていたのは、例えば次のような指摘です。

  • 責務が多い
  • 同じような処理が増えていきそう
  • 同じような処理が他にもある
  • 変更時の影響範囲が広そう

これらを別々の指摘として受け取っていた当初は、「なぜ指摘されるのか」「どうすれば指摘されなくなるか」を根本から理解できていませんでした。

後から整理すると、表現は違えど、これらはどれも将来の変更に弱いことを指摘していました。 これがまさに将来の扱いやすさに言及する「コード設計に関する指摘」（以降「設計指摘」）だったのです。 したがって、設計指摘とは「将来困りそうか？」を様々な観点から判断してなされる指摘だと言えます。

3.1. 様々な設計原則

設計指摘を具体的に理解するために、まずは私が受けた設計指摘に大いに関係していた設計原則を3つ紹介します。

  • SRP（Single Responsibility Principle: 単一責任原則）
    • 1つのクラスやメソッドには1つの責務のみを与えるべし
  • OCP（Open-Closed Principle: 開放閉鎖原則）
    • 既存コードの修正なしで拡張可能にするべし
  • DRY（Don't Repeat Yourself）
    • 重複を排除するべし

設計原則はどれも、将来の変更に対して壊れにくく、修正しやすく、ミスが入りにくい構造をあらかじめ作るための考え方です。

つまり、レビュワーが考えることはまさに設計原則そのものなのです。 それゆえ、レビューでは設計原則に基づいた指摘、すなわち設計指摘が頻出します。

3.2. 設計指摘を具体的に理解する

設計原則を知るともらった指摘の根底にある考えが分かるので、それらをより具体的に理解できます。

例えば、

  • 「責務が多い」 → SRP の観点で、変更理由が増えそう

  • 「同じような処理が増えそう」 → OCP / DRY の観点で、修正漏れが起きそう

  • 「影響範囲が広い」 → 依存関係が整理されておらず、防御しにくい。あるいは DRY を守れていない

こうして、当時は抽象的に思えた指摘をやや具体的に理解することができました。

3.3. 設計指摘をものにするためには？

設計指摘の根底にある考えは「このままでは将来が不安である」というものでした。 これだと抽象的に思えますが、実はある程度定まった基準が存在しており、それが設計原則でした。

設計原則を知り、実際のコードに応用できるようになれば、設計指摘で悩むことはなくなると思っています。

そういうわけで、次節では設計指摘を受けるコードとはどんなものなのかを見ていきます。 設計指摘を受ける理由やどのように直せば良いのかをまとめ、更に理解を進めます。

4. before / after で見る設計指摘の具体例

ここまで読めば、設計指摘の意味やその根底にある考えが分かってきたと思います。 4 節ではサンプルコードを用いて、実際にどんなコードがどんな理由で指摘され、どう直すべきなのかを before / after の形で見ていきます。

4.1. SRP: 「責務が多い」と言われたケース

まずは SRP（単一責任原則）の例です。
当時の私は以下のように、1つのクラスで色々な処理をしようとしていました。

before: 1つのクラスに複数の責務がある
public class UserService {

  public void register(User user) {
    validate(user);
    save(user);
    sendNotification(user);
  }

  private void validate(User user) {
    // 入力チェック
  }

  private void save(User user) {
    // データベースに保存
  }

  private void sendNotification(User user) {
    // 通知メール送信
  }
}

このコードの問題点は、「ユーザー登録」というユースケースの中に、複数の責務が押し込まれていることです。 以下のような別々の変更理由によって、コードの同じ箇所が変更される可能性があります。

  • 入力チェックの仕様が変わる
  • データベースへの保存方法が変わる
  • 通知手段が増える / 変わる
after: 責務ごとに分離する
public class UserService {

  private final UserValidator validator;
  private final UserRepository repository;
  private final NotificationService notificationService;

  public void register(User user) {
    validator.validate(user);
    repository.save(user);
    notificationService.notify(user);
  }
}

処理の流れ自体は変わっていませんが、

  • 入力チェック
  • データベースへの保存
  • 通知

が、それぞれ独立した責務として切り出されています。

この変更によって、

  • 仕様変更時の影響範囲が限定される
  • テストしやすくなる
  • 実装意図が読み取りやすくなる

といった効果があります。

「責務が多い」という SRP に基づく指摘は、将来の変更理由が1箇所に集まりすぎている、すなわち一人で色々やりすぎという意味であると理解できます。

4.2. OCP: 「将来増えそう」と言われたケース

次は OCP（開放閉鎖原則）の例です。当時は以下のように、分岐を増やして処理を追加していました。

before: 条件分岐で処理を切り替えている
public class PriceCalculator {

  public int calculate(Order order) {
    if (order.getType() == OrderType.NORMAL) {
      return order.getBasePrice();
    } else if (order.getType() == OrderType.DISCOUNT) {
      return order.getBasePrice() * 9 / 10;
    } else if (order.getType() == OrderType.SPECIAL) {
      return order.getBasePrice() * 8 / 10;
    }
    throw new IllegalArgumentException();
  }
}

このようなコードは「将来 if が増えそうですね」などと指摘を受けるでしょう。 そして当時の私なら「今はこれで動くし十分では？」と思ったに違いありません。

このコードの問題点は、

  • 注文種別が増えるたびに if が増える
  • 修正のたびにこのクラスを触る必要がある
  • 変更漏れのリスクが高くなる

「今動くかどうか」ではなく「将来どうなり得るか」が問題です。 そもそも設計指摘とはそういうものでした。

after: 振る舞いを分離する
public interface PriceStrategy {
  int calculate(Order order);
}

public class NormalPriceStrategy implements PriceStrategy {
  public int calculate(Order order) {
    return order.getBasePrice();
  }
}

public class DiscountPriceStrategy implements PriceStrategy {
  public int calculate(Order order) {
    return order.getBasePrice() * 9 / 10;
  }
}

public class PriceCalculator {

  private final PriceStrategy strategy;

  public int calculate(Order order) {
    return strategy.calculate(order);
  }
}

この形にすると、

  • 新しい価格計算ルールを追加しても既存コードをほぼ触らない
  • 条件分岐が増えない
  • 振る舞いごとに責務が分かれる

という状態になります。

「将来増えそう」という OCP に基づく指摘は、将来変更が入る時にコードがどう修正されるかを見ていたのだと分かりました。 変更する時に既存のコードは変更せず、新たにコードを追加するだけで対応できるようにするべきであるという OCP の意味が理解できました。

4.3. DRY: 「共通化できそう」と言われたケース

次は DRY の例です。当時は以下のように、コードの複数箇所に同じことを書いていました。
なお DRY には "知識" と "処理" に注目する2パターンがあるので、それぞれ紹介します。

before: 同じコードが複数箇所にある
public class OrderService {

  public int calculateTotal(Order order) {
    int subtotal = order.getSubtotal();
    int tax = subtotal * 10 / 100;
    return subtotal + tax;
  }

  public int calculateRefund(Order order) {
    int subtotal = order.getSubtotal();
    int tax = subtotal * 10 / 100;
    return subtotal - tax;
  }
}

このコードの問題点を "知識" に注目して挙げると、

  • 税率10%という "知識" が二箇所に書かれている
  • 税率変更時に両方修正が必要

これにより修正漏れのリスクが高くなっています。

after 其ノ壱: 知識を一箇所に集約
public class TaxCalculator {

  private static final int TAX_RATE = 10;

  public static int calculate(int amount) {
    return amount * TAX_RATE / 100;
  }
}

public class OrderService {

  public int calculateTotal(Order order) {
    int subtotal = order.getSubtotal();
    return subtotal + TaxCalculator.calculate(subtotal);
  }

  public int calculateRefund(Order order) {
    int subtotal = order.getSubtotal();
    return subtotal - TaxCalculator.calculate(subtotal);
  }
}

こうすれば、例えば税率を8%に変える時、TaxCalculator の冒頭一箇所のみを修正すれば済みます。
元のコードに比べて修正漏れのリスクが低くなっています。

一方で元のコードの問題点を "処理" に注目して挙げると、

  • 税金計算ロジックが二箇所に書かれている
  • 変更時に両方修正が必要

これらを解決するには、"知識" の場合にならって "処理" を一箇所にまとめれば良いです。

after 其ノ弐: 処理を一箇所に集約
public class OrderService {

  public int calculateTotal(Order order) {
    int tax = calculateTax(order.getSubtotal());
    return order.getSubtotal() + tax;
  }

  public int calculateRefund(Order order) {
    int tax = calculateTax(order.getSubtotal());
    return order.getSubtotal() - tax;
  }

  private int calculateTax(int subtotal) {
    return subtotal * 10 / 100;
  }
}

重複していた税計算処理が一箇所のみになるので、この場合も元のコードに比べて修正漏れのリスクが低くなります。

DRY 原則を守るとは、同じコードをまとめ、修正箇所を減らすことだと分かりました。

4.4. before / after から学んだこと

これらの例から、設計指摘を受けるコードの特徴や改善方法がイメージできたと思います。 またコードの問題点を before / after を通して見ることで、設計が、2 節で述べた「中長期的な保守性」に大きな影響を与えることも分かったと思います。

このような経験から、私は設計指摘が出てしまう原因には実装者とレビュワーの視点の違いがあると考えています。

実装者としては、

  • 仕様を満たしているか
  • テストが通っているか

に目が行きがちです。 実際、before のコードはどれも「今の要件」だけを見れば問題ありません。

一方レビュワーは、

  • 機能追加や仕様変更
  • 別の人が触る未来

を考えています。設計指摘はこれらを事前に想像したうえで出されていました。

この視点の違いこそ、レビューで指摘が出る理由です。 それゆえ実装時には 1.2 節で述べたような観点を持つことが重要なのだと思います。

5. 実装時に持つべき視点

設計指摘の意味が分かると、以前とは異なり、実装時点でレビュー視点を持てるようになりました。 実装中に自然と次のような問いを立てるようになりました。

  • このクラスは、何のために存在しているのか
  • 変更理由は1つに収まっているか
  • 将来どんな仕様変更が入りそうか
  • そのときどこを直すことになりそうか

これらは全てこれまでレビューで指摘されてきたポイントであり、別の言い方をすれば、設計原則が言おうとしていることでした。 その結果、レビューで指摘されそうな点に早い段階で気づけるようになりました。

こうして実装時に色々な観点を持てるようになりました。 自分の実装の良し悪しを常に同じ判断基準で考えることができるので、実装のたびにブレることが少なくなったと思います。

もしレビューで設計指摘を受けたときは、これらの観点で一度コードを見直してみてください。 「今動くか」ではなく「将来どう変わるか」を考えるだけで、見えるものが大きく変わると思います。

また、最近から私はレビューする側にも回っています。 レビューされる側として多くの指摘を受けてきた経験は、レビューをする時でも大いに役立っていると感じます。

自分なりに着眼点を持ち、「今の実装が動くかどうか」だけでなく、将来のことを考えてレビューをしています。

6. 同じ立場の人たちへ

実装時あるいはレビュー時に、本記事で挙げたような観点を持っているのといないのでは差があります。
おさらいすると、

  • クラスや関数の責務は1つか
  • 将来どんな変更が入りそうか
  • そのときどこを直すことになりそうか
  • 修正する箇所は少ないか
  • 他の人が読んだときに意図が伝わるか

などであり、重要なのは、これらはどれも「今」ではなく「将来」を見ているということです。

常に同じ判断基準を持ち、経験とともにそれを磨き、増やしていけば、実装やレビューの質は確実に上がると思っています。

7. まとめ

この記事では、

  • なぜレビューやコード設計が重要なのか
  • 抽象的に思える設計指摘の意味や直し方

を、私自身の変化を軸に整理してきました。
コード設計が中長期的な保守性に大きな影響を与えるので、将来の変更に耐えられるかという視点から設計指摘がなされるのでした。

そして具体的なコードを見て設計への理解をより深め、実装やレビューの時に持つと良い視点としてまとめることができました。 レビューで設計指摘を受けている人やこれからレビューを始める人にはぜひ意識してもらいたいです。

この記事がそのような人の助けになれば嬉しいです。



以上の内容はhttps://tech-blog.rakus.co.jp/entry/20260319/software-design-principles-for-beginnersより取得しました。
このページはhttp://font.textar.tv/のウェブフォントを使用してます
不具合報告/要望等はこちらへお願いします。
モバイルやる夫Viewer Ver0.14