🎃

「良いコード/悪いコードで学ぶ設計入門」を読んで学んだことを整理

naoki matsuzaki

2023/05/28に公開

設計

リファクタリング

tech

今回、「良いコード/悪いコードで学ぶ設計入門」という本を読みました。

その本を読んだことで得られた知識などを整理しておこうと思い、記事を書きます。

まずは何がダメなのかを理解すること

設計において、悪い例を把握することは重要だと感じます。

今自分が書いているコードが本当に大丈夫なのかについては、良し悪しを判断できるようにならなければなりません。

「悪しき構造の弊害を知覚」することが重要であることです。

連番命名やif文のネスト

連番命名：付けたくなります。関数の命名で時間を取られるのが嫌だと感じ、似たような処理だから連番で命名してしまうことありませんか。

また、初心者で多いのは、if 文のネスト構文です。

if (条件1) {
 // 数十行に及ぶ処理
  if (条件2) {
   // 数十行に及ぶ処理
    if (条件3) {
     // 数十行に及ぶ処理
    }
  }
} else {
}

条件 1 にクリアしたら、次の条件 2 に合致するかを判定します。その間、数十行に及ぶ処理を読む時、条件 1 のことを頭の片隅に置きながら読まなければなりません。

これは、別の書籍ですが「リーダーブルコード」にも同じことが書いてあります。

if 文がネストしたら早期 return を検討することがポイントです。

低凝集な状態を避けるべき

クラスにデータとロジックが分散した状態を「低凝集」と呼んでいます。

低凝集な状態を避けるべきと記載してありました。

その理由を下記に示します。

コードが重複する
- いろんな場所で使われる
- 追加回収などの修正漏れが発生しやすくなる
未初期化状態が出来上がる
- 初期化しないと使えないクラス
- 未初期化状態が発生しうるクラス
不正値が混入
- 外側から値を渡すと不正値を渡せる
- クラス利用側でバリデーションを実装することもあるが、それが重複を発生させやすい

クラスにはデータとロジックを凝集させ、バリデーションもクラスにて実行することで重複しないコード設計が可能になります。

クラスのメリットとしては、データとロジックを 1 つの場所に凝縮させることができます。

これが単なる変数として定義したものを使うと、他の場所で使われることもあり、使い方が異なる可能性もあります。

クラスのインスタンス変数として管理し、そのクラスのメソッドで利用するようにすることで、強く関係し合うデータとロジックを凝集させることが可能となります。

サンプルコードとして、HP をインスタンス変数として管理する場合、攻撃受けるメソッドで HP が減るが、その HP を作成するためにメソッド内でインスタンス化するという方法のコードです。

<?php
class Animal
{
    private $hp;
    public function __construct($hp) {
        $this->hp = $hp;
    }

    // メソッドの宣言
    public function attack($damage) {
        $currentHp = $this->hp - $damage;
        return new Animal($currentHp);
    }
}
?>

ダメージを受けた分の HP を表示するロジックは、上記のように attack メソッドを実装することにより、ここからだけ呼び出せば良いことになります。

意図が伝わる変数名や関数を使う

変数名がわからないと何の値が入っているのか分かりにくくなります。

一度定義した変数は、使いまわしてコロコロと値を更新しないようにしましょう。(再代入)現在の値の状態を意識しながら読まないといけなくなります。

こうした変数が変化することを防ぐため、変数を不変にするのが良いでしょう。

JavaScript でも、const を推奨しています。

let は変数の値を変更できますが、const を使えば値が変更できません。

可変状態にしておくことで、意図しない影響が発生しやすくなります。(可変に定義しているつもりでなくても、変更されていることがあるということ)

また、可変によって副作用も起きやすくなります。

※副作用とは関数が引数を受け取り、戻り値を返す以外に外部の状態を変更することを指します。

つまりインスタンスの可変状態が、意図せず副作用のある関数が作り込まれてしまいます。

考え方の基本：不変にすることでインスタンス化させることで新しい値を作成し、互いの影響を受けにくなります。

一連の処理をべた書きせず、一連のまとまりで関数として切り出すことで見通しがよくなります。

クラス設計

設計においてクラス設計がとても重要になります。

クラス設計における考え方のポイントとしては、クラス単体で正常に動作するよう設計するのがポイントです。

例えば、ドライヤーや電子レンジもそれ単体で動かすことができ、別のものを合体させて使うことはない電化製品です。

これらは、初期設定も特に必要なく、基本のものは使える状態です。

クラス設計も同じ考え方です。

