LoginSignup
2
0

More than 5 years have passed since last update.

Setter(アクセサ)に型チェック以外の値のバリデーションを含めるのは、『単一責任の原則』に2箇所で違反し、『間違うことが難しいコードを書くという原則』にも違反している という説

Posted at

こちらの記事のコメントで、 @4kizuki に以下のように言われました。

リーダブルコードにあるように、コンストラクタやアクセサ(Getter & Setter)は、"アクセスする"以外の処理は(本当は)すべきではありません。しかし、型チェックまではしても良いと思います。理由は、少なくともコンパイル言語はコンパイル時にエラーになるので明らかに「基本的な実装ミス(エラー)」だからです。

この点に関しては、少し議論の余地があると思います。間違うことが難しいコードを書くという原則では、コンパイラやIDEが間違いだと判定できるという意味でSetterにおいて型チェックを行うというのは当然ですが、Setterがそれしか行わない場合、オブジェクトの正しさが失われてしまいます。これは当然ながら問題です。プログラムのすべての時点においてオブジェクトの正しさを保障したい場合、Setterがある程度のバリデーションを行うのは仕方がないと考えています。
(一部抜粋)

以上の発言は、「 Biulder パターンで組み立てる要素を追加していく各メソッド」を Setter と言っていたので、特に異論はありませんでした。

この記事では、上記の記事でのやり取りから、「 Biulder パターンである」という前提を「 ValueObject である」にした場合(より一般的でもとの意味に近い Setter の場合)の話をします。

ValueObject の Setter でバリデーションをするとどうなるか

『Setterがある程度のバリデーションを行う』事自体が、「オブジェクトの正しさを失う」と同時に「間違うことが難しいコードを書くという原則に違反する」という、多重に原則違反をしている状態になります。

  • 「コンストラクタとアクセサは、データ構造を実装する特殊なメソッド」なので、検証の"アルゴリズム"を書かないべき
    • 普通、メソッドはアルゴリズムを実装するもの
    • コンストラクタとアクセサは、例外
  • Setter(Accessor)の責任は「アクセス可能性の表現」であり、「データの妥当性の検証」ではない
  • 「データの妥当性の検証(バリデーション)」は、バリデーションを責任としている箇所に集約

「Setter の引数で型をバリデーションすべき」については同意

型は自明なので、チェックすべきだと考えています。特に、プリミティブな型までであれば、積極的にチェックすべきでしょう。
なぜならば、少なくともコンパイル言語はコンパイル時にエラーとして検出できる、 明らかに「基本的な実装ミス(エラー)」だから です。例えば、"年齢" に DateTime や DirectoryIterator になることはおそらくありません。 int か float などの Number でしょう。

しかし、年齢が「自然数」というのは自明ではありません。なぜならば、「ヶ月を小数点以下で表現する」とか「受胎後~誕生以前を、マイナスの浮動小点数で表現する」などは十分にありえるからです。

以上より『年齢は「自然数」ではない』については、「現在の仕様」としては正しいかもしれませんが、絶対的な正しさではないことがわかります。少なくとも変更される可能性があるため、変更時に変更しなければいけない箇所の分散を避けるために、アクセサ(Setter)でバリデーション"しない"べきでしょう。

以上の理論は、『データ構造の選択はおそらく絶対的に正確である。なぜならば、変更される場合には使用する側と使用される側の両方のアルゴリズムの変更も必要なため、容易に変更されない。対して、アルゴリズムはデータを取り扱うだけなので、アルゴリズムの変更のみで完結可能であることが多い。そのため、アルゴリズムは頻繁にもしくは積極的に変更される。よって、各アルゴリズムを責任ごとに密集すさせるべき。』という前提の上に成り立っています。

バリデーションはバリデーションを責任とする箇所で行うべきです。アクセサの責任は「アクセスの保証」というデータ構造的なものであり、「変数の妥当性の検証」というアルゴリズム的なものではありません。

まとめ: アクセサにバリデーションのアルゴリズムは書かないべき

以上の事より、「アクセサでバリデーションをする」という選択肢そのものが、「アクセサの責任の範疇ではないものがアクセサに含まれる」&「バリデータにそのの責任を集約できていない」という点で、単一責任の原則に2度も違反することになります。単一責任の原則に違反している時点で、その実装は「間違うことが難しいコードを書く」という原則にも違反しているといえるでしょう。

