Skip to content

Instantly share code, notes, and snippets.

@yokotaso
Last active September 27, 2018 23:44
Show Gist options
  • Save yokotaso/adb24cafcc22282fcdb965cc9b7079c2 to your computer and use it in GitHub Desktop.
Save yokotaso/adb24cafcc22282fcdb965cc9b7079c2 to your computer and use it in GitHub Desktop.
リーダブルコード 7,8,9章

7章 制御フローを読みやすくする

条件やループなどの制御フローはできるだけ「自然」にする。コードの読み手が立ち止まったり読み返したりしないように書く

条件式の引数の並び順

どっちが読みやすい?

if(length >= 10) ...
if(10 <= length) ...
while(bytes_received < bytes_expected) ...
while(bytes_expected > bytes_received) ...

比較式の左側が変化する。右側は変化しない とするとわかりやすい

if(length >= 10) ...
while(bytes_received < bytes_expected) ...

if/else ブロックの並び順

// 次のif文は同じだが、読みやすさは?
if(a == b) {

} else {

}

if(a != b) {

} else {

}

指針はつぎのようにすると複雑になりにくい

  • 否定形よりは肯定形を使う if(debug) がわかりやすい
  • 単純な条件を先に書くようにする
  • 関心を引く条件や目立つ条件を先に書く

最終的にはわかりやすさを基準に自分で判断する

if(!url.hasParameter("expand_all")) {

} else {

}

if(url.hasParameter("expand_all")) {

} else {

}

最初のif文に関心を引く条件や目立つ条件を先に書くのと肯定形を先に書くという指針したがって 後者のif文の方がわかりやすそう。

逆に否定形を強調死体ケースは前者を採用すると良い

三項演算子

十分に短く書くことができて、理解しやすい場合に限り三項演算子を使うとよい

// NG
return exponent >= 0 ? mantissa * ( 1 << exponent) : mantissa / (1 << -exponent);

do/whileループを避ける

function listHasNode(node, name, maxLength) {
    do {
        if(node.name() === name ) {
            return true;
        }
        node = node.next();
    } while(node != null && --maxLength >0);
}

やるなといっても次のような書き方を絶対にしてはいけない

// 疑似 do/whileは絶対にやらない

本体(1度目)

while(condition) {
    本体(2度目)
}

do/whileループはwhileループで書き直す事ができることが多い

function listNode(node, name, maxLength) {
    while(node != null && maxLength-- > 0) {
        if(node.name() == name) {
            return true;
        }
        node = node.next();
    }
}

関数から早く返す

メソッドの中では、returnを何度も使っていい(知っていた?)

function contins(String str, String substr) {
    if(str === null || substr === null) {
        return false;
    }

    if(substr === '') {
        return false;
    }
    ...
}

このような書き方は広く使われているもので ガード節 という。ガンガン使おう。

リソースのクリーンアップ

JavaScriptにはリソースのクリーンアップの機能がないので、省略。

PythonとJavaのコード例だけを示す。pythonやjavaを書くことがあると便利な機能なのでサンプルコードだけ。

# Withの中身が処理されるときに、openの返り値のfはf.close()が必ず呼ばれる
with open('sample.txt') as f
    ....
// try ... の処理が終わるときに br.close()が必ず呼ばれる
try (BufferedReader br = new BufferedReader(new FileReader(path))) {
    return br.readLine();
}

gotoを使わない

JavaScriptではgotoがないので省略。(あなたの今後の人生のために)絶対に使ってはいけないとだけ覚えておこう

ネストを深くする

ネストの深いコードは理解しにくい

if(userResult === SUCCESS) {
    if(permissionList !== SUCCESS) {
        reply.writeErrors("error reading permissions");
        reply.done();
        return;
    }
} else {
    reply.writeErrors(userResult);
}
  • 自分がどこを読んでいるかを忘れていまう
  • たくさんのifやforの条件を頭の中にためておく必要がある

ネストが増える仕組み

なぜ、ネストがどんどん増えていくのか?最小は単純なプログラムでも、機能追加やバグ修正を繰り返すとプログラムは簡単に複雑になっていく。

変更をするときはコードを新鮮な目で見直す。全体を見直して、リーダブルなコードに再構成する

はやめにreturnしてネストを削除する

if(userResult !== SUCCESS) {
    reply.writeErrors(userResult);
    reply.done();
    return;
}

if(remissionList !== SUCCESS) {
    reply.writeErrors(permissionResult);
    reply.done();
    return;
}

