LoginSignup
51
46

More than 5 years have passed since last update.

@ProjectICKX(ICKX Project)inProject ICKX

意味定義への異常な愛情 ～または私は如何にして心配するのを止めて定義変更を愛するようになったか～

Last updated at Posted at 2016-08-16

世の中には"たまたまそうなっている値"に対して、安易な対応を行い、変更に弱くなったロジックが存在します。
この項では意味に対して明確な定義を行い、ロジックと分離する事で、仕様変更に強い状況を作りだせる事を解説します。

結論

ロジックと定義は明確に切り分けましょう。

なお、定義変更時、ロジックに影響があった場合、適切な切り分けが行われていない事を示します。

仕様の変遷による解説

都道府県表示：block 1

  • 都道府県コードが与えらた場合に都道府県名を表示する

変更に弱いロジック

//定義
class Pref {
    const PREF_CD_HOKKAIDOU = '1';
    ～中略～
    const PREF_CD_OKINAWA   = '47';

    public function getName ($pref_cd) {
        if ($pref_cd == self::PREF_CD_HOKKAIDOU) {
            $pref_name = '北海道';
        } else if ($pref_cd == self::PREF_CD_AOMORI) {
            $pref_name = '青森県';
        ～～中略～～
        } else if ($pref_cd == self::PREF_CD_OKINAWA) {
            $pref_name = '沖縄県';
        } else {
            $pref_name = false;
        }
        return $pref_name;
    }
}

//バリデーション
$pref = new Pref;
if ($pref->getName($pref_cd) == false) {
    new \Exception('エラーが発生しました。');
}

//実使用箇所
$pref = new Pref;
echo $pref->getName($pref_cd);

変更に強いロジック

//定義
class Pref {
    const PREF_CD_HOKKAIDOU = '1';
    ～中略～
    const PREF_CD_OKINAWA   = '47';

    const MAP = [
        self::PREF_CD_HOKKAIDOU => '北海道',
        ～中略～
        self::PREF_CD_OKINAWA   => '沖縄県',
    ];
}

//バリデーション
if (!isset(Pref::MAP[$pref_cd])) {
    new \Exception('エラーが発生しました。');
}

//実使用箇所
echo Pref::MAP[$pref_cd];

解説

信じがたい話ではありますが、この段階ではほとんど差が無いと認識される場合があります。

都道府県表示：block 2

  • 都道府県コードが与えらた場合に都道府県名を表示する
  • 併せて、東日本、西日本のエリアを作成し、エリア名 => 都道府県名 として表示する
  • 東日本は北海道から長野県、西日本は岐阜県から沖縄県とする

変更に弱いロジック

//定義
class Pref {
    const PREF_CD_HOKKAIDOU = '1';
    ～中略～
    const PREF_CD_OKINAWA   = '47';

    public function getName ($pref_cd) {
    ～block1と同一なので省略～
    }

    public function getArea ($pref_cd) {
        if ($pref_cd >= '1' && $pref_cd <= '20') {
            return '東日本';
        } else if ($pref_cd >= '21' && $pref_cd <= '47') {
            return '西日本';
        } else {
            return false;
        }
    }
}

//バリデーション
$pref = new Pref;
if ($pref->getName($pref_cd) == false) {
    new \Exception('エラーが発生しました。');
}
if ($pref->getArea($pref_cd) == false) {
    new \Exception('エラーが発生しました。');
}

//実使用箇所
$pref = new Pref;
echo sprintf('%s => %s', $pref->getArea($pref_cd), $pref->getName($pref_cd));

変更に強いロジック

//定義
class Pref {
    const PREF_CD_HOKKAIDOU = '1';
    ～中略～
    const PREF_CD_OKINAWA   = '47';

    const AREA_CD_EAST_JP   = '1';
    const AREA_CD_WEST_JP   = '2';

    const MAP = [
        ～中略～
    ];

    const AREA_MAP = [
        self::AREA_CD_EAST_JP   => '東日本',
        self::AREA_CD_WEST_JP   => '西日本',
    ];