/////////// 以下、ふるいメモ ///////////

こちらの記事のコメントで、 @4kizuki に以下のように言われました。

リーダブルコードにあるように、コンストラクタやアクセサ(Getter & Setter)は、"アクセスする"以外の処理は(本当は)すべきではありません。しかし、型チェックまではしても良いと思います。理由は、少なくともコンパイル言語はコンパイル時にエラーになるので明らかに「基本的な実装ミス(エラー)」だからです。

この点に関しては、少し議論の余地があると思います。間違うことが難しいコードを書くという原則では、コンパイラやIDEが間違いだと判定できるという意味でSetterにおいて型チェックを行うというのは当然ですが、Setterがそれしか行わない場合、オブジェクトの正しさが失われてしまいます。これは当然ながら問題です。プログラムのすべての時点においてオブジェクトの正しさを保障したい場合、Setterがある程度のバリデーションを行うのは仕方がないと考えています。
(一部抜粋)

これについて、『Setterがある程度のバリデーションを行う』事自体が、「オブジェクトの正しさを失うことになっている」ということと、「間違うことが難しいコードを書くという原則に違反している」と思います。

論旨を列挙すると、

  • 「コンストラクタとアクセサは、データ構造を実装する特殊なメソッド」なので、検証の"アルゴリズム"を書かないべき
    • 普通、メソッドはアルゴリズムを実装するもの
    • コンストラクタとアクセサは、例外
  • Setter(Accessor)の責任は「アクセス可能性の表現」であり、「データの妥当性の検証」ではない
  • 「データの妥当性の検証(バリデーション)」は、バリデーションを責任としている箇所に集約

「Setter の引数で型をバリデーションすべき」については同意

型は自明なので、チェックすべきだと考えています。
なぜならば、少なくともコンパイル言語はコンパイル時にエラーとして検出できる、 明らかに「基本的な実装ミス(エラー)」だから です。例えば、"年齢" に DateTime や DirectoryIterator になることはおそらくありません。 int か float などの Number でしょう。

しかし、年齢が「自然数」というのは自明ではありません。なぜならば、「ヶ月を小数点以下で表現する」とか「受胎後~誕生以前を、マイナスの浮動小点数で表現する」などは十分にありえるからです。

以上より『年齢は「自然数」ではない』については、「現在の仕様」としては正しいかもしれませんが、絶対的な正しさではないことがわかります。少なくとも変更される可能性があるため、変更時に変更しなければいけない箇所の分散を避けるために、アクセサ(Setter)でバリデーション"しない"べきでしょう。

以上の理論は、『データ構造の選択はおそらく絶対的に正確である。なぜならば、変更される場合には使用する側と使用される側の両方のアルゴリズムの変更も必要なため、容易に変更されない。対して、アルゴリズムはデータを取り扱うだけなので、アルゴリズムの変更のみで完結可能であることが多い。そのため、アルゴリズムは頻繁にもしくは積極的に変更される。よって、各アルゴリズムを責任ごとに密集すさせるべき。』という前提の上に成り立っています。

バリデーションはバリデーションを責任とする箇所で行うべきです。アクセサの責任は「アクセスの保証」というデータ構造的なものであり、「変数の妥当性の検証」というアルゴリズム的なものではありません。

まとめ: アクセサにバリデーションのアルゴリズムは書かないべき

以上の事より、「アクセサでバリデーションをする」という選択肢そのものが、「アクセサの責任の範疇ではないものがアクセサに含まれる」&「バリデータにそのの責任を集約できていない」という点で、単一責任の原則に2度も違反することになります。単一責任の原則に違反している時点で、その実装は「間違うことが難しいコードを書く」という原則にも違反しているといえるでしょう。

付録

私の考える、「年齢とは自然数である」というドメイン定義をされた場合のミニマルな実装

以下のように実装する。

Age.php
<?php
class Age
{
    /** @var int $age */
    private $age;

    private function __construct($age)
    {
        // コンストラクタ内では、意地でもプロパティの初期化以外を書かない
        $this->age = $age;
    }

    public static function create($age)
    {
        // validate
        if ( !(is_int($age) && (0 < $age)) ) {
            throw new InvalidArgumentException("年齢は自然数");
        }

        return new static($age);
    }
}
2
0
1

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
2
0