良いクラスというのは、インスタンス変数とそのインスタンス変数を不正状態から防御して正常に操作するメソッドが書かれており、メソッドには、必ずインスタンス変数を使うようにすることです。

インスタンス変数の使い方のコツ

インスタンス変数などをコンストラクタで初期化する際、バリデーションをコンストラクタで実施し、不正な値を最初から弾くようにする
インスタンス変数を不変(イミュータブル)とする
インスタンス変数を変更したいときはインスタンスを作成する

値が変わることは予測不能の値が入ってしまうことがあります。そのため、基本は不変のイミュータブルな状態にしておくのが良いです。

ただ、インスタンス変数をイミュータブルにし、「どうやってインスタンス変数を変更するのか」ということですが、インスタンス変数を変更する処理を新たにクラスインスタンスを生成するようにすれば良いです。

関数の引数について

関数の引数には独自の型を渡すのが良いです。

例えば、int や string などのプリミティブ型だと予測不能な値が入ることもあります。

プリミティブ型の何がいけないんだよということですが、例えば金額の 100 を入れる関数があったとき、そこに枚数の 50 が入ってもガードできません。

意味の違う数字が入っても気付けないので、クラスを渡すのが良いです。

クラスを渡すことに少し抵抗を覚えますが、意味のあるまとまりごとにクラスを作ることが重要です。

特に引数が多い時は、そのデータをインスタンス変数として持つクラスを作ることを検討しましょう。
→こちらに関しては得私は意識して書いてこなかったので、気にしながらコーディングしたいと思っています。

また、関数の引数は結果を返さないようにしましょう。

関数の引数に結果を返す引数を渡すと低凝集構造となり、重複ロジックを生み出しやすくなります。

こういった関数の引数を結果を返すために利用する引数を出力引数と呼びます。

生焼けオブジェクト

当該書籍で、引数なしのデフォルトコンストラクタで、後からインスタンスに設定するのは生焼けオブジェクトと呼んでいました。

この構造は、未初期化状態を誘発するクラス構造となっています。

コンストラクタで確実に正常値を渡すように、インスタンス化時は必要な引数を渡してあげることがポイントとなります。

サンプルコードとして JavaScript のプロトタイプベースで書いてみました。

function Human(name, age) {
  this.name = name;
  this.age = age;
  Object.freeze(this);
  console.log(`私の年齢は、${this.age}です`);
}

Human.prototype = {
  showName() {
    console.log(`私の名前は${this.name}で、年齢は${this.age}です。`)
  },
  addAge(human){
    const age = this.age + human.age;
    return new Human(this.name, age);
  }
};

const human = new Human('なお',31);
human.addAge(new Human('太郎',10));
human.showName();

// output
私の年齢は、31です
私の年齢は、10です
私の年齢は、41です
私の名前はなおで、年齢は31です。

このように、クラス初期化時には引数を確実に渡してあげるようにしましょう。

クラス初期化時のコンストラクタで、バリデーションロジックを組み込むとさらに堅牢な構造となります。

高凝集なクラス構造とは

メソッドには static メソッドが存在しています。static メソッドはクラスをインスタンス化せずに利用できるメソッドです。

インスタンス化せずに利用できるので便利なように見えますが、データとロジックが分離しやすくなります。

データとロジックを分離しても良い下記のような凝集度に関係ないケースなら使っても良さそうです。

ログ出力
フォーマット変換

staticメソッドの使い道

static メソッドは悪いことだけではありません。

コンストラクタを公開状態にしておくとあらゆるところでインスタンス化される状態となるため、プライベートとして定義するのも良いと書いてあります。

例えば、会員登録時のポイントが通常とプレミアム登録で異なる時、それぞれがあらゆるところで利用されるため、全て見直さなければなりません。

しかし、コンストラクタをプライベートにし、static メソッドでそのコンストラクタを呼び出すようにしてあげたらクラスで変更をすればいいので高凝集な構造となります。

<?php
class Point
{
    private $point;

    public function __construct($point) {
        $this->point = $point;
    }
}

$standard = new Point(20);
$premium = new Point(100);
?>

上記だと、通常とプレミアムでの登録の違いによるインスタンス生成だが、もし下記のようになったらどうしましょう。

通常ポイントは、20pt から 10pgt に変更
プレミアムポイントは、100pt から 150pt に変更
通常・プレミアムに加え、シルバー会員の登録を増やし、ポイントは 100pt とする

こうなると全ての new Point()のコンストラクタに渡す引数を全て見直さなければなりません。