    const PREF_CD_AREA_CD_MAP = [
        self::PREF_CD_HOKKAIDOU     => self::AREA_CD_EAST_JP,
        self::PREF_CD_AOMORI        => self::AREA_CD_EAST_JP,
        ～中略～
        self::PREF_CD_GIFU          => self::AREA_CD_WEST_JP,
        self::PREF_CD_SHIZUOKA      => self::AREA_CD_WEST_JP,
        self::PREF_CD_AICHI         => self::AREA_CD_WEST_JP,
        ～中略～
        self::PREF_CD_OKINAWA       => self::AREA_CD_WEST_JP,
    ];
}

//バリデーション
if (!isset(Pref::MAP[$pref_cd])) {
    new \Exception('エラーが発生しました。');
}
if (!isset(Pref::PREF_CD_AREA_CD_MAP[$pref_cd])) {
    new \Exception('エラーが発生しました。');
}

//実使用箇所
$area_cd = Pref::PREF_CD_AREA_CD_MAP[$pref_cd];
echo sprintf('%s => %s', Pref::AREA_MAP[$area_cd], Pref::MAP[$pref_cd]);

解説

ここでもまだ、目に見える差はでていません。

都道府県表示：block 3

  • 都道府県コードが与えらた場合に都道府県名を表示する
  • エリアを北海道、東日本、西日本、九州に分割し、エリア名 => 都道府県名 として表示する
  • 北海道は北海道のみ、東日本は青森県から長野県、西日本は岐阜県から高知県、九州は福岡県から沖縄県とする

変更に弱いロジック

//定義
class Pref {
    const PREF_CD_HOKKAIDOU = '1';
    ～中略～
    const PREF_CD_OKINAWA   = '47';

    public function getName ($pref_cd) {
    ～block1と同一なので省略～
    }

    public function getArea ($pref_cd) {
        if ($pref_cd == '1') {
            return '北海道';
        if ($pref_cd >= '2' && $pref_cd <= '20') {
            return '東日本';
        } else if ($pref_cd >= '21' && $pref_cd <= '39') {
            return '西日本';
        } else if ($pref_cd >= '40' && $pref_cd <= '47') {
            return '沖縄県';
        } else {
            return false;
        }
    }
}

//バリデーション
$pref = new Pref;
if ($pref->getName($pref_cd) == false) {
    new \Exception('エラーが発生しました。');
}
if ($pref->getArea($pref_cd) == false) {
    new \Exception('エラーが発生しました。');
}

//実使用箇所
$pref = new Pref;
echo sprintf('%s => %s', $pref->getArea($pref_cd), $pref->getName($pref_cd));

変更に強いロジック

//定義
class Pref {
    const PREF_CD_HOKKAIDOU = '1';
    ～中略～
    const PREF_CD_OKINAWA   = '47';

    const AREA_CD_EAST_JP   = '1';
    const AREA_CD_WEST_JP   = '2';
    const AREA_CD_HOKKAIDOU = '3';
    const AREA_CD_KYUSYU    = '4';

    const MAP = [
        ～中略～
    ];

    const AREA_MAP = [
        self::AREA_CD_EAST_JP   => '東日本',
        self::AREA_CD_WEST_JP   => '西日本',
        self::AREA_CD_HOKKAIDOU => '北海道',
        self::AREA_CD_KYUSYU    => '九州',
    ];

    const PREF_CD_AREA_CD_MAP = [
        self::PREF_CD_HOKKAIDOU     => self::AREA_CD_HOKKAIDOU,
        self::PREF_CD_AOMORI        => self::AREA_CD_EAST_JP,
        ～中略～
        self::PREF_CD_GIFU          => self::AREA_CD_WEST_JP,
        self::PREF_CD_SHIZUOKA      => self::AREA_CD_WEST_JP,
        self::PREF_CD_AICHI         => self::AREA_CD_WEST_JP,
        ～中略～
        self::PREF_CD_OKINAWA       => self::AREA_CD_KYUSYU,
    ];
}

//バリデーション
if (!isset(Pref::MAP[$pref_cd])) {
    new \Exception('エラーが発生しました。');
}
if (!isset(Pref::PREF_CD_AREA_CD_MAP[$pref_cd])) {
    new \Exception('エラーが発生しました。');
}

//実使用箇所
$area_cd = Pref::PREF_CD_AREA_CD_MAP[$pref_cd];
echo sprintf('%s => %s', Pref::AREA_MAP[$area_cd], Pref::MAP[$pref_cd]);

解説

ようやく目に見える差が出てきました。
変更に弱いロジックでは制御構造（IF文）そのものに修正が入っています。
そのため、制御構造自体のバグが発生する可能性が生まれてしまっています。
また、数値による範囲指定のため、ケアレスミスが発生しやすい状況となっています。

一方、変更に強いロジックでは設定配列を変更するのみで、制御構造には一切修正が入りません。
自明な事ですが、制御構造自体が変わる事によるバグは発生することがありえません。
また、定義とセットになっている箇所を変更するため、誤りに気付きやすい状況になっています。

都道府県表示：block 4

