「リーダブルコード」をよんで

更新中

読みやすいコードを書く

行数が小さく、読みづらいと感じるコードよりも行数が大きくなっても読みやすい簡潔なコードを書くべき。

読みづらい例.

return nodes != null ? nodes * 3 : 0

読みやすい例.

if (nodes != null) {
   return nodes * 3;
} else {
   return 0;
}

変数やクラス名、関数名には情報を詰め込む

明確な単語を選ぶ

例えば getPage() という関数が存在した場合、こちらの関数で使用されている get はあまり明確な単語ではない。getのみだとどこから取得するのか、また取得するにしてもどのように取得するのかまで想像しにくい。
例えばネット上から取得するなら downloadPage() や fetchPage() でもよい。他にもDBから検索して1件のみ取得する場合ならfindPage()も候補の一つ。
他の命名系のQiitaの記事を見ているとコレクションから一つの値を取得するためにpluck(摘む)を使用されていらっしゃる方もいた。ただ取得する部分にフォーカスするのではなく状況や動作から適切な英単語を探すようにするべき。

汎用的な名前を避ける

例えば関数内で使用する戻り値の変数に retval という意味の変数がたまにある。この変数には「戻り値である」という意味以外に情報がない。故にどんな戻り値なのか、その戻り値の目的は何なのかわかりにくい。他、temp(一時的)も避けるべき。
処理結果を意味する単語を命名したり状態をよりくわしく説明するのが良い。

例1 : retval -> sum_squares(2乗した合計値である情報を追加)
例2 : temp -> temp_file(ファイルである情報を追加)
※いづれにしてもtempはスコープが小さい変数に対して使用すべき

具体的な名前を使う

・値の単位を付ける。

単位がない.

long start = new Date().getTime();

単位がある.

long start_ms = new Date().getTime();

DateクラスのメソッドgetTimeはミリ秒を返すので下の単位を付けた例のほうが好ましい。上の例だと秒を返すものだと勘違いし、計算をするうえでバグが含まれやすくなる。またすでに書かれているコードから単位が付与されているほうがバグを発見しやすい。

例1 : sleep_time -> sleep_time_sec(寝た秒数)
例2 : download(float limit) -> download(float max_kbps)(最大転送用(kbps))

・属性情報を追加する
単位だけでなく変数に入っている値の情報がどんな状態なのか情報を追加することでバグを予防できる。

例 : 属性情報なし : password , 属性情報あり : plaintext_password

password のみだとパスワードが暗号化されているのかされていないのか見ただけではわからないが後者のようにplaintextとつけるだけで「まだパスワードは暗号化されていないもの」だと一目でわかる。

名前の長さを決める

覚えづらいし折り返しが必要になるほど長ければ行も増える可能性があるので暗黙的に長い名前の命名は避けるべき。どうしても長い名前を付与しなければいけない場合は単語を省略するか、再度単語の中に最低限なくしても意味が伝わる部分がないか検討してみる。

例1, BackEndEngineer -> BEEngineer (BackEndを省略)
例2, ConvertToString -> ToString (文字列に変換するならConvertなくても伝わる)

※省略方法の参考
https://qiita.com/Yusuke196/items/b5e51ee6d77ca1b672c2

誤解されない命名の仕方

名前が他の意味として受け取られることがないか常に自問自答してみる

filterはややこしい

付与された条件で絞り込む処理の関数名で使用されることが多いfilterだが「リーダブルコード」では推奨されていない。例えば下記のようなコードを目の当たりにしたとき、filterの条件に当てはまるデータを制限(弾いている)しているのか、条件にあてはあるものを抽出しているのかわかりにくいからだ。

filter.

results = Database.all_objects.filter("year <= 2011");

results にはyear <= 2011に当てはまるデータ？year <= 2011ではないデータのどちらが格納される？

選択(抽出)するならselectが良いし、除外するならexcludeもよいだろう。

filter.

selected_results = Database.all_objects.select("year <= 2011");
excluded_results = Database.all_objects.exclude("year <= 2011");

限界値を決めるときは min, max を使う

あるカートには商品が10点までしか入らない仕様だとするとき、下記の変数名はわかりにくい。

CART_TOO_BIG_LIMIT = 10;

この場合、下記のようなif分でバグを生み出してしまう可能性がある(境界条件に関するエラー)

境界条件に関するエラー.

if (shopping_cart.num_items >= CART_TOO_BIG_LIMIT) {
   Error("カート内の商品が多すぎます");
}

本来は10個まで入れても問題ないカートのはずだが名前が曖昧なことが原因でカートに入っている商品が10個の場合でもエラーを吐いてしまう実装をしてしまう恐れがある。
原因の根幹は変数に入っている値を含むのか含まないのか変数名から読み取ることができない点にある。

そこで変数名にmax_を使用してみる。

変数名にmax_を使用.

MAX_NUM_ITEM_IN_CART = 10;
if (shopping_cart.num_items > MAX_NUM_ITEM_IN_CART) {
   Error("カート内の商品が多すぎます");
}

maxという単語から限界という意味が暗黙的に付与されて値を含有していることが最初の変数名よりはわかるようになったと思う。

他、範囲を示す場合にはmin、max以外に始発点にstart、終点にlastを変数名に付与するのもよい。この使い分けは状況に応じて使い分けたほうがよさそう。

Bool値を決める名前

例えば下記のコードを見て bool値のread_passwordはどんな条件でtrue、もしくはfalseが入るかわかるでしょうか。

bool値.

bool read_password = true;

上記の変数はこれから読み取る必要のあるパスワードである場合にTrueが格納されるのか、
すでに読み取ったパスワードである場合にTrueが格納されるのかわかりにくい。

なので例えばこれから読み取る必要のあるパスワードか否かなら

これから読み取る必要のあるパスワードか否か.

bool need_password = true;

すでに読み取ったパスワードである場合か否かなら

すでに読み取ったパスワードである場合か否か.

bool is_authenticated_password = true;

がよさげ。

※bool値の変数名には接頭辞にis、has、can、shouldを付与するとわかりやすい。
※名前は否定形はやめたほうが良い → bool disable_do = false; → わかりづらい。

コードの美しさ

コードの見栄えを気にする

美しいコードほど読みやすいし、内容の理解も早い。

例えば下記のように関数名が長く引数の値が数値でそれぞれコメントで情報を付与しているパターン

関数名が長く引数の値が数値でそれぞれコメント.

public static final TcpConnectionSimulator wifi = new TcpConnectionSimulator(
   500, /* kbps */
   80, /* millisecs latency */
   200, /* jitter */
   1 /* packet loss % */
);

public static final TcpConnectionSimulator t3_fiber = new TcpConnectionSimulator(
   45000, /* kbps */
   10, /* millisecs latency */
   0, /* jitter */
   0 /* packet loss % */
);

幅も広いし、縦にも長いし、同じコメントが付与されていて非常にわかりにくい。
下記のように書き直すことで読みやすくなるはずだ。

関数名が長く引数の値が数値でそれぞれコメント.

// TcpConnectionSimulator(kbps, millisecs latency, jitter, packet loss %)

public static final TcpConnectionSimulator wifi =
   new TcpConnectionSimulator(500  , 80  , 200 ,    1);

public static final TcpConnectionSimulator t3_fiber =
   new TcpConnectionSimulator(45000, 10  , 0   ,    0);

初期化以降の文を折り返すことで幅を狭めることに成功し、同じコメントを二回繰り返さないようにすることで縦の長さも解消、また引数を区切るカンマを関数間でに縦にそろえることでより見やすくなった。

カンマを縦にそろえることは下記のようにオブジェクトの配列を格納している例だと効果を実感しやすい。

オブジェクトの配列.

person[] = [
   { 'name' : 'tanaka taro'      , 'age' : 23, 'bloodType' : 'O' , 'from' : 'Tokyo' },
   { 'name' : 'suzuki ichiro'    , 'age' : 23, 'bloodType' : 'AB', 'from' : 'Saitama' },
   { 'name' : 'lionel messi'     , 'age' : 23, 'bloodType' : null, 'from' : 'Hokkaido' },
   { 'name' : 'cristiano Ronaldo', 'age' : 23, 'bloodType' : 'A' , 'from' : 'Mie' },
]