ここで、static メソッドを利用し、ポイントの変更をクラスで実施できるようにし、変更に耐えうる構造とします。

class Point
{
    private $point;

    private function __construct($point) {
        $this->point = $point;
    }

    public static function standard()
    {
        return new Point(10);
    }

    public static function premium()
    {
        return new Point(150);
    }

    public static function silver()
    {
        return new Point(100);
    }
}

$standard = Point::standard();
$premium = Point::premium();
$silver = Point::silver();
?>

デメテルの法則

本書籍で、「デメテルの法則」が出てきました。

デメテルの法則とは。ソフトウェア工学の設計原則の 1 つであり、オブジェクト指向プログラミングにおけるモジュール間の結合度を低くし、変更の影響範囲を制限するための原則です。

端的に言えば、「利用するオブジェクトの内部を知るべきではない」という考え方です。

メソッドチェインは、戻り値の要素に次々とアクセスする書き方を指してますが、低凝集に陥りやすいです。

ソフトウェア設計において、「尋ねるな、命じろ」という言葉があります。

オブジェクトの内部状態(変数)を尋ねたり、その状態に応じて呼び出し側が判断したりするものではなく、呼び出し側はただメソッドで命ずるだけです。

つまり、命令された側で適切な判断や制御をするように設計するべきと書いてあります。

ifやswitch文の利用について

初学者では、似たような処理で通常とプレミアムで特別な動きをする際、if 文を書きがちです。

if (通常会員の時) {
    // 通常会員の処理
} else {
    //プレミアム会員の処理
}

しかし、これでは他の会員が存在するようになるとさらに if 文が長くなります。

このような設計はイけていないので、インタフェースを使ってロジックを分離することにします。

if文はネストを避け、早期returnさせる

if 文のネストによる可読性低下を避けるため、早期 return や早期 break,continue を推奨しています。

また、コレクションの処理で、元々メソッドが用意されているのに、それを利用せず自作してしまうことでコード量が増えてしまうことも指摘しています。(元々あるものを再度作ってしまうことから車輪の再開発とも呼んでいる)

インタフェース設計を利用したストラテジーパターン

<?php

interface Shape {
    public function area();
}

class Rectangle implements Shape
{
    private $width;
    private $height;

    public function __construct($width, $height) {
        $this->width = $width;
        $this->height = $height;
    }

    public function area()
    {
        $area = $this->width * $this->height;
        echo $area;
        return $area;
    }
}

class Circle implements Shape
{
    private $radius;

    public function __construct($radius) {
        $this->radius = $radius;
    }

    public function area()
    {
        $area = $this->radius * $this->radius;
        echo $area;
        return $area;
    }
}

class ExecuteMethod
{
    public static function showArea(Shape $shape)
    {
        return $shape->area()
    }
}

$rectangle = new Rectangle(15);
$rectangle->area();

$circle = new Circle(10,20);
$circle->area();

ExecuteMethod::showArea($rectangle);
// →四角形の面積が算出できる
ExecuteMethod::showArea($circle);
// →円の面積が算出できる

このコードでは、図形の面積を求める処理が記載されています。

四角形、円共に面積を求めるarea()メソッドが定義されていますが、どちらもShapeインタフェースを利用しています。

インスタンス化によって面積の算出方法が異なるものの、利用側でインタフェースを使うことでどのクラスが渡されようと同じメソッドが使えるようになります。

ストラテジーパターンとポリシーパターンを利用した設計

module Member
  def postage
    raise NotImplementedError, 'Subclasses must override this method'
  end

  def add_point(price)
    raise NotImplementedError, 'Subclasses must override this method'
  end

  def can_same_day_shipping
    raise NotImplementedError, 'Subclasses must override this method'
  end

  def amount
    raise NotImplementedError, 'Subclasses must override this method'
  end
end

class NormalMember
  include Member

  USAGE_AMOUNT = 1000

  def postage
    100
  end

  def add_point(price)
    price * 0.05
  end

  def can_same_day_shipping
    false
  end
end

class SilverMember
  include Member

  USAGE_AMOUNT = 1000

  def postage
    50
  end

  def add_point(price)
    price * 0.1
  end

  def can_same_day_shipping
    false
  end
end

class GoldMember
  include Member

  USAGE_AMOUNT = 1000

  def postage
    0
  end

  def add_point(price)
    price * 1.5
  end

  def can_same_day_shipping
    true
  end
end

module CustomerRule
  def comply_with_rule(price)
    raise NotImplementedError, 'Subclasses must override this method'
  end