reply.writeErrors("");
reply.done();

ネストが浅くなって読みやすくなった!if文のネストがなくなったので、頭の中に複数のif文の条件を覚えておく必要がなくなった!

ループ内部のネストを削除する

for(var i = 0; i < results.length; i++) {
    if(results[i] !== NULL) {
        nonNullCount++;

        if(result[i].name !== "") {
            console.log("Considering candidate ...");
            ...
        }
    }
}

NULLチェックのif文をガード節に書き換えてみよう

for(var i = 0; i < results.length; i++) {
    if(results[i] === NULL) {
        continue;
    }
    nonNullCount++;

    if(result[i].name === "") {
        continue;
    }
    console.log("Considering candidate ...");
    ...
}

ガード節でcontinueを使うように修正した。ただしcontinueを使うとわかりにくくなることも多いので注意が必要。

Javascriptだと次用な書き方ができる。ただし乱用しすぎるとこれもわかりにくくなるので注意が必要

var nonNullResult = result.filter((elm) => elm != NULL);
var nonNullCount = nonNullResult.length;
nonNullResult
  .filter((elm) => elm.name !== ""))
  .forEach((elm) => {
      console.log("Considering candidate ...");
      ...
  });

処理の流れや実行タイミングがわかりにくいもの

JavaScriptではそういった機能を利用する機会が少ないと思うので簡単に流れがわかりくくなるものを紹介

  • スレッド
  • シグナル・割り込みハンドラ
  • 例外
  • 関数ポインタと無名関数
  • 仮想メソッド

ちなみにJavaScriptだと async,await や Promise, サービスワーカといった機能があります。

まとめ

コードの処理フローは工夫次第でわかりやすく書くことができる

  • ifやwhileの条件文の書き方(固定値を式の右に)
  • if/elseは肯定形をifの条件分に持っていく
  • if/elseのうち、強調したいことをifの中に書く
  • 三項演算子、do/whilehなどはコードが理解しにくくなるので注意
  • ネストは浅く保つ。浅く保つ。
  • guard節をうまく活用して、ネストを浅く保つ

8章 巨大な式を分割する

巨大な四季は飲み込みやすい大きさに分割する

説明用変数

1行にいろいろな処理が込み入っていてわかりにくくなるときがある。そんなときは説明用の変数を取り入れる

// わかりにくい!
if(line.split(" ")[0].split() === "root") 

// すこしわかりやすい!
var username = line.split(" ")[0].split()
if(username === "root")

要約変数

コメントに要約を書いているときがあるが、それを変数名に割り当てるとよりわかりやすくなる

// 所収者なので編集が可能
if(request.user.id === document.ownerId) {

}

// 所有者ではないのでReadOnly
if(request.user.id !== docunent.ownerId) {

}
// 所収者なので編集が可能
var isDocumentOwner = request.user.id === document.ownerId;
if(isDocumentOwner) {

}

// 所有者ではないのでReadOnly
if(!isDocumentOwner) {

}

ド・モルガンの法則を使う

高校のときに習ったド・モルガンの法則を使って処理がわかりやすくなる時がある

// 2つの文は意味は同じだが、わかりやすさが違う
if(!(fileExists && !isProtected)) { ... }
if(!fleExists || isProtected) { ... }

短絡評価を悪用しない

if(a || b) と書いたときに aがtrueならbは評価されない。これを悪用して読みにくいコードを作れる

assert((!(bukect = findBucket(key)) || !bucket.isOccupied()));

次のように書き直す

var bucket = findBucket(key);
if(bucket != null) {
    assert(!bucket.isOccupied());
}

ただし、便利に使うこともできる.それはnullの場合は計算を辞めたいケース

if(object && object.method()) { ... }

また複数のnullになりうる値のうち、最初にnullではない値を取得したければ

var a = null;
var b = "OK";
var c = "NG";
var d = a | b | c; // d = "OK"

複雑なロジックを工夫して簡単にする

begin,endプロパティを持つオブジェクトを比較して、2つのオブジェクトが重なりがあるかを判定するメソッドを考えよう

range.png

最終的に出来上がった式がこちら

function overrapWith(a, b)
return (a.begin >= b.begin && a.begin < b.end) ||
       (a.end > b.begin && a.end <= b.end) ||
       (a.begin <= b.begin && a.end >= b.end);

より優雅な手法を見つける