  • 都道府県コードが与えらた場合に都道府県名を表示する
  • エリアを北海道、東日本、西日本、九州に分割し、エリア名 => 都道府県名 として表示する
  • 動向解析の結果、静岡県は東日本所属にした方が良い事が判明した。静岡県は東日本へ移動せよ。

変更に弱いロジック

//定義
class Pref {
    const PREF_CD_HOKKAIDOU = '1';
    ～中略～
    const PREF_CD_OKINAWA   = '47';

    public function getName ($pref_cd) {
    ～block1と同一なので省略～
    }

    public function getArea ($pref_cd) {
        if ($pref_cd == '1') {
            return '北海道';
        } else if ($pref_cd >= '2' && $pref_cd <= '20' || $pref_cd == '22') {
            return '東日本';
        } else if ($pref_cd >= '21' && $pref_cd <= '39') { // <= 大変キドキする範囲指定。22：静岡県が含まれてしまっている。先行するifのor部分が無くなったら・・・？
            return '西日本';
        } else if ($pref_cd >= '40' && $pref_cd <= '47') {
            return '沖縄県';
        } else {
            return false;
        }
    }
}

//バリデーション
$pref = new Pref;
if ($pref->getName($pref_cd) == false) {
    new \Exception('エラーが発生しました。');
}
if ($pref->getArea($pref_cd) == false) {
    new \Exception('エラーが発生しました。');
}

//実使用箇所
$pref = new Pref;
echo sprintf('%s => %s', $pref->getArea($pref_cd), $pref->getName($pref_cd));

変更に強いロジック

//定義
class Pref {
    const PREF_CD_HOKKAIDOU = '1';
    ～中略～
    const PREF_CD_OKINAWA   = '47';

    const AREA_CD_EAST_JP   = '1';
    const AREA_CD_WEST_JP   = '2';
    const AREA_CD_HOKKAIDOU = '3';
    const AREA_CD_KYUSYU    = '4';

    const MAP = [
        ～中略～
    ];

    const AREA_MAP = [
        self::AREA_CD_EAST_JP   => '東日本',
        self::AREA_CD_WEST_JP   => '西日本',
        self::AREA_CD_HOKKAIDOU => '北海道',
        self::AREA_CD_KYUSYU    => '九州',
    ];

