Zenn
Open17

良いコード/悪いコードで学ぶ設計入門

#
設計
#
まとめ
#
ミノ駆動本
きたやんきたやん

6章

  • 条件分岐がネスト、何しているかわからない
    • 早期リターンでネストを解消、条件ロジックと実行ロジックを分離
  • interfaceを使う
    • interface(抽象)に依存すると、型(クラス)判定用のロジックが不要になる。
  • Switch文が重複すると、case文の実装漏れが起きる。
  • ポリシーパターンで条件判定ロジックを部品化し、再利用できる。
    • laravelの認可で用いるPolicyと同じ設計
    • 条件分岐が多い部分で使える。
    • 条件分岐で使う各ロジックをルールクラスに切り出し→ポリシークラスからルールクラスを呼び出し、ルールを満たすか判断する設計。
  • interfaceをよく理解していないと、リスコフの置換原則に違反する。
    • interface を class に置換しても、動作するべきである
  • フラグ引数で動作を切り替えるべきではない
    • フラグを消して、動作ごとにメソッドを作成するべき。
    • ストラテジパターン で動作の切り替えを行う
きたやんきたやん

7章 コレクション

  • 車輪の再発明、四角い車輪の再発明をしない
  • 早期continueでループ処理をスキップする
  • ファーストクラスコレクションには、コレクション型インスタンス変数を配置する。ロジックでそれらを操作して、高凝集に保つ
  • コレクションを外部に渡す場合は読み取り専用にする
    • 外部で追加されると、低凝集に陥る
きたやんきたやん

8章 密結合

  • 同じように見えるロジックでも、ドメイン知識が違えば共通化するべきではない。
  • 継承は可能な限り避けるべき。
    • 親クラスに依存する。親クラスの内容を常に気にして実装する必要が出る。
    • 継承より委譲するべき。クラス内でprivateなインスタンス変数を持ち、呼び出すようにする。(コンポジション構造)
  • インスタンス変数、メソッドの依存関係を図にする→影響スケッチ
  • なんでもpublicにすると、他のパッケージやクラスから容易に参照される。
    • クラス内では整合性を保っていたのに、他の部分で改竄されてしまう。
  • クラスのサイズに気をつける。privateメソッドの数が多いクラスは、複数の責務を持ってしまっている可能性が高い。分割していくこと。
  • 疎結合高凝集であって、密結合高凝集ではない。
  • 表示を責務とするクラスの中に、表示以外のロジックが実装されている→スマートUI
  • 巨大データクラス→様々なユースケースで使われる→無関係なデータを変更してしまい、バグになる可能性が生じる。
  • メソッド内に一連の処理がダラダラと書き連ねてある構造→トランザクションスクリプトパターン。(手続型プログラミング)
  • 神クラス→トランザクションスクリプトが重症化し、あらゆる責務が乱雑に絡み合っているクラス。不正検知ロジックが書きなぐられているため、不正が発生したことを追跡するのが困難。
  • 単一責任の原則を遵守するクラスは100〜多くて200行に収まる。
きたやんきたやん

9章 さまざまなアンチパターン

  • どんな条件でも実行されないコード(デッドコード、到達不能コード)
    • 可読性が低下する
    • 意図があるのか疑い、混乱する
    • 将来的に到達できるようになり、バグを及ぼす可能性がある

YAGNI

You aren’t going to need it.

  • 実際に必要になった時にのみ実装する方針
  • 使われなくなったロジックはデッドコードになる
  • 先回りで作られたロジックは仕様にない→複雑

マジックナンバー

ロジック内に直接書き込まれている意味不明な数値
実装者本人にしか意図を理解できない
複数の箇所でマジックナンバーは実装されがち→重複コード

文字列型執着

単一の文字列変数に複数の値を格納すること
string $book = "Title,1997,08,19,Steve Jobs";
わかりにくい
別々の変数に格納する

グローバル変数

