🔖

【コードレビュー】可読性向上：ロジックの単純化テクニック

2023/10/26に公開

はじめに

前回からのステップアップです。

前回では、表面上の改善としてロジック部分の変更やリファクタリングをしなくても「読みやすいコード」へ改善可能な項目についてまとめました。

今回は、実際のロジック部分に注目し、「理解しやすいコード」についてまとめます。

経緯としては、コードレビューにおいて感覚的に指摘するわけにもいきません。
言語化された指摘を行うために「リーダブルコード」を読み直しました。

本書は四部構成となっています。

表面上の改善
ループとロジックの単純化 ← 今回紹介
コードの再構成
抜粋テーマ

今回は「第二部：ループとロジックの単純化」で紹介されているものから抜粋しました。

ループとロジックの単純化

「ループとロジックの単純化」は理解しやすいコードの第一歩です。

複雑なループ
巨大な式
膨大な変数

以上の三点を意識することで、可読性が改善され、「理解しやすい」コードになります。
また、単純化されたコードは、変更が容易になり、バグも見つけやすくなります。

「第二部：ループとロジックの単純化」から以下を抜粋します

第七章：制御フローを読みやすくする
第八章：巨大な指揮を分割する
第九章：変数と読みやすさ

1. 制御フローを読みやすくする

1.1 条件式の引数の並び順

左側に「調査対象」、右側に「比較対象」

// Good
if age < 20 {
    ............
}

// Bad
if 20 > age {
    ............
}

左側	右側
「調査対象」の式。変化する。	「比較対象」の式。あまり変化しない。

「もし、年齢が20歳未満なら...」といったように自然な文章となる。
Badケースでいうと「もし、20未満の年齢なら...」と少し違和感を覚えるだろう。
「もし調査対象が比較対象なら...」となるような引数の並びを意識しよう。

1.2 if/elseの並び順

if/else文のブロックは、並び順を自由に変えることができる。
以下のようなケースは同じことを示す。

if a == b {
    // ケース１
} else {
    // ケース２
}

if a != b {
    // ケース２
} else {
    // ケース１
}

日常では深く考えることも少なくないが、コードを書く上で無意識的に書き分けている。
しかし、この並び順には優劣があり、以下の3点を意識する。

条件は否定形よりも肯定形を使う
単純な条件を先に書く
関心を引く条件や目立つ条件を先に書く

1.3 関数から早く返す

関数から早く結果を返すのはいいことです。むしろ望ましいこともある。
早く返すということは関数で複数のreturn文を使用する。
一方、関数でreturn文を複数使ってはいけないと思っている人も少なくない。

func contains(str, substr string) bool {
    if str == "" || substr == "" {
        return false
    }
  // 処理
  return true
}

複数のreturn文を採用することにはいくつかのメリットがあります。

シンプルな制御フロー：条件ごとに明確な戻り値を返すことができ、可読性が向上する
早期リターン：条件満たした場合に、早期リターンすることで不要な計算や処理を避ける
効果的エラーハンドリング：エラーが発生した場合にエラーを返し、正常な場合には処理結果を返す

1.4 ネストを浅くする

ネストの深いコードは理解しにくい。
ネストが深くなると、読み手は「精神的スタック」に条件をプッシュしなければいかない。
閉じ括弧}を見てからポップしようにも、その条件が何だったのかうまく思い出せない。

if文を複数用いるとネストが深くなり、理解が難しくなる。
工夫次第でネストを浅くすることができる。

早めに返してネストを削除する
ネストを少なくするために、「異常ケース」の場合は関数から早めに結果を返す。
ループ内部のネストを削除する
ループ内部でネストする場合は、以降の処理を行わない「continue」を使用する。
ただし、複数ループの場合「continue」がわかりにくくなることも多い。

ネストが増える仕組みとしては、既存ソースに対して新しく仕様を追加した際に多く起こる。
コードを追加する場合には、より簡潔な変更を行おうとする。
その結果、条件追加が追加されネストが深くなってしまう。

2. 巨大なを式分割する

人間が覚えれる情報のかたまり（チャンク数）は7±2と言われています。→ミラーの法則
例えばチャンク数をコードに置き変えて考えると、コードの式が大きくなればなるほど、それだけ理解が難しくなる。

2.1 説明変数を利用する

式を簡単に分割するには、式を表す変数を使えばよい。
この変数のことを「説明変数」と呼ぶ。

//説明変数不使用パターン
if strings.Split(line, ":")[0] == "root" {
        ...........
}

説明変数を利用すると以下のようになる。

//使用パターン
userName := strings.Split(line, ":")[0]
if  userName == "root" {
        ...........
}

あえて不明瞭な形にしているが不使用パターンの場合は、少し立ち止まり考える必要が出てくる。
説明変数を利用すると、その後の条件が何をするのかが見えてくるようになる。

2.2 要約変数を利用する

式を説明する必要がない場合でも、式を変数に代入しておくと便利です。
大きなコードの塊を小さな名前に置き換えて、管理や把握を簡単にする変数のことを「要約変数」と呼ぶ。

// 要約変数不使用パターン
if request.userId == document.ownerId {
    // ユーザーは文書を編集できる
}
if request.userId != document.ownerId {
    // 文章は読み取り専用
}

条件式で随時変数を意識する必要があり、少し立ち止まり考えてしまう。

このコードで伝えたいことは「ユーザーが文書を所持しているか？」なので要約変数を追加する。
そうすることでこの概念をより明確に表現できる。

