Skip to content

Instantly share code, notes, and snippets.

@yokotaso
Last active October 1, 2018 09:52
Show Gist options
  • Save yokotaso/0e31a296330c9f8a263cc793d8703f53 to your computer and use it in GitHub Desktop.
Save yokotaso/0e31a296330c9f8a263cc793d8703f53 to your computer and use it in GitHub Desktop.
リーダブルコード 10-11章

10章 無関係の下位問題を抽出する

プログラムのコツは、大きな問題を小さな問題に解決すること

  1. 関数やブロックをみて「このコードの高レベルの目標はなにか?」を考える
  2. コードの各業に対してコードの目標を達成しているか、無関係の問題を解決していないか?を考える
  3. 無関係の問題を解決しているコードが相当量あれば、それらを抽出して別の関数にする

入門的な例

// 与えられた緯度経度に最も近い'array'の要素を返す
// 地球が完全な球体であることを前提としている
var findClosestLocation = function(lat, lng, array) {
    var closest;
    var closest_dist = Number.MAX_VALUE;
    for(var i = 0;i < array.length;i++) {
        var lat_rad = radians(lat);
        var lng_rad = radians(lng);
        var lat2_rad = radians(array[i].latitude);
        var lng2_rad = radians(array[i].lngitude);

        // 「球面三角法の第二余弦定理」を使う
        var dist = Math.acos(
            Math.sin(lat_rad) * Math.sin(lat2_rad) + 
            Math.cos(lat_rad) * Math.cos(lat2_rad) *
            Math.cos(lng2_rad - lng_rad)
        );

        if(dist < closest_dist) {
            closest = array[i];
            closest_dist = dist;
        }
    }
    return closest;
}

ループ内のコードで無関係の問題を扱っているのは、2地点の距離を計算している部分だろう。ここを関数に切り出そう

var spherical_distance = function(lat1, lng1, lat1, lng2) {
    var lat1_rad = radians(lat1);
    var lng1_rad = radians(lng1);
    var lat2_rad = radians(lat2);
    var lng2_rad = radians(lng2);
    // 「球面三角法の第二余弦定理」を使う
    return Math.acos(
        Math.sin(lat1_rad) * Math.sin(lat2_rad) +
        Math.cos(lat1_rad) * Math.cos(lat2_rad) *
        Math.cos(lng2_rad - lng1_rad);
    );
}

関数を抽出すると次のようになる。少し読みやすくなった。そしてspherical_distanceにはテストコードがかけるようになった。

// 与えられた緯度経度に最も近い'array'の要素を返す
// 地球が完全な球体であることを前提としている
var findClosestLocation = function(lat, lng, array) {
    var closest;
    var closest_dist = Number.MAX_VALUE;
    for(var i = 0;i < array.length;i++) {
        var lat_rad = radians(lat);
        var lng_rad = radians(lng);
        var lat2_rad = radians(array[i].latitude);
        var lng2_rad = radians(array[i].lngitude);

        // 「球面三角法の第二余弦定理」を使う
        var dist = spherical_distance(lat, lng, array[i].latitude, array[i].longtude);

        if(dist < closest_dist) {
            closest = array[i];
            closest_dist = dist;
        }
    }
    return closest;
}

純粋なユーティリティコード

  • 配列をなにかの文字でつなげる
  • 文字列の空白をトリムする
  • 文字列の中に変数を埋め込む

などプログラムを書いているとよく出てくるコードがある。

ライブラリがあるときはそちらを使えばよいが、そうでないときは、自分で作ってしまうと良い

その他の汎用コード

ajax_post({
    url: 'http://example.com',
    data: data,
    on_success: function(response_data) {
        var str = "{";
        for(var key in response_data) {
            str += " " + key + " = " + response_data[key] + "¥n";
        }
        alert(srt + "}");
        // 引き続き response_dataの処理
    }
})

on_successの中のfor文ではAjaxリクエストのレスポンスをログに出すことだ。 これは、デバック用と無関係なコードなので、メソッドに抽出しよう。

var format_pretty = function(obj) {
    var str = "{";
    for(var key in response_data) {
        str += " " + key + " = " + response_data[key] + "¥n";
    }
    return str + "}";
}

思いもよらない恩恵

プログラムで本筋ではない部分を抽出する理由はたくさんある

  • format_prettyが再利用可能になる
  • format_prettyの改善が気楽に行える.

このformat_prettyは次のようなケースには対応していないが、対応は気楽だ

  • objはオブジェクトを期待している。普通の文字列や(undefined)だと例外が発生する
  • objは単純な方を期待している。ネストしたオブジェクトはサポートしておらず、きれいにログ出力されない

