のこのこかずのこ

10年エンジニアやってるけどいまだになんもわからん

技術書読んだメモ : 良いコード/悪いコードで学ぶ設計入門

こちらの本を読みました!

もう何年も仕事で超絶レガシーコードを触っていて嫌になることが多々あるので、そして自分もレガシーコードを書いている気がして不安になるので、気になって読みました。

JavaC#のような静的型付けオブジェクト指向言語のクラス設計の話が中心だったので、普段自分が触っているJavaScriptPHPにドンピシャな本ではなかったですが、それでも学ぶことが多かったです。サンプルコードは馴染みのないJavaでしたが、特定の言語に限る話ではないので理解できないことはなく、NG例とOK例がセットで載っているので何が良くて何がダメなのかすらわからない初心者にはとてもわかりやすかったです。普段の仕事ではクラスを書くこともあまりないのですが(ロジックベタ書き)(絶対ダメ)、自分がクラス設計とかオブジェクト指向について完全に誤認していた部分があることが分かって、あ、読んどいてよかった……となりました。言葉だけ知ってて概念をイマイチ理解できていなかったものたちもだいぶクリアになりました。

以下、特に自分が特に誤解してた〜とか知らなかった〜とかやらかしそう〜と思った箇所の備忘録。

不変(イミュータブル)をちゃんと設定する

❌ NGコード これは副作用があるからだめ!

class AttackPower {
  static final int MIN = 0;
  int value;

  AttackPower(int value) {
    if(value < MIN) {
      throw new IllegalArgumentException();
    }
    this.value = value;
  }
  
  void reinForce(int increment) {
    value += increment;
  }
  void disable() {
    value = MIN;
  }
}

主作用というのは、関数(メソッド)が引数を受け取り、値を返すこと。副作用とは主作用以外に状態変更すること。参照透過性(同じ条件(引数)を与えたとき、実行結果が常に等しくなる)を意識すると副作用に気付きやすそう。

インスタンス変数にfinal演算子を付与して不変にする。でもそれだと変更できなくなるので、変更時には変更値を持った新しいインスタンスを生成する。 さらに、引数がintやstringのプリミティブ型ではミスや不正状態を防げないので、型を指定すると良い。

⭕️ Goodコード

class AttackPower {
  static final int MIN = 0;
  final int value;
  // コンストラクタは省略
  AttackPower reinForce(final AttackPower increment) {
    return new AttackPower(this.value + increment.value);
  }
  void disable() {
    return new AttackPower(MIN);
  }
}

出力引数は危険

❌ NGコード

class ActorManager {
  // ゲームキャラの位置を移動する
  void shift(Location location, int shiftX, int shiftY) {
    location.x += shiftX;
    location.y += shiftY;
  }
}

引数のLocationが変更されることが外部からわからない、低凝集になりやすい! データとデータを操作するロジックは同じクラス内に凝集する。

⭕️ Goodコード

class Location {
  final int x;
  final int y;
  Location(final int x, final int y) {
    this.x = x;
    this.y = y;
  }
  Location shift(final int shiftX, final int shiftY) {
    final int nextX = x + shiftX;
    final int nextY = y + shiftY;
    return new Location(nextX, nextY);
  }
}

継承より委譲

継承はよっぽど注意して扱わないと危険!スーパークラス依存を避けるため、下手に継承するより委譲したほうがいい。 委譲とはコンポジション構造にすること。継承するのではなく、privateなインスタンス変数として持ち、呼び出す。

❌ 闇雲な継承

class FighterPhysicalAttack extends PhysicalAttack {
    @Override
    int singleAttackDamage() {
        return super.singleAttackDamage() + 20;
    }
    @Override
    int doubleAttackDamage() {
        return super.doubleAttackDamage() + 10;
    }
}

⭕️ 委譲

class FighterPhysicalAttack {
    private final PhysicalAttack physicalAttack;
    // コンストラクタ省略
    int singleAttackDamage() {
        return physicalAttack.singleAttackDamage() + 20;
    }
    int doubleAttackDamage() {
        return physicalAttack.doubleAttackDamage() + 10;
    }
}

その他もろもろ

  • クラス設計とは、インスタンス変数を不正状態に陥らせないための仕組みづくり。 ≒ コンストラクタにバリデーションを定義すること。
  • ifやswitchの分岐処理を描きそうになったら、まずinterfaceを使うことを検討する!
  • DRY原則において、DRYにすべきはそれぞれの概念単位。同じようなロジック、似ているロジックであっても、概念が違えばDRYにすべきではない。 ex. 通常割引価格と夏季割引価格はロジックは似ていても流用すべきでない。
  • getter/setter メソッドは開発生産性が良くないコードでよく見られる!よそのクラスを気にしたりいじったりするメソッド構造に陥りやすいので闇雲に実装すべきではない。
  • メソッドはコマンド(=状態の変更)とクエリ(=問い合わせ、状態を返す)のどちらか一方だけを行うよう設計する。
// コマンド
void gainPoint() {
    point += 10;
}
// クエリ
int getPoint() {
    return point;
}

以上のようなポイントを、普段使っているJavaScriptに応用しようと思うと、機能自体がなくて難しいものもある…。けど、調べた感じ、TypeScriptではかなりカバーできそう。(TypeScriptには不変にするためのreadonly プロパティがあったり、interfaceがあったり。) TypeScriptもちゃんと勉強したい!