どこからでも参照・操作可能な変数
多くのロジックで参照・変更される→値がどこで、どのタイミングで書き変わったか把握するのが困難
排他制御が必要になる場合もある(同時に操作される可能性が生じるため)→ロック、パフォーマンス低下、デッドロック
巨大データクラスもさまざまなデータを保有→多くの箇所から参照される→グローバル変数と同様の性質

null問題

nullが入り込む前提でロジックを組みたくない
至る所でnullチェックするのは手間
nullチェックが漏れるとバグになる
null を返さない、渡さない
「未設定状態」も状態の1つとして扱う
null安全にする
null 非許容型

例外の握り潰し

例外を捕捉しても何もしないこと
エラーが起こっても、外から検知できない
このような不正は、しばらく経ってからサービスの利用者によって発見・報告される
開発者はデータベースやログ、コードを追いかけることになり、疲労する
問題検出時にはすぐに気づけるようにする
ロギング、リカバリ

メタプログラミング

プログラム実行時に、そのプログラムの構造自体を制御する技術
クラス構造を読み書きできる機構(=リフレクション)
ハードコード
メタ情報(パッケージ名・クラス名)からインスタンスを生成するなど
パッケージ・クラスが文字列として扱われている=ハードコード
静的解析の効果がなくなる

技術駆動パッケージング

パッケージの区切り方、フォルダの分け方
どれがどれに強く関連し合うかわからない

サンプルコードのコピペ

鵜呑みにするな

銀の弾丸

闇雲に自分が知っている便利な手法を使うと、逆に問題が深刻化するケースがある
ソフトウェア開発に銀の弾丸はない

きたやんきたやん

11章 コメント

退化コメント

コードに比べ、コメントはメンテナンスされにくい
コメントが偽情報として振る舞うようになる→退化コメント
ロジックの挙動をそのままなぞると、更新頻度は高いのに情報量は少ない→退化しやすい

コメントで命名を誤魔化す

コメントで補足説明しているから、適当な命名でいいや。→ダメです
補足説明ではなく、メソッド名や変数名自体をブラッシュアップする

意図、仕様変更時の注意点を伝える

コードを読む時、書き換える時の自分の気持ちを考えればわかる
どう言う意図でこのコードは書かれているのか
どう変更したら安全性を保てるか

ドキュメントコードを使う

当然
phpdoc
言語によっては、ドキュメント化してくれるツールもある

きたやんきたやん

12章 メソッド

自身のクラスのインスタンス変数を使う

他のクラスのインスタンス変数(プロパティ)を変更するメソッドはダメ。
低凝集に陥る

class ActorManager
{
  // ゲームキャラの位置を移動する。
  shift(Location $location,int $shiftX,int $shiftY): void
  {
    $location->x += $shiftX;
    $location->y += $shiftY;
  }
}
// これは Location クラスのインスタンス変数を変更している。
// Location クラスで location->x を変更するメソッドを定義するべき

不変をベースに、予期せぬ動作を防ぐ

プロパティが別の場所で更新されるなどによって、予期しない動作になることがある
プロパティを不変にする。

尋ねるな、命じろ

あるクラスが別のクラスの状態を判断したり、状態に応じて他のクラスの値を変更したりするのはダメ(低凝集)
getter, setterはあまり良くない

コマンドクエリ分離

状態の変更と取得を、同時に行わない。
CQS
コマンド(変更)またはクエリ(問い合わせ)のどちらかを実行する(CQRSとは別)

引数

引数は不変にする。入力値を変更すると意味が推測しづらい
フラグ引数は使わない
ストラテジパターンなどを使う
nullを返さない
Exception, nullチェックに繋がる
出力引数は使わない
入力値として使う。
引数は可能な限り少なく
引数が多い=処理内容が多い

戻り値

型を使う
エラーは例外として返す。
値の意味は一つ
staticメソッドは、自身のクラスのインスタンス変数を操作できない。→低凝集にならないようにする。