format_prettyが独立した関数になっていければ、これらを改善するのは大変だが、関数になっていればこうした機能は簡単に追加できる。

var format_pretty = function(obj, indent) {
    if(obj === null) return "null";
    if(obj === undefined) return "undefined";
    if(typeof obj === "string") return '"' + obj + '"';
    if(typeof obj !== "object") return String(obj);
    
    if(indent === undefined) indent = "";
    
    var str = "{";
    for(var key in response_data) {
        str += indent + key + " = ";
        str += format_pretty(response_data[key], indent + indent) + "¥n";
    }
    return str + indent + "}";
    
}

汎用コードをたくさん作る

汎用的なコードはプロジェクトを隔ててutilディレクトリなどに入れておこう。 また、汎用的なコードはプロジェクトをまたいで使える。

Javascriptだとlodashというライブラリが汎用ライブラリとして有名。 基本的な汎用メソッドはlodashを使うようにして、プロジェクト特有ではない汎用コードを汎用化するとよい。

プロジェクトに特化した機能

business = Business()
businnes.name = request.POST["name"]

url_path_name = business.name.lower()
url_path_name = re.sub(r"['\.]", "", url_path_pattern)
url_path_name = re.sub(r"[^a-z0-9]+", "-", url_path_name)
business.url = "/biz/" + url_path_name

business.date_created = datetime.datetime.now()
business.save_to_database()

url_path_nameはnameはA.C.Joe's Tire & Smog,Inc., であれば、  /biz/ac-joes-tire-smog-inc に修正しているようだ。 こういった変換コードは、無関係の下位の問題なので抽出できる

CHARS_TO_REMOVE = re.compile(r"[\.]+")
CHARS_TO_DASH = re.compile(r"[^a-z0-9]+")

def make_url_friendly(text):
    text = text.lower()
    text = CHARS_TO_REMOVE.sub('', text)
    text = CHARS_TO_DASH.sub('-', text)
    return text.strip('-')

business = Business()
business.name = request["POST"]
business.url = "/biz/" + make_url_friendly(bussiness.name)
business.date_created = datetime.datetime.now()
business.save_to_database()

ファイルの場所はどこでもいいが大事なのはmake_url_friendlyを抽出して読みやすくなるということだ

既存のインターフェースを簡単にする

document.cookieの内容がname1=value1;name2=value2;... となっているとしてその中から最大値を取得するメソッドを考えてみる

var max_result;
var cookies = document.cookie.split(';');
for(var i = 0; i < cookies.length;i++) {
    var c = cookie[i];
    c = c.replace(/^[ ]+/, '')
    if(c.indexOf("max_results=") === 0) {
        max_results = Number(c.substring(12, c.length))
    }
}

読みにくいの書き換えよう。与えられた引数の値を取得するget_cookieを作ろう

var max_results = Number(get_cookie("max_results"))

cookieに値を設定するメソッドも考えてみる(document.cookie=...では実はcookieの内容を書き換えることはできず、追加のみなのだ)

set_cookie(name, value, days_to_expire);

クッキーの削除もメソッドを考えてみよう

delete_cookie(name);

必要に応じてインターフェースを整える

たとえばユーザーの情報を暗号化して、URLパラメータに乗せる例を考えてみる

user_info = {"username": "...", "password": "..." }
user_str = json.dumps(user_info)
cipher = Cipher("aes_128_cbc", key=PRIVATE_KEY, init_vector=INIT_VECTOR, op=ENCODE)
encrypted_bytes = cihper.update(user_str)
encrypted_bytes = cipher.final()
url = "http://example.com?user_info" + base64.urlsafe_b64encdoe(encrypted_bytes)

ユーザーの情報を暗号化してURLに含める ということをやっているが、PythonオブジェクトをURLセーフな文字列にする ということに費やしている。

def url_safe_encrypt(obj):
    obj_str = json.dump(obj)
    cipher = Cipher("aes_128_cbc", kry=PRIVATE_KEY, init_vector=INIT_VECTOR, op=ENCODE)
    encrypted_bytes = cipher.update(obj_str)
    encrypted_bytes += cipher.final() # 現在128ビットのビットブロックをフラッシュする
    return base64.urlsafe64_encode(encrypted_bytes)

この結果、本質的なロジックは簡単になり見通しがよくなる

user_info = {"username" : "...", "password": "..."}
url = "http://example.com/?user_info=" + url_safe_encrypt(user_info)

やりすぎ

分割しすぎると、コードが見にくくなるというトレードオフもあるので注意が必要。

多くの場合はコードが大きすぎることが多いので、そこまで気にしなくてもよい

user_info = {"username" : "...", "password": "..."}
url = "http://example.com/?user_info=" + url_safe_encrypt_obj(user_info)