条件式の逆を考えてみると簡単になることがある

  • 配列を逆順にたどってみる
  • データを後ろから入れてみる

今回は条件の逆を考えてみる。

  • 一方の終点が、ある範囲の支点よりも前にある場合
  • 一方の支点が、ある範囲の終点よりも後にある場合
function overrapWith(a, b) {
    // 一方の終点が、もう一方の始点よりも前にある
    if(b.end <= a.begin) {
        return false
    }
    // 一方の始点が、もう一方の終点よりもあとにある
    if(b.begin >= a.end) {
        return false;
    }
    // 残ったものは重なっている
    return true;
}

巨大な四季を分割する

var update_highlight = function(messeage_num) {
    if($("#vote_value" + message_num).html() === "Up") {
        $("#thumbs_up" + message_num).addClass("hightlited");
        $("#thumbs_down" + message_num).removeClass("hightlighted");
    } else if($("#vote_value" + message_num).html() === "Down") {
        $("#thumbs_up" + message_num).removeClass("hightlited");
        $("#thumbs_down" + message_num).addClass("hightlighted");
    } else {
        $("#thumbs_up" + message_num).removeClass("hightlited");
        $("#thumbs_down" + message_num).removeClass("hightlighted");
    }
}

同じ式を要約変数としてつくるとよい

var update_highlight = function(messeage_num) {
    var thumbs_up = $("#thumbs_up" + message_num);
    var thumbs_down = $("#thumbs_down" + message_num);
    var vote_value = $("#vote_value" + message_num).html();
    var hi = "highlighted";
    if(vote_value === "Up") {
        thumbs_up.addClass(hi);
        thumbs_down.removeClass(hi);
    } else if(vote_value === "Down") {
        thumb_up.removeClass(hi);
        thumbs_down.addClass(hi);
    } else {
        thumbs_up.removeClass(hi);
        thumbs_down.removeClass(hi);
    }
}

まとめ

説明用の変数を導入することで得られるメリット

  • 巨大な式を分割できる
  • 簡潔な名前で式を説明することで、コードを文章化できやすくする
  • コードの主要な「概念」を読み手が意識しやすくする

条件式が簡単にならないかを考える

  • ド・モルガンの法則
  • Guard節を活用する
  • 条件の逆をとると簡単にならないかを考える

9章 変数と読みやすさ

  1. 変数が多いと変数を追跡するのが難しくなる
  2. 変数のスコープが大きいと把握するのに時間がかかる
  3. 変数が頻繁に更新されると現在の値を把握するのが難しくなる

変数を削除する

役に立たない変数は削除する

now = datetime.datetime.now()
root_message.last_view_time = now

nowにはどういう意味があるだろうか?

  • 複雑な式を分割していない
  • より明確になっていない datetime.datetime.now() でも十分に伝わる
  • 一度しか使っていないので、重複コードの削除になっていない
root_message.last_view_time = datetime.datetime.now()

中間結果を削除する

var remove_one = function(array, value_to_remove) {
    var index_to_remove = null;
    for(var i = 0;i < array.length;i++) {
        if(array[i] === value_to_remove) {
            index_to_remove = i;
            break;
        }
    }

    if(index_to_remove !== null) {
        array.splice(index_to_remove, 1);
    }
}

この例であれば、index_to_removeは中間結果を保持しているだけなので、見つかった時点で削除とreturnをしてしまうときれいに書ける

var remove_one = function(array, value_to_remove) {
    var index_to_remove = null;
    for(var i = 0;i < array.length;i++) {
        if(array[i] === value_to_remove) {
            array.splice(i, 1);
            return;
        }
    }
}

タスクはできるだけ速めにすませてしまうように意識しよう

制御フロー変数を削除する

var done = false;
while(... && !done) {
    if(...) {
        done = true;
        continue;
    }
}

done のような変数を制御フロー変数とよんでいる。うまくプログラミングすれば制御フロー変数は削除できる。

while(...) {
    if(...) {
        break;
    }
}

ネストが深くなると、どうしても制御フロー変数をつかいたくなるので、関数に切り出してネストが深くならないようにしよう

変数のスコープを縮める

変数のスコープが長いとなぜ辛いのか?

  • 変数が衝突する可能性がある
  • 変数の書き換えミスが起きやすい
  • 一緒に考えないといけない変数の数が増えやすい

変数のスコープが短くなるのはいいことだ

  • 関数を短く区切る
  • Classのメンバ変数をローカル変数できないか検討してみる
  • クラスを分割して、不要な変数を考慮しないでいいようにする