end

class NormalMemberRule
  include CustomerRule

  def comply_with_rule(price)
    price < 1000
  end
end

class SilverMemberRule
  include CustomerRule

  def comply_with_rule(price)
    price < 10000
  end
end

class GoldMemberRule
  include CustomerRule

  def comply_with_rule(price)
    price >= 10000
  end
end

class CustomerPolicy
  def initialize
    @rules = []
  end

  def add_rule(member_rule)
    @rules << member_rule
  end

  def comply_with_all_rules(price)
    @rules.all? { |rule| rule.comply_with_rule(price) }
  end
end

normal_member = NormalMember.new
silver_member = SilverMember.new
gold_member = GoldMember.new
normal_member_rule = NormalMemberRule.new
silver_member_rule = SilverMemberRule.new
gold_member_rule = GoldMemberRule.new

normal_customer_policy = CustomerPolicy.new
normal_customer_policy.add_rule(normal_member_rule)

silver_customer_policy = CustomerPolicy.new
silver_customer_policy.add_rule(silver_member_rule)

gold_customer_policy = CustomerPolicy.new
gold_customer_policy.add_rule(gold_member_rule)

price = 2000

if normal_customer_policy.comply_with_all_rules(price)
  puts 'normal'
  p normal_member
elsif silver_customer_policy.comply_with_all_rules(price)
  puts 'silver'
  p silver_member
elsif gold_customer_policy.comply_with_all_rules(price)
  puts 'gold'
  p gold_member
end

ストラテジーパターンとポリシーパターンを組み合わせることで、条件自体をクラス化して判定できます。

密結合について

ソフトウェアには原則として「単一責任の原則」というのがあります。

これは、クラスが担う責任はたった 1 つに限定すべきということです。

例えば商品割引に関するクラスがあります。

その中で、300 円割引機能があったとし、追加機能として夏季限定の価格クラスのロジックを追加実装しました。

しかし、途中で商品割引価格を 300 円から 400 円に変更したあと、夏季限定の価格にバグが起きるようになりました。

これは、夏季限定価格が商品割引クラスのメソッドを流用したことが影響したものでした。

なぜこのようなことが起きてしまったのかという点で、ロジックの置き場がチグハグになっていることを原因としています。

説明すると、商品割引に関するクラスが、下記のようにたくさん詰め込まれてしまっているからです。

商品情報のチェック
割引価格の計算
割引適用するかの判断
総額上限チェック

これをそれぞれのクラスに分けてみます。

定価クラス
- 定価を算出するロジックを実装
通常割引価格クラス
- 割引価格を算出するロジックを実装
夏季割引価格クラス
- 夏季限定の割引価格を算出するロジックを実装

通常価格と夏季限定での割引価格クラスは定価クラスを利用する形にします。

そうすることで、関心ごとが分離され、独立した構造となる。

これを疎結合の状態と呼んでいます。

一方、通常割引価格クラスと夏季割引価格クラスの算出ロジックは共通しており、割引価格が異なる状態になっています。

これは、重複コードとなり、DRY 原則に違反しているように見えるが、無理にまとめることで責務が増えすぎるのは良くありません。

本書では、同じような or 似ているロジックは概念が異なれば DRY にするべきではないと書いていました。

継承での問題点

密結合は、継承によっても発生する可能性があります。

継承関係あるスーパークラス(親)の構造に子クラスが依存するため、スーパークラスの変更が子クラスのバグに繋がりやすくなります。

これも先ほどの商品価格の例で話すと、商品通常割引と夏季限定割引のクラスがスーパークラスとして定価のクラスを継承した状態とします。

この状態は、商品通常割引と夏季限定割引のクラスを同じスーパークラスを継承するのが良くありません。

その中で夏季限定割引だけ、ある一定数の商品価格でないと割引しないようにする修正が入ると、スーパークラスに新しい関数が追加され、それを夏季限定割引クラスだけが利用する構造となります。

上記のような問題点が発生するため、継承より委譲を推奨しており、コンポジション構造というのが出てきます。

これは、利用したいクラスをスーパークラスとして継承するのではなく、インスタンス変数を持つようにすることです。

コンポジション構造の例を示します。

<?php

class Price
{

    public function __construct($price)
    {
        $this->price = $price;
    }

    public function calculate()
    {
        return $this->price;
    }
}

class DiscountPrice
{
    private $price;

    public function __construct($price)
    {
        $this->price = $price;
    }

    public function calculate()
    {
        return $this->price->calculate() * 0.8;
    }
}