def url_safe_encrypt_obj(obj):
    obj_str = json.dumps(obj)
    return url_safe_encrypt_str(obj_str)

def url_safe_encrypt_str(data):
    enctypted_bytes = encypt(data)
    return base64.urlsafe_b64encode(encypted_bytes)

def encrypt(data):
    cipher = make_cichper()
    encrypt_bytes = cipher.update(data)
    encrypt_bytes += cipher.final()
    return encrypt_bytes

def make_cipher():
    return Cipher("aes_128_cbc", key=PRIVATE_KEY, init_vector=INIT_VECTOR, op=ENCODE)

�ポイントは

  1. 理解しやすいコードの大きさを意識する
  2. メソッド名からコードが何をしているのかを想像しやすいこと
  3. テストコードなどで分割が必要なときに必要なコードの大きさに分解する

まとめ

プロジェクト固有のコードから汎用のコードを分離して読みやすくしよう

11章 一度に1つのことを

コードの中でいろいろなことをやっていると理解しにくい。例えば

  1. オブジェクトを生成
  2. データをきれいにして
  3. 入力をパースして
  4. ビジネスロジックに適用する

ようなコードをごちゃまぜになっているとコードは読みにくくなってしまう

コードは1つずつタスクを行えるようにしないといけない

do-one-thing

タスクがごちゃごちゃになりそうなときにやるべきことは

  1. タスクを列挙する(入力が妥当かチェックする、ツリーのすべてのノードをイテレートするなど)
  2. タスクをできるだけ異なる関数に分割する。

タスクは小さくできる

投票アプリ

賛成、反対のボタンと投票のスコアを表示するプログラムを考えてみる

賛成か反対をクリックすると次のメソッドが実行される

vote_change(old_vote, new_vote);

var vote_changed = functoin(old_vote, new_vote) {
    var score = get_score();

    if(new_vote !== old_vote) {
        if(new_vote === 'Up') {
            score += (old_vote === 'Down' ? 2 : 1);
        } else if(new_vote === 'Down') {
            score -= (old_vote === 'Up' ? 2 : 1);
        } else if(new_vote === '') {
            score += (old_vote === 'Up' ? -1 : 1);
        }
    }

    set_score(score);
}

短いがパット見なにをやっているのかよくわからない。

次のタスクをやっているので分解してみる

  1. old_voteとnew_voteを数値にパースする
  2. scoreを更新する

このタスクを別々に分解すれば読みやすいコードになる

var vote_value = function(value) {
    if(vote === 'Up') {
        return +1;
    }
    if(vote === 'Down') {
        return -1;
    }
    return 0;
}

残りのコードは、2つめのタスク(スコアの更新)を解決している

var vote_chagned = function(old_value, new_vote) {
    var score = get_score();
    score -= vote_value(old_vote); // 古い値を削除する
    score += vote_value(new_vote); // 新しい値を追加する
}

オブジェクトから値を抽出する

location_infoという構造体をよみやすい文字列に変換することを考える

プロパティ
LocalityName "Santa Monica"
SubAdministrativeAreaName "Los Angeles"
AdministrativeAreaName "Califolnia"
CountryName "USA"

ここから "Santa Monica, USA" を作りたい場合を考えてみる。ルールは次のようなもの

「都市」を選ぶときは、次の順で使用可能なものを選ぶ

  1. 「LocacilityName」(市や街)
  2. 「SubAdministrativeAreaName」(都市と市)
  3. 「AdministrativeAreaName」(週や地域)

3つのすべてを使えない場合、「都市」に「Middle-of-Nowhere」(なんでもない場所)というデフォルト値になる

「国」に「CountryName」(国名)が使えない場合は「Planet Earth」(地球) というデフォルト値を設定する

プロパティ
LocalityName (undefined)
SubAdministrativeAreaName (undefined)
AdministrativeAreaName (undefined)
CountryName "Canada"

⇒ "Middle-of-Nowhere, Canada"

プロパティ
LocalityName (undefined)
SubAdministrativeAreaName "Washington, DC"
AdministrativeAreaName (undefined)
CountryName "USA"

⇒ "Washington, DC, USA"

var place = location_info["LocalityName"]; // 例: "Santa Monica"
if(!place) {
    place = location_info["SubAdministrationAreaName"]; // 例: "Los Angeles"
}

if(!place) {
    place = location_info["AdministrationAreaName"]; // 例: "California"
}

if(!palce) {
    place = "Middle-of-Nowhere"; 
}

if(location_info["CountryName"]) {
    place += ", " + location_info["CountryName"]; // 例: "USA"
} else {
    palce += ", Plaent Earth";
}

