😈

【入門】結局getter/setterは悪なのか

2021/09/26に公開2件

PHP

先に結論

setterは悪。
getterは必要なケースがあり、一概に悪といえない。
getterによりビジネスロジックをオブジェクト外に流出させてしまうのは悪。

想定読者

オブジェクト指向に入門した方々
getter/setterがNGというのは概念的に理解したけど、具体的な改善方法を知りたい方々
というか1年前の僕

ネットでgetter/setterについて検索すると、「必要ない」だとか「カプセル化を破る」だとか「Tell, Don't Ask」だとかが出てきます。

getter setter 使わない - Google Search

普通の人なら「getter/setterはアンチパターンなのだ」と認識することでしょう。
しかし、現場のコードを見ると普通に使われてたりします。
特にgetterは検索結果など知らんという顔で歩いており、オブジェクト指向に入門した方は非常に困惑すると思います。

また、オブジェクト指向の説明はどうしても概念的になりやすい、という側面を持っています。
getter/setterに関しても、実装による説明は控えめで、文章での説明が多くなってしまいます。
getter/setterを使うな、と言われても入門者にとっては「じゃあどうすれば良いの？」という感じだと思います。

少なくとも1年前の僕は困惑しました。そんな方々に届けばと思って書きました。
本記事ではPHPによる実装を通じて、悪いgetter/setterを表現し、それが後々どう問題になってしまうのか、どう改善すれば良いのかを説明したいと思います。

なぜgetter/setterの善悪議論は終着しないのか

少し論点を整理したいと思います。

getterとsetterの善悪を一律で判断できない
getterは悪とされるパターンとそうでないパターンが存在する

まずgetterとsetterはそれぞれオブジェクトへの読み込みと書き込みであり、性質が全く異なるものです。
単純なgetterであれば何度読み込んでも副作用はないはずである一方、setterは書き込みという副作用があります。
この2つを同列で扱って議論すると、議論が終着しなくなってしまいます。

setterはコンストラクタ等でせっかく満たしていたオブジェクト生成条件を壊して、mutableなオブジェクトを作れてしまいます。
その結果、処理が理解しにくくなったり、思わぬバグを生み出す原因になることがあります。
本記事の意見としてはsetterは「悪」としています。

一方でgetterには悪となるケースとそうでないケースがあります。
getterは使い方を間違えると「悪」となるケースがあり、それはビジネスロジックがオブジェクトの外に流出しているケースです。
この場合、ビジネスロジックが書かれている場所が分散してしまい、ビジネスロジックの可読性・再利用性などを低下させてしまいます。
ではgetterは必ず悪なのかというと、インターフェース層などでプリミティブな値を出力するケースで必要になることがあります。
つまり、getterを一律で「不要」「悪」と断じることはできません^[1]。

これらについて、具体的にPHPの実装を元に説明できればと思ってます。

実装

バージョン

php -v

PHP 8.0.0rc1 (cli) (built: Oct 13 2020 08:42:44) ( NTS )
Copyright (c) The PHP Group
Zend Engine v4.0.0-dev, Copyright (c) Zend Technologies

悪いgetterとsetterを使った実装

仕様1,2

エンジニアのAさんは、こんな仕様の関数を作ることになりました。

アイテムの単価と数量を引数とし、合計額を計算する
計算に使った単価と数量はオーダー日時とともにアイテムオーダーとして保存する
単価か数量が0以下の場合はエラーとなる
エラーの場合は0を返す

実装は以下となりました。
ちなみに、この段階ではまだそこまで悪い実装にはなっていません。

calc.php

<?php

function calc_total_price(int $unitPrice, int $quantity): int
{
    $itemOrder = new ItemOrder($unitPrice, $quantity, new DateTimeImmutable());
    if (!$itemOrder->validates()) {
        return 0;
    }

    $itemOrderRepository = new ItemOrderRepository();
    if (!$itemOrderRepository->persist($itemOrder)) {
        return 0;
    }

    return $itemOrder->getUnitPrice() * $itemOrder->getQuantity();
}

// 仕様1: 単価*個数が返ってくること
assert(calc_total_price(200, 4) === 800);

// 仕様2: 単価がマイナスではエラーとなること
assert(calc_total_price(-500, 4) === 0);

echo 'all green' . PHP_EOL;


class ItemOrder
{
    private int $unitPrice;

    private int $quantity;

    private DateTimeImmutable $orderedAt;

    public function __construct(int $unitPrice, int $quantity, DateTimeImmutable $orderedAt)
    {
        $this->unitPrice = $unitPrice;
        $this->quantity = $quantity;
        $this->orderedAt = $orderedAt;
    }

    public function getUnitPrice(): int
    {
        return $this->unitPrice;
    }

    public function getQuantity(): int
    {
        return $this->quantity;
    }

    public function getOrderedAt(): DateTimeImmutable
    {
        return $this->orderedAt;
    }

    public function validates(): bool
    {
        return 0 < $this->unitPrice && 0 < $this->quantity;
    }
}

class ItemOrderRepository
{
    public function persist(ItemOrder $itemOrder): bool
    {
        $params = [
            'unit_price' => $itemOrder->getUnitPrice(),
            'quantity' => $itemOrder->getQuantity(),
            'ordered_at' => $itemOrder->getOrderedAt()->getTimestamp(),
        ];
        // アイテムオーダーをDBとかに保存する処理が書かれてるとします

        return true;
    }
}

php calc.php

all green

いくつか実装について説明します。

テストはassert関数によって表現してます^[2]。
calc_total_price関数が本体で、この中でDBへの保存と合計額を返すという処理を実行します。
ItemOrderクラスは単価と数量とオーダー日時を持ったオブジェクトを作ります^[3]。
ItemOrderRepositoryクラスについてはItemOrderオブジェクトのDBへの永続化を担っている、という程度の認識で大丈夫です。あまり理解してなくても本記事の趣旨を理解するのには影響しません^[4]。
validates関数も実装しているので、単価や数量にマイナス値とかが入ることはなくAさんも安心しました(伏線)

仕様は満たせてますし、一旦はこのまま進めます。

仕様3

追加の仕様が来ました。

個数が5個以上なら単価は20円引きとなる

たくさん買ってくれる場合には割引することで個数UPを促進する、というよくある追加仕様かと思います。
以下のように実装しました。

 <?php
 
 function calc_total_price(int $unitPrice, int $quantity): int
 {
     $itemOrder = new ItemOrder($unitPrice, $quantity, new DateTimeImmutable());
     if (!$itemOrder->validates()) {
         return 0;
     }
 
+    if (5 <= $itemOrder->getQuantity()) {
+        $discountedPrice = $itemOrder->getUnitPrice() - 20;
+        $itemOrder->setUnitPrice($discountedPrice);
+    }
+
     $itemOrderRepository = new ItemOrderRepository();
     if (!$itemOrderRepository->persist($itemOrder)) {
         return 0;
     }
 
     return $itemOrder->getUnitPrice() * $itemOrder->getQuantity();
 }
 
 // 仕様1: 単価*個数が返ってくること
 assert(calc_total_price(200, 4) === 800);
 
 // 仕様2: 単価がマイナスではエラーとなること
 assert(calc_total_price(-500, 4) === 0);
 
+// 仕様3: 個数が5個以上なら単価は20円引きとなること
+assert(calc_total_price(200, 5) === 900);
+
 echo 'all green' . PHP_EOL;
 
 
 class ItemOrder
 {
     private int $unitPrice;
 
     private int $quantity;
 
     private DateTimeImmutable $orderedAt;
 
     public function __construct(int $unitPrice, int $quantity, DateTimeImmutable $orderedAt)
     {
         $this->unitPrice = $unitPrice;
         $this->quantity = $quantity;
         $this->orderedAt = $orderedAt;
     }
 
     public function getUnitPrice(): int
     {
         return $this->unitPrice;
     }
 
     public function getQuantity(): int
     {
         return $this->quantity;
     }
 
     public function getOrderedAt(): DateTimeImmutable
     {
         return $this->orderedAt;
     }
 
     public function validates(): bool
     {
         return 0 < $this->unitPrice && 0 < $this->quantity;
     }
+
+    public function setUnitPrice(int $unitPrice)
+    {
+        $this->unitPrice = $unitPrice;
+    }
 }

 (中略)

php calc.php

all green

実装について説明すると以下のようになります。

個数が5個以上のときに単価を下げる、という処理にsetterを使いました。
個数が5個以上かどうかはcalc_total_price関数にベタ書きしました。

Aさんはテストも通ったので安心しました(伏線)。
しかし、実はこの時点でバグがあります。皆さんは分かりますでしょうか？

仕様4,5

この改修をリリースしてすぐ、問題が発覚しました。

(略)

 // 仕様1: 単価*個数が返ってくること
 assert(calc_total_price(200, 4) === 800);
 
 // 仕様2: 単価がマイナスではエラーとなること
 assert(calc_total_price(-500, 4) === 0);
 
 // 仕様3: 個数が5個以上なら単価は20円引きとなること
 assert(calc_total_price(200, 5) === 900);
 
+// バグ: 合計額がマイナスになる!?
+echo calc_total_price(15, 10) . PHP_EOL;
+
 echo 'all green' . PHP_EOL;
 
(中略)

php calc.php

-50
all green

なんと引数となる単価が20円以下かつ個数が5個以上のケースで、DBに保存される単価および合計金額がマイナスになってしまいます。
ちゃんとマイナスにならないようにvalidates関数を入れたのに何故・・・と思ったところでAさんは処理順が間違えていたことに気づきました。

この設計の場合、バリデーションはDBに保存するギリギリで行うべきですが、バリデーション後にsetterを呼んでしまっています。
これではバリエーションの意味がなくなってしまっています。
よってAさんは処理順を修正することにしました。

加えて、そもそものビジネス上の仕様として、単価が20円以下かつ個数が5個以上のケースが考慮されていないことに気付きます。
Aさんは仕様のミスだと怒ることによって、体裁を保つことができました^[5]。
以下の仕様を追加することになりました。

単価が100円以下のときには単価の値引きしない

また、合計額についても値引きの追加仕様が入ることになりました。

合計額が3000円以上のときには合計額は300円引きとなる

Aさんはちょっとバグが怖いなーと言いながら実装し、最終的に以下の実装となりました。

 <?php
 
 function calc_total_price(int $unitPrice, int $quantity): int
 {
     $itemOrder = new ItemOrder($unitPrice, $quantity, new DateTimeImmutable());
-    if (!$itemOrder->validates()) {
-        return 0;
-    }
-
-    if (5 <= $itemOrder->getQuantity()) {
+    if (5 <= $itemOrder->getQuantity() && 100 < $itemOrder->getUnitPrice()) {
         $discountedPrice = $itemOrder->getUnitPrice() - 20;
         $itemOrder->setUnitPrice($discountedPrice);
     }
 
+    if (!$itemOrder->validates()) {
+        return 0;
+    }
+
     $itemOrderRepository = new ItemOrderRepository();
     if (!$itemOrderRepository->persist($itemOrder)) {
         return 0;
     }
 
-    return $itemOrder->getUnitPrice() * $itemOrder->getQuantity();
+    $totalPrice = $itemOrder->getUnitPrice() * $itemOrder->getQuantity();
+    if (3000 <= $totalPrice) {
+        $totalPrice = $totalPrice - 300;
+    }
+
+    return $totalPrice;
 }
 
 // 仕様1: 単価*個数が返ってくること
 assert(calc_total_price(200, 4) === 800);
 
 // 仕様2: 単価がマイナスではエラーとなること
 assert(calc_total_price(-500, 4) === 0);
 
 // 仕様3: 個数が5個以上なら単価は20円引きとなること
 assert(calc_total_price(200, 5) === 900);
 
 // バグ: 合計額がマイナスになる!?
 echo calc_total_price(15, 10) . PHP_EOL;
 
+// 仕様4: 単価が100円以下のときには単価の値引きをしないこと
+assert(calc_total_price(10, 5) === 50);
+
+// 仕様5: 合計額が3000円以上のときには合計額は300円引きとなること
+assert(calc_total_price(1000, 4) === 3700);
+
 echo 'all green' . PHP_EOL;

(中略)

php calc.php

150
all green

この実装にはとりあえずバグはありません。
ただ、最初は簡単な処理であったはずのcalc_total_priceがいつのまにか結構太ってきています。
Aさんはこう言ってます。でも仕様を満たしてるから別に良いですよね？(伏線)

追加の関数

新しく関数を追加することになりました。

アイテムの合計額が10000円以上にならないかを確認する^[6]
確認するだけで、DBには保存しない

「基本的には前の処理を使えば良いはずだから簡単ですよね？」と言われてしまいました。
Aさんも最初は単純にcalc_total_price関数を使うだけで良いと思っていましたが、calc_total_price関数内のDBへの保存が邪魔していることに気付きました。
仕方なく以下の実装にしました。

<?php

require_once 'calc.php';

function is_over(int $unitPrice, int $quantity): bool
{
    $itemOrder = new ItemOrder($unitPrice, $quantity, new DateTimeImmutable());

    // また同じような処理を書いてる・・・
    if (5 <= $itemOrder->getQuantity() && 100 < $itemOrder->getUnitPrice()) {
        $discountedPrice = $itemOrder->getUnitPrice() - 20;
        $itemOrder->setUnitPrice($discountedPrice);
    }

    if (!$itemOrder->validates()) {
        return false;
    }

    $totalPrice = $itemOrder->getUnitPrice() * $itemOrder->getQuantity();
    if (3000 <= $totalPrice) {
        $totalPrice = $totalPrice - 300;
    }

    return 10000 <= $totalPrice;
}

// 仕様1: 合計額が10000円未満 (1000 - 20) * 10 - 300 = 9500 < 10000
assert(is_over(1000, 10) === false);

// 仕様2: 合計額が10000円 (1050 - 20) * 10 - 300 >= 10000
assert(is_over(1050, 10) === true);

echo 'all green' . PHP_EOL;

php is_over.php

all green

なんとcalc_total_price関数をほぼコピペして実装してしまいました。
もちろんAさんもDRY原則は知っていましたが、もう色々考えるのが面倒になってしまったようです。

何が問題だったか

この記事はgetterとsetterについての記事です^[7]。
Aさんの設計は以下の2つの問題点があります。

setterによってオブジェクトを破壊してしまっている

        $discountedPrice = $itemOrder->getUnitPrice() - 20;
        $itemOrder->setUnitPrice($discountedPrice);

Aさんは仕様3を満たすためにsetterを導入しました。
しかし、結果的にはオブジェクトの正しい状態を壊してしまい、バグを発生させてしまいました。
バリデーションの処理順を変えることで一旦は解決しているように見えますが、バリデーションとsetterの順番という暗黙的な約束を作ってしまっていて、今でも潜在的な危険性を持っています。
まだAさんが実装しているうちは問題ないかもしれませんが、第三者がこの部分を改修したときにバグを起さない保証はないでしょう。

getterで取得した値を使ってビジネスロジックを書いてしまっている

    if (5 <= $itemOrder->getQuantity() && 100 < $itemOrder->getUnitPrice()) {

    $totalPrice = $itemOrder->getUnitPrice() * $itemOrder->getQuantity();
    if (3000 <= $totalPrice) {
        $totalPrice = $totalPrice - 300;
    }

getterは便利で、Aさんはgetterで取得した値を元に判定を行ったり、計算したりしています。
しかし、結果的に「追加の関数」のときにこの処理と同じ処理を書くことになってしまいました。
分岐・加工・計算というのは一般的にはビジネスロジックです。
このビジネスロジックを書く場所が分散されてしまっています。
後日このビジネスロジックの仕様を確認したいときに、おそらくcalc_total_price関数とis_over関数の2つを追う必要に迫られるでしょう。ビジネスロジックの可読性を下げています。
また、同じような追加機能の要望が来た時にもやはり同じようにコピペで対応するしかなくなると思います。ビジネスの再利用性も下げていることが分かります。

この分岐・計算はよく見るとItemOrderクラスのインスタンス変数だけでも可能であることに気付きます。
calc_total_price関数に書く必要はなく、ItemOrderクラスにこれらのビジネスロジックを書いておけば良かったのです。
なぜItemOrderクラスにビジネスロジックを書くべきかというと、ビジネスロジックのインプットとなるデータである単価、数量はItemOrderクラスが持っているからです。
ItemOrderクラスがビジネスロジックを持つことにより、データが使われるビジネスロジックがどこなのか、ビジネスロジックのインプットとなるデータはどれなのかが両方とも追いやすくなります。
calc_total_price関数にビジネスロジック、ItemOrderクラスにデータ、というように分離しているわけでもないので、再利用性も高まります。

つまり、データとビジネスロジックは密結合にしておくべきだったのに、getterによりビジネスロジックをデータを持っているオブジェクトの外へと流出させてしまったのが問題でした^[8]。
ビジネスロジックを書いている場所が分散してしまったことで、ビジネスロジックの可読性と再利用性を下げています。
分かりやすいサインとして、getterで取得した値を分岐・加工・計算に使っていたらアンチパターンのサイン、と覚えておくと良いと思います^[9]。

一方、逆にgetterを使わざるを得ないのが以下の部分です。

        $params = [
            'unit_price' => $itemOrder->getUnitPrice(),
            'quantity' => $itemOrder->getQuantity(),
            'ordered_at' => $itemOrder->getOrderedAt()->getTimestamp(),
        ];
        // アイテムオーダーをDBとかに保存する処理が書かれてるとします

DBに保存するときにはinteger, stringなどのプリミティブな値として取り出し、出力する必要が出てきます。
例はDBですが、それ以外でもMVCのViewの部分、APIリクエスト・レスポンスの値など、いわゆるインターフェース層にあたる部分に対しては同様に出力する必要が出てくると思います。
getterは完全になくすことは出来ません。
よって、getterを生やした上で、レビュー等でビジネスロジックをオブジェクト外に流出させていないかを指摘し合うという運用でカバーする方針になるかと思います。

設計の改善

ここからは浮かび上がった2つの問題を改善するリファクタリングをしていきます。

setterの根絶

setterを根絶する方法は、そもそもオブジェクト生成(コンストラクタ)の段階でオブジェクトとして完成させることです。
「ItemOrderクラスとして生成できた = オブジェクトとして正しい状態である」というルールにします。
具体的には以下の実装になります。

 <?php
 
 function calc_total_price(int $unitPrice, int $quantity): int
 {
-    $itemOrder = new ItemOrder($unitPrice, $quantity, new DateTimeImmutable());
-    if (5 <= $itemOrder->getQuantity() && 100 < $itemOrder->getUnitPrice()) {
-        $discountedPrice = $itemOrder->getUnitPrice() - 20;
-        $itemOrder->setUnitPrice($discountedPrice);
-    }
-
-    if (!$itemOrder->validates()) {
+    try {
+        $itemOrder = new ItemOrder($unitPrice, $quantity, new DateTimeImmutable());
+    } catch (Exception $e) {
+        return 0;
+    }
 
     $itemOrderRepository = new ItemOrderRepository();
     if (!$itemOrderRepository->persist($itemOrder)) {
         return 0;
     }
 
     $totalPrice = $itemOrder->getUnitPrice() * $itemOrder->getQuantity();
     if (3000 <= $totalPrice) {
         $totalPrice = $totalPrice - 300;
     }
 
     return $totalPrice;
 }
 
(中略)
 
 class ItemOrder
 {
     private int $unitPrice;
 
     private int $quantity;
 
     private DateTimeImmutable $orderedAt;
 
     public function __construct(int $unitPrice, int $quantity, DateTimeImmutable $orderedAt)
     {
+        if (5 <= $quantity && 100 < $unitPrice) {
+            $unitPrice = $unitPrice - 20;
+        }
+
+        if ($unitPrice <= 0) {
+            throw new Exception('invalid unit price');
+        }
+
+        if ($quantity <= 0) {
+            throw new Exception('invalid quantity');
+        }
+
         $this->unitPrice = $unitPrice;
         $this->quantity = $quantity;
         $this->orderedAt = $orderedAt;
     }
 
     public function getUnitPrice(): int
     {
         return $this->unitPrice;
     }
 
     public function getQuantity(): int
     {
         return $this->quantity;
     }
 
     public function getOrderedAt(): DateTimeImmutable
     {
         return $this->orderedAt;
     }
-
-    public function validates(): bool
-    {
-        return 0 < $this->unitPrice && 0 < $this->quantity;
-    }
-
-    public function setUnitPrice(int $unitPrice)
-    {
-        $this->unitPrice = $unitPrice;
-    }
 }
 
(中略)

実装について簡単に説明します。

オブジェクト生成時の引数がオブジェクトとして正しくない場合は例外を投げ、そもそもオブジェクト生成ができないようにする^[10]
単価値引きはそもそもコンストラクタ時で完結させ、後でsetする必要がないようにする^[11]
オブジェクト生成ができなかったときのハンドリングは使用側に任せる(この場合catchしてreturnさせています)

この実装のメリットとして、以下の3つがあります。

setterによってオブジェクトが破壊される恐れがなくなった。
バリデーションメソッドが不要になった。正確にはオブジェクト生成ができているというのがバリデーションに通ってることを意味するようになった。
calc_total_price関数の処理が減った。

getterによるビジネスロジックの流出を阻止

次にgetterを使ってビジネスロジックをオブジェクト外に流出させてしまっている箇所を改善します。
こちらは簡単で、ビジネスロジックをオブジェクトに持たせるだけで良いです。

 <?php
 
 function calc_total_price(int $unitPrice, int $quantity): int
 {
     try {
         $itemOrder = new ItemOrder($unitPrice, $quantity, new DateTimeImmutable());
     } catch (Exception $e) {
         return 0;
     }
 
     $itemOrderRepository = new ItemOrderRepository();
     if (!$itemOrderRepository->persist($itemOrder)) {
         return 0;
     }
 
-    $totalPrice = $itemOrder->getUnitPrice() * $itemOrder->getQuantity();
-    if (3000 <= $totalPrice) {
-        $totalPrice = $totalPrice - 300;
-    }
-
-    return $totalPrice;
+    return $itemOrder->totalPrice();
 }

(中略) 
 
 class ItemOrder
 {
     private int $unitPrice;
 
     private int $quantity;
 
     private DateTimeImmutable $orderedAt;
 
     public function __construct(int $unitPrice, int $quantity, DateTimeImmutable $orderedAt)
     {
-        if (5 <= $quantity && 100 < $unitPrice) {
+        if (self::isUnitPriceDiscounted($unitPrice, $quantity)) {
             $unitPrice = $unitPrice - 20;
         }
 
         if ($unitPrice <= 0) {
             throw new Exception('invalid unit price');
         }
 
         if ($quantity <= 0) {
             throw new Exception('invalid quantity');
         }
 
         $this->unitPrice = $unitPrice;
         $this->quantity = $quantity;
         $this->orderedAt = $orderedAt;
     }
 
     public function getUnitPrice(): int
     {
         return $this->unitPrice;
     }
 
     public function getQuantity(): int
     {
         return $this->quantity;
     }
 
     public function getOrderedAt(): DateTimeImmutable
     {
         return $this->orderedAt;
     }
+
+    public function totalPrice(): int
+    {
+        $totalPrice = $this->unitPrice * $this->quantity;
+        if ($this->isTotalPriceDiscounted()) {
+            $totalPrice = $totalPrice - 300;
+        }
+
+        return $totalPrice;
+    }
+
+    private function isTotalPriceDiscounted(): bool
+    {
+        return 3000 <= $this->unitPrice * $this->quantity;
+    }
+
+    private static function isUnitPriceDiscounted(int $quantity, int $unitPrice): bool
+    {
+        return 5 <= $quantity && 100 < $unitPrice;
+    }
 }
 
(中略)

最終実装

<?php

function calc_total_price(int $unitPrice, int $quantity): int
{
    try {
        $itemOrder = new ItemOrder($unitPrice, $quantity, new DateTimeImmutable());
    } catch (Exception $e) {
        return 0;
    }

    $itemOrderRepository = new ItemOrderRepository();
    if (!$itemOrderRepository->persist($itemOrder)) {
        return 0;
    }

    return $itemOrder->totalPrice();
}

// 仕様1: 単価*個数が返ってくること
assert(calc_total_price(200, 4) === 800);

// 仕様2: 単価がマイナスではエラーとなること
assert(calc_total_price(-500, 4) === 0);

// 仕様3: 個数が5個以上なら単価は20円引きとなること
assert(calc_total_price(200, 5) === 900);

// バグ: 合計額がマイナスになる!?
echo calc_total_price(15, 10) . PHP_EOL;

// 仕様4: 単価が100円以下のときには単価の値引きをしないこと
assert(calc_total_price(10, 5) === 50);

// 仕様5: 合計額が3000円以上のときには合計額は300円引きとなること
assert(calc_total_price(1000, 4) === 3700);

echo 'all green' . PHP_EOL;


class ItemOrder
{
    private int $unitPrice;

    private int $quantity;

    private DateTimeImmutable $orderedAt;

    public function __construct(int $unitPrice, int $quantity, DateTimeImmutable $orderedAt)
    {
        if (self::isUnitPriceDiscounted($unitPrice, $quantity)) {
            $unitPrice = $unitPrice - 20;
        }

        if ($unitPrice <= 0) {
            throw new Exception('invalid unit price');
        }

        if ($quantity <= 0) {
            throw new Exception('invalid quantity');
        }

        $this->unitPrice = $unitPrice;
        $this->quantity = $quantity;
        $this->orderedAt = $orderedAt;
    }

    public function getUnitPrice(): int
    {
        return $this->unitPrice;
    }

    public function getQuantity(): int
    {
        return $this->quantity;
    }

    public function getOrderedAt(): DateTimeImmutable
    {
        return $this->orderedAt;
    }

    public function totalPrice(): int
    {
        $totalPrice = $this->unitPrice * $this->quantity;
        if ($this->isTotalPriceDiscounted()) {
            $totalPrice = $totalPrice - 300;
        }

        return $totalPrice;
    }

    private function isTotalPriceDiscounted(): bool
    {
        return 3000 <= $this->unitPrice * $this->quantity;
    }
    
    private static function isUnitPriceDiscounted(int $quantity, int $unitPrice): bool
    {
        return 5 <= $quantity && 100 < $unitPrice;
    }
}

class ItemOrderRepository
{
    public function persist(ItemOrder $itemOrder): bool
    {
        $params = [
            'unit_price' => $itemOrder->getUnitPrice(),
            'quantity' => $itemOrder->getQuantity(),
            'ordered_at' => $itemOrder->getOrderedAt()->getTimestamp(),
        ];
        // アイテムオーダーをDBとかに保存する処理が書かれてるとします

        return true;
    }
}

ほぼ不要と思いますが、実装について簡単に説明します。

合計額の計算のビジネスロジックをgetterを使ってオブジェクト外に流出させずに、ItemOrderクラスに持たせた。
ifの分岐をprivateメソッドとして命名することで、どういうビジネスロジックなのかをコードとして明文化した^[12]。

これによってビジネスロジックの可読性・再利用性が高まっているはずです。

追加の関数により可読性・再利用性が向上したことの確認

上のリファクタリングによって、追加の関数がどうなったかを確認します。

<?php

require_once 'calc.php';

function is_over(int $unitPrice, int $quantity): bool
{
    try {
        $itemOrder = new ItemOrder($unitPrice, $quantity, new DateTimeImmutable());
    } catch (Exception $e) {
        return false;
    }

    return 10000 <= $itemOrder->totalPrice();
}

// 仕様1: 合計額が10000円未満 (1000 - 20) * 10 - 300 = 9500 < 10000
assert(is_over(1000, 10) === false);

// 仕様2: 合計額が10000円 (1050 - 20) * 10 - 300 >= 10000
assert(is_over(1050, 10) === true);

echo 'all green' . PHP_EOL;

php is_over.php

all green

元の実装と比べれば、以下のことが分かると思います。

コンストラクタとメソッド呼び出しの2手のみの手続きで済んでいる。
あまりItemOrderクラスの中身を理解せずとも処理を書けている^[13]。
DRYとなり、ビジネスロジックの再利用性が高まっている。

まとめ

getter/setterの善悪は一律で判断できない。
setterはバグの温床になりうるので悪。
setterを根絶するにはオブジェクト生成の段階でオブジェクトとして完成させる。
getterは悪とされるパターンとそうでないパターンが存在する。
インターフェース層のためにgetterが必要となる。
getterによりビジネスロジックをオブジェクトの外へと流出させてしまうのは悪。

補足

オブジェクト指向をちゃんと勉強したい場合は「現場で役立つシステム設計の原則」を読めば間違いないです。全人類、この本を読みましょう。
例えばRailsのActiveRecordはこの記事の思想とは大きく反していると思います。やっぱり現場によるので、柔軟に対応しましょう。
ドヤ顔で解説記事を書きましたが、筆者もオブジェクト指向にそこまで自信があるわけではないです^[14]。コメント・マサカリを歓迎しております。

脚注

設計によってはgetterを使わなくても良い設計にできることもあるようですが、入門者はあまり気にしなくて良いと思います。 ↩︎
PHPUnitはこの記事の趣旨に対してオーバーエンジリアリングです。 ↩︎
モデリングに関してはノーコメントでお願いします。 ↩︎
リポジトリや現在時刻のDIでの実装はこの記事の趣旨に対してオーバーエンジリアリングです。 ↩︎
もちろんビジネスサイドも悪いかもしれませんが、こういう仕様の抜け道はエンジニアが気付きやすいので、気付いて指摘するべきですね。 ↩︎
そんな特定のユースケースなんてある？？っていうツッコミは無しでお願いします。 ↩︎
唐突な再宣言ですが、これがないとgetter/setter以外の設計に対する大量のマサカリを受けることになる、と供述しています。 ↩︎
ここまでドヤ顔で解説していますが、「現場で役立つシステム設計の原則」に同じことが書いてあります。 ↩︎
アンチパターンのサインというだけで、絶対にNGというわけではありません。結局はどこの責務なのかという話になります。 ↩︎
本来は独自例外クラスを定義すべきですが、この記事の趣旨に対してオーバーエンジリアリングです。 ↩︎
アイテムオーダーのクラスと、単価が値引きされた結果のクラスは、ちゃんと分けるべきって？僕もそう思います。 ↩︎
数字の定数化は面倒だった。いやこの記事の趣旨に対してオーバーエンジリアリングですよ(?)。 ↩︎
でもそれってマジでクールなことでさ、オレは知る必要がないのさ ↩︎
この記事書き終わった後にTwitterのTL見たら、偶然にもオブジェクト指向の話がバチバチに盛り上がっていて怖くなった、と供述しています。 ↩︎

Discussion

saitofjp 2021/09/27に更新

setter/getterの議論の範囲で言えば、getterは悪なのです。

インターフェース層などでプリミティブな値を出力するケース

これはレイヤーをまたがる際の共通的な悩みですね。。ですが、オブジェクトをレイヤ超えて持ち出すのがそもそも望ましくなかったりします。

最近だとクリーンアーキテクチャという本が、そのまんまの解答を提示していて、依存性の逆転を使うことで、レイヤーを超えたオブジェクトの受け渡しが可能になります。
レイヤーを超える必要がない場合、提示されたユースケースが消えるためgetterは悪となります。

例えば、ItemOrderに下記のような拡張をします。

// ドメインレイヤーのオブジェクト
class ItemOrder {
   writeInfo(OrderInfoWriter  ifw)
}
interface OrderInfoWriter {}

// veiw レイヤーのオブジェクト
class OrderInfoDtoWriterImpl impl OrderInfoWriter {}

するとこんな風に使えます。

// viewレイヤ
OrderInfoWriter ifw = new OrderInfoDtoWriterImpl ()

// ビジネスレイヤ
// ビジネスロジックが終わったら、writerに書き出す。
itemOrder.writeInfo(ifw)

// viewレイヤ 好きに描画する
OrderInfo info = ifw.buildDto()
info.unitPrice
...

ItemOrder には、getterが全く必要ありません。ただし、OrderInfo が結局getterの代わりにそのままアクセスしてるだけじゃん。となりますね。これは、構造体やDTOと呼ばれる情報を保持するオブジェクトです。
ItemOrderの中身ではなく、描画するためのOrderの情報が格納されているだけになります。

ここでsetter/getterの議論の難しさが登場します。
オブジェクトの責務に備わないsetter/getter(限らずメソッド全般)は悪だとなります。
そのため、ロジックを実行するためのオブジェクトにstter/getterは必要ありません。

さらに、setter/getterではなく、プロパティアクセスという方法があります。
情報保持オブジェクトは、保持役のためプロパティアクセスでも安全ならでそれでよいのです。
プロパティ系の構文を持っていない言語では、文法的な統一性やReadOnlyのため、プロパティアクセスのような意図でgetterを挟むことが良くあります。

さらに、ややこしいのが、OOPにもMutableな思想が根付きました。
たとえ情報保持オブジェクトでも、setterを持つ必要性がなくなってきた。
そのため、情報保持オブジェクトは、取得のみプロパティアクセスできれば良い。となり始めました。

さらに、さらにややこしいのが、実用面的なトレードオフからActiveRecordのような意図的に責務を兼任させるオブジェクト指向のパターンもある。レイヤーをまたがる場合のオーバヘッドをトレードオフとするわけですね。

結果として、ItemOrder にgetterがはえてても良いと考えてもいいんじゃね。となります。
逆に言えば、言語仕様的な制限やトレードオフ的な部分も全部クリアして、超理想論にまで突き詰めた場合、オブジェクト指向のカプセル化の原則に違反するsetter/getterは、悪になります。

setter/getterだけで、ここまで実はオブジェクト指向の真髄に迫れるのが面白く、思わずコメントさせて頂きましたｗ

熊木 2021/09/28

ありがとうございます！
なるほど、別レイヤからは単にgetterを使うのではなく、データ描写だけされたDTOを使うってことですね。
確かにその方法でgetterも根絶できそうです、いやめちゃくちゃ勉強になりました。

プロパティ系の構文を持っていない言語では、文法的な統一性やReadOnlyのため、プロパティアクセスのような意図でgetterを挟むことが良くあります。

そもそもプロパティアクセスをしたいならgetterという選択肢しかない言語もあるわけですね。
なのでDTOでも「getter」をはやさなくちゃいけず、それがgetter許容論に曲解されてしまうと。

さらに、さらにややこしいのが、実用面的なトレードオフからActiveRecordのような意図的に責務を兼任させるオブジェクト指向のパターンもある。

これはかなりややこしいですよね。ActiveRecordだとむしろ本記事のAさんみたいに、setterで詰め込んでDB保存前にvalidateメソッドさえ走らせれば良い、という設計になっているので。
Rails出身の方がDDD・オブジェクト指向を学び直したときになかなか苦労しそうだと思ってます。

逆に言えば、言語仕様的な制限やトレードオフ的な部分も全部クリアして、超理想論にまで突き詰めた場合、オブジェクト指向のカプセル化の原則に違反するsetter/getterは、悪になります。

そうですね、少なくとも純粋なドメインを表現するオブジェクトには、前述の方法でgetterは不要そうです。