$price = new Price(2000);
$discountPrice  = new DiscountPrice($price);
$discountPrice->calculate();

?>

これにより、価格と割引価格のどちらかのロジックが変更になっても変更しないクラスは影響を受けなくなります。

継承よりもコンポジションについては、こちらに丁寧に書かれていて、参考になりました。

高凝集は密結合になりやすい

高凝集にクラスを作ろうとするあまり、いつの間にか密結合になってしまっているという状態に陥りやすいです。

高凝集・疎結合を目指す必要があるが、クラスは概念を考え、別の概念が入っていないかのチェック、入っていたらクラスをできるだけ分けるというのが重要になります。

例では、販売価格のクラスには下記メソッドがあります。

販売手数料を算出するメソッド
配送料を計算するメソッド
獲得するショッピングポイントを計算するメソッド

しかし、これらは販売価格とは別概念になっているので、別クラスに分けるべきです。

販売手数料のクラス
配送料クラス
ポイントクラス

クラスの分け方は目的駆動での名前設計が必要

目的駆動での名前設計という考えで「問題解決を意図した作り」にします。

ここで、名前設計が不十分だと、それにさまざまなクラスが関連付き、密結合状態になります。

例えば EC サイトの設計で「商品」という名前でクラスを設計すると、下記が全て含まれます。

予約
発注
購入
発送

これは商品という大雑把な名前が引き起こしています。

商品というクラス名は、上記の予約や購入も全てできるような意味に捉えることができてしまうからです。

商品クラスをそれぞれで分けるために下記のようにクラスを分けましょう。

予約品
発注品
購入品
発送品

つまり、いかに名前の設計が高凝集・疎結合という状態に大きく関わっているかを示しています。

関数の設計のポイント

インスタンス変数を安全に操作するようメソッド設計することで、クラスの正常性も担保できます。

メソッドでは、必ず自クラスのインスタンス変数を使うようにすることがポイントです。

別クラスのインスタンス変数を使うと低凝集に陥ります。

もし他のクラスのインスタンス変数を使わざるを得ない時が来たら、変更したいインスタンス変数を持つクラスに変更メソッドを実装するようにしましょう。インスタンス変数を不変にする、そうすることで予期せぬ動作を防げます。

また、よそのクラスの状態を判定するメソッドは同じく低凝集になりやすいです。「尋ねるな、命じろ」という言葉の通り、呼び出されるメソッド側で複雑な制御をするように設計しましょう。

コマンド・クエリ分離という考え方があります。コマンド(変更)またはクエリ(問い合わせ)はメソッド内に混合させないという考え方です。

参考：https://rakusui.org/cqs/

つまり、オブジェクトの状態を変更するようなメソッドの時は、値を返してはいけません。

その他関数におけるポイント

引数は不変にすること
フラグ引数を使わないこと(条件分岐するならストラテジパターンを検討)
null を渡さない
出力引数は使わない(出力値として使わない)
戻り値はプリミティブ型にせず、独自の型を使い、想定されていない値を渡さないようにする

ベースのクラス設計について

モデリングはとても重要です。

設計の考え方として「システムは目的達成のための手段」で、その目的達成のため「最低限考慮が必要な要素を備えたものがモデル」であることを強調しています。

【悪い例からの検討】

EC サイトの商品のモデルだけを定義した場合、商品には、様々な情報があります(商品名、賞味期限、価格、製造年月、製造メーカー...)

あげるとキリないですが、これをひとまとめのモデルとして考えてはいけません。

そこで、商品モデルに最低限必須の要素を考え、モデルで表現しましょう。

注文時の商品モデル
配送時の商品モデル
仕入れ時の商品モデル

このようにして目的ごとに商品モデルが変わるように設計しましょう。

このクラス設計については、現実世界では商品という 1 つのものだが、モデリングで考えると１つにしないほうが良いです。

現実世界での物理的存在と情報システム上のモデルが 1:多になるケースがあることを認識しなければなりません。

User で考えた場合、現実世界では一人の人間だが、情報システムとしては下記の複数モデルで定義されるケースが多いです。

個人アカウント
法人アカウント
プロフィール
会社概要
職務経歴

以上のような話から、モデルはあくまでモノではなく、目的達成手段であることを理解しなければなりません。

目的達成するためのモデル設計は適切に設計できます。

また、モデルとクラスはイコールにはならず、モデルはしくみを単純化したものに過ぎません。モデル 1 個から複数のクラスを構成するように考えます。

GitHubで編集を提案

株式会社ヤマデンPublication