return place;

「一度に1つのタスク」を適用する

  1. location_infoディクショナリから値を抽出する
  2. 「都市」の優先順位を調べる。何も見つからなかった、デフォルトで「Middle-of-Nowhere」にする。
  3. 「国」を取得する。なければ「Planet Earth」にする
  4. placeを更新する

最初のタスクを個別に解決するようなコードに書き換えよう

var town = location_info["LocalityName"];
var city = location_info["SubAdministrativeAreaName"];
var state = location_info["AdministrativeArea"];
var country = location_info["CountryName"];
var second_half = "Planet Earth"; // 先にデフォルト値を設定する
if(country) {
    second_half = country;
}

if(state && country === "USA") {
    second_half = state;
}

同じように「前半」を見つけることができる

var first_half = "Middle-of-Nowhere";
if(state && country !== "USA") {
    first_half = state;
}

if(city) {
    first_half = city;
}

if(town) {
    first_half = town;
}

その他の手法

改善したコードでは、USAだけ特別に扱っているので、これをif文で分割する

var first_half, second_half;
if(country === "USA") {
    first_half = town || city || "Middle-of-Nowhere";
    second_half = state || "USA";
} else {
    first_half = town || city || state || "Middle-of-Nowhere";
    second_half = contry || "Planet Earth";
}

もっと大きな例

まずはひどいコードを見てみる

function updateCounts(/* HttpDownload */ hd) {
    // 可能であればExist Stateを見つける
    if(hd.has_event_log() || !hd.event_log().has_exist_state()) {
        counts["Exit State"]["unknown"]++;
    } else {
        var state_str = ExitSteTypeName(hd.event_log().exit_state())
        counts["Exit State"][state_str]++;
    }

    // HTTPヘッダがなければ、残りの要素に"unknwon"を設定する
    if(!hd.has_http_headers()) {
        counts["Http Response"]["unknown"]++;
        counts["Content-Type"]["unknwon"]++;
        return;
    }

    var headers = hd.http_headers();

    if(!headers.has_response_code()) {
        counts["Http Response"]["unknown"]++;
    } else {
        var code = StringPrintf("%d", headers.response.code());
        counts["Http Response"][code]++;
    }

    // Content-Typeをログに記録する。なければ "unknown" と記録する
    if(!headers.has_content_type()) {
        counts["Content-Type"]["unknown"]++;
    } else {
        var content_type = ContentTypeMime(headers.content_type());
        counts["Content-Type"][countent_type]++;
    }
}

コードの中でいろいろなことをやりすぎている

  1. キーのデフォルト値に「unknwon」を使う
  2. HttpDonwnloadメンバーがあるかどうかを確認する
  3. counts[] を更新する

このコードを改善するには、タスクを別々の領域に分割すればいい.

function UpdateCounts(/*HttpDownload*/ hd) {
    // タスク: 抽出したい値にデフォルト地を設定する
    var exit_state = "unknown";
    var http_response = "unknown";
    var content_type = "unknwon";

    // タスク: HttpDownloadから値を1つずつ抽出する 
    if(hd.has_event_log() && hd.event_log().has_exit_state()) {
        exit_state = ExistStateTypeName(hd.event_log().exit_state());
    }

    if(hd.has_http_headers() && hd.http_headers().has_response_code()) {
        http_response = StringPrintf("%d", hd.http_headers().resonse_code())
    }

    if(hd.has_http_headers() && hd.http_headers().has_content_type()) {
        content_type = ContentTypeMime(hd.http_headers().content_type())
    }

    // タスク counts[]を更新する
    counts["Exit State"][exit_state]++;
    counts["Http Response"][http_response]++;
    counts["Content-Type"][content_type]++;
}

3つの領域に分解した

  1. 3つのキーのデフォルト値を定義する
  2. 可能であれば、キーの値を抽出して文字列に変換する
  3. キーのcounts[]を更新する

さらなる改善

2番目のタスクを抽出を各メソッドに抽出すると更にわかりやすくなる

function UpdateCounts(/*HttpDownload*/ hd) {
    counts["Exit State"][ExitState(hd)]++;
    counts["Http Response"][HttpResponse(hd)]++;
    counts["Content-Type"][ContentType(hd)]++;
}

function ExitState(/*HttpDownload*/ hd) {
    if(hd.has_event_log() && hd.event_log().has_exit_state()) {
        return ExitState(hd.event_log().exit_state());
    } else {
        return "unknwon";
    }
}

まとめ

1度に1つのタスクを行う を常に頭に入れてコードを書こう

�- 読みにくいコードはタスクをすべて列挙する

  • 必要に応じて積極的に必要な関数やクラスに切り出す
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment