リーダブルコード読書会を終えて 自分にできてなかったことを振り返る

※読んでみて私が実践できていなかったことをまとめてみます。

リーダブルコードの読み方を考察

• 最初に末尾の解説から読むと、目的がわかりやる気が上がる
• 基本的なルールを覚えることが大事
  • 取捨選択は必要であるが、基本ルールをもとに、自分の中で明確なルールを説明し、不要と思うものは採用しない。
• SIではプロジェクトにより、既にルールが確立されている場合が多くあるが、ビジネスとして割り切って、そこはルールに従う。

読みやすさの定理

• コードは他の人が最短時間で理解できるように書かなければいけない
  • 本書の全てにおいて、上記が優先される。まず可読性を最優先し、次に性能や保守性などを考慮して、どちらを取るか考える

自分が実践できていなかったもののまとめ

名前に情報を詰め込む

• 汎用的な名前を避ける
  • 「return ret」などを止め、意味のある名前で返却
• ループイテレーター
  • user[] menber[]の2重ループi, jを利用せず、ui, miなどにすると不具合が減る
• 値の単位
  • Start(int delay) → delay_secなど具体的な単位を付ける
• スコープが小さければ短い名前で良い

C#.cs

var dt = new DataTable("tName");　// OK
Hoge(dt);