オブジェクトの中身が整理されてわかりやすくなったと個人的に感じた。

制御フローをわかりやすくする

条件式の引数の並び順

下記二つの制御フローだと上のほうがわかりやすい。

調査対象が左辺.

if (length > 10) { // 処理 }

比較条件が左辺.

if (10 < length) { // 処理 }

二つ目の例でも同様に上のほうがわかりやすい。

調査対象が左辺.

if (received_bytes < expect_bytes) { // 処理 }

比較条件が左辺.

if (expect_bytes > received_bytes) { // 処理 }

これは言葉に置き換えて考えるとわかりやすく「もし調査対象が比較条件に合致するならば、、、」のように調査対象を左辺に先に記述することによって読みやすくなるからだ。

なので if 文を使用した制御文を作る際は左辺に調査対象(変数)、右辺に比較対象(固定値)を置くのが可読性が上がる。

if else 文の条件式の書き方

・条件は否定形より肯定形を使用する
・単純な条件を先に書く
・関心を引く条件や目立つ条件を先に書く。

if (a == b) {
  // 処理
} else {
  // 処理
}

ネストを浅くする

if文のネストが深いと自分が読んでいるコードがどこのブロックに所属しているのか常に確認しながらいけないため、可読性が下がる。また各条件に合致しているのか不一致なのかも常に頭に入れながら読まなくてはいけないため、疲れる。

if (user_result == SUCCESS) {
  if (permisstion_result != SUCCESS) {
    replyWriteErrors("error permission");
    reply.done();
    return;
  }
  replyWriteErrors("");
} else {
  replyWriteErrors(user_result);
}
reply.done();

このようなコードが埋めれてしまうのは後から仕様が変わって単純に内部にネストする形でif文を付け足してしまったりしているからで、使用が変わったりしてコードに変更を加える際は一度、コード全体を見直して読みやすいか、よりシンプルなつくりにできないかを考えるべき。

例えば下のように直すと少し読みやすい。

ポイントとしては
・早めにブロックから脱出するようにする。

if (user_result != SUCCESS) {
  replyWriteErrors(user_result);
  reply.done();
  return;
}

if (permisstion_result != SUCCESS) {
  replyWriteErrors("error permission");
  reply.done();
  return;
}

replyWriteErrors("");
reply.done();
return;

汎用コードを作る

一つの処理を冗長に長々と書くのはあまり好ましくない。
読まないといけないコード量が膨大になって本来の高レベルの目的を理解するのに時間が掛かってしまう。下記コードは汎用的なコードを抽出する前後の例だ。

抽出前.

var findClosestLocation = function(lat,lng,array) {
  var closest;
  var closest_dist = Number.MAX_VALUE;
  for (var i = 0; i < array.length; i += 1) {
    
    var lat_rad = radians(lat);
    var lng_rad = radians(lng);
    var lat2_rad = radians(array[i].latitude);
    var lng2_rad = radians(array[i].longitude);

    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;
}

var spheqrical_distance(lat1, lng1, lat2, lat2) {
  var lat_rad = radians(lat1);
  var lng_rad = radians(lng1);
  var lat2_rad = radians(lat2);
  var lng2_rad = radians(lat2);

  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));
}

var findClosestLocation = function(lat,lng,array) {
  var closest;
  var closest_dist = Number.MAX_VALUE;
  for (var i = 0; i < array.length; i += 1) {
    // 抽出部分
    var dist = spheqrical_distance(lat, lng, array[i].latitude, array[i].longitude)
    if (dist < closest_dist) {
      closest = array[i];
      closest_dist = dist;
    }
  }
  return closest;
}

冗長部分をspheqrical_distance関数として抽出し、コードの可読性がぐんと上がった。
またspheqrical_distanceの中身を切り離したことにより何度もspheqrical_distance関数の中身を気にする必要がなくなった。
関数としてテストも行えるし、使いまわせるようにもなった。
また今回のケースだけでなく今後、関数内に微修正依頼が入った場合にも修正が比較的簡単に行えるというメリットがある。