// 使用パターン
var userOwnsDocument bool
userOwnsDocument = (request.userId == document.ownerId)

if userOwnsDocument {
    // ユーザーは文書を編集できる
}
if !userOwnsDocument {
    // 文章は読み取り専用
}

最初にuserOwnsDocumentを定義したことで、「このメソッドで参照する概念」を事前に伝えることができるようになった。

2.3 巨大な文を分割する

説明変数と要約変数の使用は、DRY原則を実践するのに役立ちます。

// Bad
if getUserAge("hogehoge") < 18 {
    fmt.Println("Child")
} else if getUserAge("hogehoge") >= 18 && getUserAge("hogehoge") <= 60 {
    fmt.Println("Adult")
} else {
    fmt.Println("Senior")
}

コードとしてはそれほど大きくないが、一箇所に集まると巨大な文になり可読性が落ちる。
このように同じ式がある場合、使用するデータを最上部に抽出することで可読性が向上する。

// Good
userAge := getUserAge("hogehoge")
priceCategory := ""

if userAge < 18 {
    priceCategory = "Child"
} else if userAge >= 18 && userAge <= 60 {
    priceCategory = "Adult"
} else {
    priceCategory = "Senior"
}
fmt.Println(priceCategory)

このように説明変数と要約変数を使用することで以下のような利点を得ることができる。

繰り返しによるタイプミスを軽減
横幅が短くなり、可読性が向上
変数名の変更などが容易になる

3. 変数と読みやすさ

変数を適当に使うとコードが理解しにくくなり、問題点が出てくる。

変数が多いと変数の追跡が難しくなる
変数のスコープが大きいとスコープを把握する時間が長くなる

これらの問題への対処を記述していく。

3.1 不要変数を削除する

前項では、「説明変数」や「要約変数」を使うことでコードを読みやすくした。
本項では、コードが読みにくくなる変数を削除する。
余計な変数を削除することで、コードは簡潔で理解しやすくなります。

【役に立たない一時変数の削除】

// メッセージの時刻更新処理
currentTime := time.Now()
message.lastViewTime = currentTime

このcurrentTimeは以下のような理由で意味がないと判断できる。

複雑な指揮を分割していない
time.Now()で十分な明確性がある
一度しか使っていないため、重複コードの削除になっていない

よくあるケースとして、実装当初は何度もcurrentTimeを使おうと思っていたが、結局使わなかったなどがあげられる。

【制御フロー変数を削除する】
ループ中の処理で以下のようなコードを見かけたことはないだろうか？

done := false // 制御フロー変数

for /*ループ条件*/ && !done {
	...........
	if /*条件*/ {
		done = true
		continue
	}
}

この変数doneはループ中にtrueに設定されている。
これはプログラムを実行を制御するためだけの変数であり、実際のプログラムに関係あるデータは含まれていない。
以下のようにすることで、制御フロー変数は解消できる

for /*ループ条件*/  {
	...........
	if /*条件*/ {
        break
	}
}

これなら簡単に解消ができる。
しかし、breakが使えないほどネストされたループも場合もあるだろう。
そのように複雑な場合には、ループ内部のコードやループ全体を新しい関数に移動すると良い。

3.2 変数のスコープを縮める

「グローバル変数を避ける」
これは皆さんも意識して取り組んでいることと思います。
グローバル変数は、どこでどのように使われているかが追跡が難しいです。
命名によってはローカル変数と衝突する場合もあります。
ローカル変数を使っていると思ったら、グローバル変数だった（またその逆）などミスにつながる。
グローバル変数に限らず全ての変数の「スコープを縮める」ことがよりよい。

// 不必要なグローバル変数宣言

var str_ string

func Method1(){
    str_ = "hogehoge"
    Method2()
}

func Method2(){
    // str_ を用いる処理
    if str_  == "hogehgoe" {
        ..............
    }
}

// その他 str_ を用いていないメソッド
............

str_は、Method1とMethod2で利用されている。
仮にメソッドが10個ある場合、読み手はその10個のメソッド全てに目を通す必要が出てくる。（ないことの確認）

今回のケースはstr_をローカル変数に格下げするとよい。

// ローカル変数に格下げ
func Method1(){
    var str_ string
    str_ = "hogehoge"
    Method2(str_)
}

func Method2(str_ string){
    if str_  == "hogehgoe" {
        ..............
    }
}

// その他 str_ を用いていないメソッド
............

もう一例として、Method1が莫大なメソッドとすると、メソッドのローカル変数といえどスコープが大きくなってしまう。
その場合、そのメソッドを複数に分割することでスコープを小さく保つことができる。

スコープに関することは言語により決め方が異なる。
ここでGo言語でのエラーおけるスコープを縮める手段を紹介する。

// err変数はエラーハンドリングブロック外でも有効
err := Method()
if err != nil {
    ........エラー処理
}
// err は見えている

以下のスタイルはエラーハンドリングをコンパクトに保ち、err変数がブロック内でのみ有効であることを示します。

// err変数がブロック内でのみ有効
if err := Method(); err != nil {
    ........エラー処理
}
// err は見えない

まとめ

プログラムの変数はすぐに増え、いづれ追跡ができなくなる。
そのために変数を減らし、なるべく「軽量」にすることで*+理解しやすいコード**となる。

邪魔な変数を削除する
変数スコープをなるべく小さくする

以上のテクニックを意識することで、理解しやすく、メンテナンスがしやすいコードを書くことができます。
可読性はコード品質の向上に貢献し、協力開発や保守性の向上に大いに役立つことでしょう。

前回の記事