    const PREF_CD_AREA_CD_MAP = [
        self::PREF_CD_HOKKAIDOU     => self::AREA_CD_HOKKAIDOU,
        self::PREF_CD_AOMORI        => self::AREA_CD_EAST_JP,
        ～中略～
        self::PREF_CD_GIFU          => self::AREA_CD_WEST_JP,
        self::PREF_CD_SHIZUOKA      => self::AREA_CD_EAST_JP,
        self::PREF_CD_AICHI         => self::AREA_CD_WEST_JP,
        ～中略～
        self::PREF_CD_OKINAWA       => self::AREA_CD_KYUSYU,
    ];
}

//バリデーション
if (!isset(Pref::MAP[$pref_cd])) {
    new \Exception('エラーが発生しました。');
}
if (!isset(Pref::PREF_CD_AREA_CD_MAP[$pref_cd])) {
    new \Exception('エラーが発生しました。');
}

//実使用箇所
$area_cd = Pref::PREF_CD_AREA_CD_MAP[$pref_cd];
echo sprintf('%s => %s', Pref::AREA_MAP[$area_cd], Pref::MAP[$pref_cd]);

解説

ここにきて、大きな違いとなってきました。
変更に弱いロジックではor条件が入り、実装を見ただけでは「なんじゃこりゃ」な状況となっています。

一方、変更に強いロジックでは「県がどこに属しているのか？」が明確になっているため、意図的に配置が変更されている事が判ります。
また、例によりアクロバティックな仕様の変更でも、制御構造に対しての変更が発生していません。

仕様の変更を超えて

このように、ロジックと定義を明確に分け、意味に対して明確な定義を行う事により、変更に強い状況を作り出す事が出来ます。
また、修正や調査に掛かる工数も大幅に削減する事が出来ます。

項目 変更に弱いロジック 変更に強いロジック
修正対象の定義の調査 同様の処理を行っている箇所を調べ尽くす必要がある 定義配列とその使用箇所のみを見れば良い
判定の変更 同様の処理を行っている箇所を全て修正する必要がある 定義配列を修正するだけで良い
修正後のテスト 判定条件の構文エラーから疑いテストする必要がある 境界値テストを行えば網羅できる
validationの信頼性 "たまたま"整列されている数値に対しての範囲チェックのため、値のグループが変わった場合に破綻する 定義済みリストに居るか居ないかのみを見れば良いため、確実なチェックが可能

意味定義への異常な愛情

ストックついてから前提の記載が足りてない事を思い出した系。

何故に"異常な愛情"とするのか。
当時、都道府県コードや地域コードに対して定数化を行う文化圏に居なかった。
その為、"過剰対応"と揶揄される事もあった。
なお、恐るべきことにアンチパターンとしてのマジックナンバーは共有されていた。
「都道府県コードはどんな現場でも1～47の数値なのでマジックナンバーではない 1 2 」との主張だった。

上記から当然、都道府県マップも価値を認められられなかった。

それでも、当該の未来が見えていたので食いついて離さなかった。
傍から見れば異常そのものである。

故に"異常"な愛情としています。

元々持ち合わせていたスタイルなので、確信を持てていた事も大きいのですけどね。

余談

ちなみに、実際に適用出来た時点ではこんな状況にあった。

  • 処理分岐がサンプルコードそのままのifとして、多数の箇所に存在していた（これはまぁ突貫工事の余波なんで同情はする
  • 関数化されておらず「既存コードをコピペした方が安全」との判断から、必要になる度にifが増えていった
  • もちろん定数は使われておらず、del_flagの1と北海道の1と1円の1が識別できず、その1が北海道である事を保証するためには処理フロー全体を読む必要があった
  • 年に3～4回の頻度で更新があった

なので、次の修正を行った。

  • 定義を全て定数化 + 配列化
  • if内で個別に判定していたものを関数化

全部で3人週、60万円ほどかかった。

その上で、地域の修正が行われた場合の対比が以下。

影響範囲調査 修正 確認 コスト
修正前 地域情報が必要になる度にコピペされているため、全コード(n十万行)の調査が必要。2人週 対象箇所を全て修正する。2人日 それぞれ個別にロジックが壊れていないかの確認をした上で、それぞれ個別にテストが必要。2人日 14人日 当時で56万円
修正後 定義が1箇所に纏まっているため、そもそも調査自体が不要 配列を弄るだけ。15分 1箇所だけガチテストすればOK。あとは見た目の崩れが無いかだけ個別に確認。3時間 0.5人日 当時で2万円

ちなみに、修正前の説得材料はこんな感じでした。

  • 一回当たり54万円のコスト削減なので、2回目の修正が入った時点で採算とれます
  • 年間で3か月程度エンジニアの稼働を取れるようになります。小規模案件なら一個対応数増やせますね
  • "コード改修"が"定数の変更だけ"という"誰でも出来る作業"になるため、単価の高いエンジニア以外の稼働でも対応できるようになります

そりゃするね、修正するね、しちゃうよね。

エンジニアとして稼働した中で3番目に良い仕事(後進の作業量激減、コスト激減、機能・値の信頼性向上、作業に対する安全度の向上)をしたお話でした。

  1. おう、だったら"北海道"の使用箇所をロジック追跡せずにピックアップしてみろや。 ↩

  2. せめて論拠が「ISO 3166-2:JPに準拠しているから」ならまだ良かったんだけどね。"たまたま"「過去に自分が見たものがそうだったから、それが仕様なんだ」って姿勢は危険だと思う。 ↩

51
46
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
Sign upLogin
51
46