public void Hoge (DataTable dataTable) { // publicメソッドの引数はdtよりdataTableの方が良いか

美しさ

• 本書にある下記のようなコード整形はフォーマッタに依存する部分があるため、現実的には実践しないことが多いと思われる。

CheckFullName("Goug Adams", "Mr.Douglas Adams");
CheckFullName("Jhon"      , "Jhon Brown");

巨大な式を分割する

• 説明変数
  • if条件などに利用する長い処理を変数に一度格納する。
  • 繋げて書いた方がカッコイイ感でまとめて書いていることが多かった。。

youyaku.java

// これはイメージなので不細工なコードになっています。
if (fuga(hoge.calc("str").substring(0, 1).equals("s"))) {...}

// 一度変数に格納する
final String setumei = fuga(hoge.calc("str").substring(0, 1));
if (setumei.equals("s")) {...}

• 要約変数
  • それほど大きな式じゃない場合でも、仕様の意図を明確にするため、要約変数に格納することも考慮する
  • 以下の例は5つの変数(概念？)が入っており、ぱっと見わかりずらいため、意味がわかる変数名に格納する。

youyaku.java

if(request.user.id == document.owner.id) {
  // ユーザーはこの文書を編集できる
}
...
if(request.user.id != document.owner.id) {
  // 文書は読み取り専用
}
// 以下のように修正
final boolean user_owns_document = (request.user.id == document.owner_id);

if (user_owns_document) {
}
...
if (!user_owns_document) {
}

• ド・モルガンの法則
  • 完全に存在を忘れていました。「!(複数の条件)」のような状態を結構やってきた気がします。ド・モルガンの法則で読みやすく。
  • not(a or b or c) ⇔ (not a) and (not b) and (not c)
  • not(a and b and c ⇔ (not a) or (not b) or (not c)
  • notを分配して、and/orを反転、(逆方向)notをくくりだす

sample.java

if(!(file_exists && !is_protected)) {...}
// ↓修正後
if(!file_exisits || is_protected) {...}

• とても複雑なロジックになってきたときは、一度戻って発想の転換をする。反対のことを考えるなど

コメントすべきことを知る

• 第3者はもちろんだが、自分自身が数年後に読んでわかるようにすること
• どういう考えのもとこのコードになったのかを書いておく
• このコードになるまでに得た検証過程なども書いておく
• 既知の不具合ややむを得ない事情で良くないコードになっている場合、その旨を記載する
• メソッド利用時の注意事項なども書いておく
  • 外部I/Fを伴うもの
  • 変更不可メソッドやクラスなど(ミュータブル、イミュータブルの通知)
  • メソッドの場合は名前でイミュータブルであることを伝えるケースも良いかも

コメントは正確で簡素に

• 入出力が複雑な場合は実例を利用する

comment.java

// 実例：("abba/a/ba", "ab")は"/a/"を返す
public String Strip(String src, String chars) {...}

無関係な下位問題を抽出する

• 1つのメソッドで実現したい内容の中に、別の概念の複雑な処理が発生することがある。本来実現したいメソッドの意図に集中したいため、別概念の処理は下位問題として切り出す。

sample.java

public void Hoge() {
    // Hogeメソッドの関心事
    // Hogeメソッドの関心事ではない処理(なんらかの演算処理など)
    for (int i = 0; i < array.length; i++) {
        // なんらかの計算処理(ここでは短いコードとするが、もっと長いものを想像)
        final int random = Math.random();
        final int x = Fuge.getNumber();
        int y = random * y;
    }
    // Hogeメソッドの関心事
}

// 非関心事を外だしにする
private static final int calc() {
        final int random = Math.random();
        final int x = Fuge.getNumber();
        return random * y;
}

• privateメソッドに切り出すケースは大きく2種ある。共通処理を外に出しておく、下位問題を外に出しておく、この両者は似ているが目的が異なるので、意識しておきたい。

CommonSampleBefore.java

public void Hoge() {

        // Hogeの処理...
        // 重複コード
        final int random = Math.random();
        final int x = Fuge.getNumber();
        int y = random * y;
}
public void Fuga() {

        // Fugaの処理...
        // 重複コード
        final int random = Math.random();
        final int x = Fuge.getNumber();
        int y = random * y;
}

CommonSampleAfter.java

// 重複コードを共通メソッド化
public void Hoge() {

        // Hogeの処理...
        calc();
}
public void Fuga() {
       // Fugaの処理..
       calc();
}

private static final int calc() {
        final int random = Math.random();
        final int x = Fuge.getNumber();
        return random * y;
}

• 下位問題のprivateメソッドへの外だしは、変更影響を局所化する効果もあるが、やりすぎるとソースコードを見る際に、いくつものメソッドを飛び回る必要性が出てくるので、コードを追うのが大変になるというデメリットも持っている。読みやすさ > 仕様変更コストと考えた方が良く、外だしもほどほどに。

一度に１つのことを

• 1つのメソッドでは、1つの目的を持つようにする。説明例思い浮かばず、雰囲気をサンプルとしてみます。
  • MemberListというコレクションに対して、「複雑な処理A」、「複雑な処理B」があり、左記は相互干渉しないとする。この場合は処理を分割した方が読みやすい。

Sample.java

public void Hoge() {
   for (final Community c in CommunityList<Community>) {
        for (final Member m in MemberList<Member>) {
            // 複雑な処理Aの実装(10step程度)
            // 複雑な処理Bの実装(10step程度)
        }
    }
}
// 以下のように分割すると可読性はあがる。
public void Hoge() {
   for (final Community c in CommunityList<Community>) {
       String x = fugaA(c);
       String y = fugaB(c);
   }
}
private static final String fugaA(Member member) {
    for (final Member m in MemberList<Member>) {
        // 複雑な処理Aの実装(10step程度)
    }
}
private static final String fugaB(Member member) {
    for (final Member m in MemberList<Member>) {
        // 複雑な処理Bの実装(10step程度)
    }
}

※ただし、ループ処理を分割する場合は、前提条件として、起こりうるループ回数を把握しておおく必要がある。

短いコードを書く

• 本章のコラムにある「未使用コードを削除する」について重要性を再度確認したい。 当たり前のことではあるのですが、大規模プロジェクトで修正が頻発すると、使われていないコードが残ることがある。

そのプロジェクトの1機能に不具合があり、類似見直しで横展開することになった。同一機能を横展開で全て修正し、テストを依頼したが、呼び出し元がありません。という事態を招くことになってしまう。せっかく修正した作業が無駄になる。こういったリスクも踏まえて、不要な処理は残さないように気を付ける。

本書に具体例がないが気を付けたい点

• 重たい処理(DBアクセスなど)にはgetXXX()などの命名をしないようにする。

  • 利用者が特に意識せず単純利用するイメージがあるため。

• 公開メソッドのヘッダコメントに詳細な処理ロジックの説明は書かない

  • 内部処理の変更があった場合、公開されていると、仕様変更になってしまうため。

• 非公開メソッドのヘッダコメントはなくても良いかもしれない。

  • 内部処理の複雑な箇所があればメソッド内コメントで補填する。

• アート的な創作物ではないが、プログラムは作品である。美しくあるべきだという感覚を持つ

• 本書の変数スコープの話との関連で、クラスの状態(メンバ変数など)に変更があるか否かを読み手に意識させないようなコードを書いた方が良い。

• private なメソッドに分割した場合、フィールド変数に影響がないものであれば全てstaticにしておく

• sample.java

  public Class Hoge() {
       private String name;
  
       private static String edit(int x, int y) {
           return x + y;
       }
  }
  

• 似たような話で、voidのメソッド引数に参照型を渡す場合、引数の値が変更されるのか否かわかるようにしておきたい。参照目的の引数についてはfinalを付ける。

final.java

private void edit(final Hoge hoge, Fuga fuga) {
      // Fugaの状態は変化するが、Hogeの状態は変化しないのでfinalとする
      // ※C#などではfinal相当のものがなかったりもする。
      fuga.setMenber(hoge.getName());
}

• 変数の数だけバグが発生しやすいという点を考慮し、一時変数でも値の変化がないものはfinalを付けるようにしたい。本書の要約変数などにfinalが付いているが、意識するためここに書き記しておく。