JavaScriptのスコープをへらす

submitted = true; // グローバル変数

var submit_form = function(form_name) {
    if(submmited) {
        return; //二重投稿禁止
    }

    submitted = true;
}

submittedはグローバル変数になっていて、変数が衝突したり、考慮することが増えたりなどいいことがない。

var submit_form = (function() {
    var submitted = false;

    return function(form_name) {
        if(submitted) {
            return; //二重投稿禁止
        }

        submitted = true;
    }
})();

(function() {...})() は即時関数を意味している。実行された結果、関数が帰ってくるので、上のプログラムと同じだが、submittedのスコープが制限される。

JavaScriptのグローバルスコープ

var をつけ忘れるとグローバル変数になってしまう。

<script>
var f = function() {
    // iはグローバル変数
    for(i = 0; i < 10; i++) { ... }
}
</script>

<script>
alert(i); // 10が表示される
</script>

グローバル変数は罪深い. JavaScriptのリントツール(eslint) という良いツールがあるので、こういうのを活用して変なミスが入らないようにするのがよい

ネストとスコープ

javaやC++などでは、宣言されたスコープの外側では変数に触ることはできない

if(...) {
    boolean isOK = false;
}
isOK = true; // コンパイルエラー!

だが、PythonやJavaScriptではこれは動いてしまう

if request:
  for value in request.values:
    if value > 0
      example_value = value
      break

for logger in debug.loggers:
  logger.log("Example:", example_value)

簡単な例なので気にならないかもしれないが、式が大きくなるとかなり読みにくくなる。 改善するには、 中間結果を削除する, タスクをできるだけ早く完了する

function logExample(value) {
    for(var i = 0; i < loggers.length; i++) {
        loggers[i].log("Example:", value)
    }
}

if(request) {
    for(var i = 0; i < request.values.length; i++) {
        if(request.values[i]) {
            logExample(request.value[i]) // すぐにvalueを使う
            break;
        }
    }
}

必要になる直前で変数を宣言する

function viefwFilteredReplies(originalId) {
    filteredReplies = [];
    rootMessage  = Messages.objects.get(originalId);
    allReplies = Messages.objects.select({rootId : originalId});

    rootMessage.viewCount += 1;
    rootMessage.lastViewTime = Date().getTime();
    rootMessage.save();

    for(var i = 0; i < allReplies.length; i++) {
        if(allReplies[i].spamVotes <= MAX_SPAM_VOTES) {
            filteredReplies.append(reply);
        }
    }

    return filteredReplies;
}

変数は使う直前で宣言すれば良い

function viefwFilteredReplies(originalId) {

    var rootMessage  = Messages.objects.get(originalId);
    rootMessage.viewCount += 1;
    rootMessage.lastViewTime = Date().getTime();
    rootMessage.save();

    var allReplies = Messages.objects.select({rootId : originalId});
    var filteredReplies = [];
    for(var i = 0; i < allReplies.length; i++) {
        if(allReplies[i].spamVotes <= MAX_SPAM_VOTES) {
            filteredReplies.append(reply);
        }
    }

    return filteredReplies;
}

変数は一度だけ書き込む

変数はスコープが大きいと理解しにくいとわかってもらえたと思う。 さらに話をすすめて、変数に書き込むのは一度だけにするのがおすすめ。

変数を操作する場所が多いと現在地の判断が難しくなる

最後の例

var setFirstEmptyInput = function(newValue) {
    var found = false;
    var i = 1;
    var elm = document.getElementById("input" + i);

    while(elm !== null) {
        if(elm.value === '') {
            found = true;
            break;
        }
    }
    i++;
    elm = document.getElementById("input" + i);
    if(found) {
        elm.value = newValue;
    }
    return elm;
}

これを改善してみよう.ポイントは次の3つ

  • var found は見つかった時点でリターンすれば消せる
  • var i はwhileからforに書き換えればスコープを減らせる
  • var elmはforの中でのみ書き換えるようにする
var setFirstEmptyInput = function(newValue) {
    for(var i = 1;true; i++) {
        var elm = document.getElementById("input" + i);
        if(elm === null) {
            return null;
        }

        if(elm.value === '') {
            elm.value = newValue;
            return elm;
        }
    }
}

まとめ

  • 邪魔な変数を削除する
  • 変数のスコープをできるだけ短くする
  • 変数に書き込むのは一度だけにする
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment