読みやすいコードを書くために

iOSエンジニア見習いが、リーダブルコードを読んで、今まで意識できていなかったことをメモしています。いつか書き直します。

参考文献

この記事は以下の書籍の情報を参考にして執筆しました。

• リーダブルコード

1章 - 理解しやすいコード

コードは他の人が最短で理解できるように書く。

同僚や未来の自分が見たときに、変更やバグを見つけれるようになるまでの時間を最短にしなければならない。

短いことが絶対にいいことではない

このような１行のコードは

assert((!(bucket = FindBucket(key))) || !bucket -> IsOccupied());

以下の２行と同じ処理だが理解にかかる時間は下のコードの方が短い。

bucket = FindBucket(key)
if (bucket != NULL) assert(!bucket -> IsOccupied)

2章 - 名前に情報を埋め込む

ループイテレータ

i・j・kなどはループイテレータとしてよく使われているので問題はない。
しかし下記のような例ではもっといい書き方がある

swift

for i in clubs {
  for j in i.members {
    for k in students {
      if(i == k){ print("\(k)") }
    }
  }
}

また先ほどのコードではif文でi == k と記述しているためエラーだが、
このように書くことでバグが目立ちやすくなり修正が容易になった。
club_iを簡潔にし、ci、si、miと書くのもあり。

swift

for club_i in clubs {
  for member_i in club_i.members {
    for student_i in students {
      if(member_i == student_i){ print("\(student_i)") }
    }
  }
}

名前に情報を追加する

Webページの読み込み時間を計測するJavaScriptのコード
ここでgetTimeが秒ではなくmsを返すのでうまく動かない。

JavaScript

var start = (new Date()).getTime();
・
・
・
var elapsed = (new Date()).getTime() -start;
document.WritteIn("読み込み時間: " + elapsed + " 秒")

変数名に単位を追加することでより明確になる。

JavaScript

var start_ms = (new Date()).getTime();
・
・
・
var elapsed_ms = (new Date()).getTime() -start;
document.WritteIn("読み込み時間: " + elapsed_ms / 1000 + " 秒")

その他重要な属性の追加

危険や注意を促す情報を追加する

var passward
この変数に入る値はプレーンテキストなので処理をする前に暗号化する必要がある。
var plaintext_passward

var data
入力されたデータをURLエンコードしている。
var data_urlenc

スコープが小さければ短い名前でもいい

識別子の見える範囲が小さければ短い名前でもOK。
スコープの範囲が広いクラスのメンバやグローバル変数はわかりやすく具体的な名前をつける。

if (debug) {
  map<String, int>m;
  LookUpNamesNumbers(&m)
  Print(m)
}

不要な単語を捨てる

ConvertToStringをToStringにしても必要な情報は損なわれない。
出来るだけ無駄な情報を省いて記述する。

名前のフォーマットで情報を伝える

アンダースコア、ダッシュ、大文字を使って名前に情報を詰め込むこともできる。
Google社のオープンソースプロジェクトで使っているC++のフォーマットを見ると、

static const int kMaxOpenFiles = 100;

class LogReader {
  public:
    void OpenFile(string local_file);

  private:
    int offset=;
    DISALLOW_COPY_AND_ASSIGN(LogReader);
};

エンティティごとに異なるフォーマットを使っている。
よく見る記述はクラス名はキャメルケースで変数名は小文字をアンダースコアで区切ったもの。
例えば定数はCONSTANT_NAMEではなくkConstantNameになっていて、MACRO_NAMEのような#defineマクロと区別できるようになっている。
クラスのメンバ変数は最後の文字がアンダースコアになっており普通の変数と区別できるようになっている。

3章 - 誤解されない名前

例:filter

results = Database.all_objects.fillter("year <= 2010")

ここでresultsには「year <= 2010のオブジェクト」か「year <= 2010ではないオブジェクト」
どちらが入っているかの誤解が生じる可能性がある。
選択するのあればselect、除外するのであればexcludeを使う。

限界値を明確にする場合はminとmaxを使う。

範囲を指定するときはfirstとlastを使う。

first →[a] [b] [c] [d]← last

包含、排他的範囲にはbeginとendを使う。

begin →[a] [b] [c] [d] [ ]← end

包含、排他的範囲の使い道は
10/16に開催されたイベントを全て示したい場合など、

10/16 12:00am ~ 10/17 12:00am

のように指定する方が、範囲で下記のように指定するより簡単。

10/16 12:00am ~ 10/16 11:59:59.99999pm

ブール値を示す変数名

swift

var read_password: Bool = true

このような変数があった場合、すでに読み取ったか、これから読み取るかよくわからない。
なのでneed_passwordやuser_is_authenticatedなどを使う。

ユーザの期待に合わせる

getで始まるメソッドはメンバ等の値を返すだけの軽量アクセサであるという規約に慣れ親しんでいる。
例えばgetMeanというメソッドを作って中に計算処理を書いた場合、ほとんどのユーザはコストが高いと思わずにgetMeanを呼び出してしまう。
なのでこのような場合はcomputeMeanなどに変えるべきである。

4章 - 美しさ

コードを美しく書いて読みやすく

縦の線をまっすぐにする

swift

helloWorld("佐藤"     , "太郎")
helloWorld("Hoge"    , "Fuga")
helloWorld("Florence", "Nightingale")

swift

let a           = User("A")
let hoge        = User("hoge")
let nightingale = User("Nightingale")
let taro        = User("太郎")

5章 - コメントすべきこと

コメントを読むとその分コードを読む時間がなくなっているので、コメントにはそれだけの価値が必要。

コメントすべきでないこと

コードからすぐにわかることをコメントに書かない。

swift

// クラスUserの定義
class User {
  // 名前を格納するプロパティ
  var name: String
  // イニシャライザ
  init(_ n: String){
    // nameに値を入れる
    name = n
  }
}

コメントで名前の補足をせず名前を変える

// レジストリーキーのハンドルを解放する。実際のレジストリは変更しない。
void DeleteRegistry(Registry);

DeleteRegistryという名前からはレジストリを削除するように読みとれるが、
コメントで実際のレジストリを変更しないことを補足している。
これは関数の名前を変更することで解決できる。ReleaseRegistryHandleなど。

自分の考えを記録する

コードを書いた時に持っている大切な考えを記録する。

例
// このデータだとハッシュテーブルよりバイナリツリーの方が40%早かった。
// 左右の比較よりハッシュ計算の方がコストが高いようだ。

// このクラスは汚くなってきている。
// サブクラスを作って整理した方がいいかもしれない。

コードの欠陥にコメントをつける

これからどうしたいのかを自由に書くことで、コードの品質や状態を知らせたり、改善の方向を知らせたりする。

例
// TODO: もっと高度なアルゴリズムを使う

プログラマがよく使う記法

記法	典型的な意味
TODO:	後で手をつける
FIXME:	既知の不具合があるコード
HACK:	あまり綺麗じゃない解決法
XXX:	危険！大きな問題あり

定数にコメントをつける

定数を宣言する時にどういう意図をもってその値を設定したか残しておく。

swift

// 合理的な限界値、人間はこんなに読めない。
let MAX_RSS_SUBSCRIPTIONS = 1000

質問されそうなことを想像する

このコードを見たC++プログラマはどうしてdata.Clear()をせずに空のベクタをスワップするのか疑問に思う。
このようにしているのはベクタのメモリを解放してメモリあろケータに戻す方法がこれしかないからだ。
これはあまり知られていないことなので、これをコメントに残す。

c++

void Clear() {
  // ベクタのメモリを解放する。「STL swap 技法」で検索してみよう。
  vector<float>().swap(data);    // ココ
}

6章 - コメントは正確で簡潔に

曖昧な代名詞を避ける

これ、あれ、それなど
例
// データをキャッシュに入れる。ただし先にそのサイズをチェックする。
この文ではそのサイズとはデータなのかキャッシュなのかわからないので書き換える。
// データをキャッシュに入れる。ただし先にデータのサイズをチェックする。

歯切れの悪い文章を磨く

例
// これまでクロールしたURLかどうかによって優先度を変える。
変更後
// これまでにクロールしていないURLの優先度を高くする。
こちらの方が単純で短くて直接的になる。

関数の動作を正確に記述する

例

// このファイルに含まれる行数を返す
int CountLines(string filename){・・・}

このようなコメントがあったとして、行数とは具体的に何か、空のファイルは0行か1行か"hello\n"は1行か2行かよくわからない。
変更後

// このファイルに含まれる改行文字('\n')を数える。
int CountLines(string filename){・・・}

実例を使ってわかりやすく

例

// 'str'の銭湯や末尾にある'chars'を除去する。
String Strip(String src, String chars){・・・}

このコメントではcharsは除去する文字列なのか、順序のない文字集合なのか。strの末尾に複数のcharsがあったらどうなるのか。といった質問に答えられない。

変更後

// 'str'の銭湯や末尾にある'chars'を除去する。
// 実例: Strip("abba/a/ba", "ba")は"/a/"を返す。
String Strip(String src, String chars){・・・}

これはStripのすべての機能を見せている。簡単すぎる実例では意味がないので注意。

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

無理に三項演算子を使わない

三項演算子を使った方が読みやすい例

time_str += (hour >= 12) ? "pm": "am";

使わなかった場合

if (hour >= 12) {
  time_str += "pm";
} else {
  time_str += "am";
}

三項演算子を使わない方が読みやすい例

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

無理に１行にまとめずにif文を使って書くと読みやすい。

if (exponent >= 0){
  return mantissa * (1 << exponent)
} else {
  return mantissa / (1 << exponent)
}

関数から早く返す

複数のreturn文を使ってはいけないと思われがちだが、関数から値を早く返す方が無駄がなくて望ましいことの方が多い。

ネストを浅くする

ネストが深いコードは理解しにくい。
下記のようにif文の処理でif文を使うようなプログラムがあるとする。

if (user_result != SUCCESS){
  if (permission_result != SUCCESS){
    reply.writeErrors("permission_result");
    reply.Done();
    return;
  }
  reply.writeErrors("");
} else {
  reply.WriteErrors("user_result")
}
reply.Done();

変更してif文を外に出す。
このようにすることでネストの深さが2から1になった。
読むときもif文を１ブロックずつ読んでいけばいいので可読性も上がる。

if (user_result != SUCCESS){
  reply.writeErrors(user_result);
  reply.Done();
  return;
}

if (permission_result != SUCCESS){
  reply.writeErrors(permission_result);
  reply.Done();
  return;
}
reply.WriteErrors("");
reply.Done();

8章 - 巨大な式を分解する

説明変数

if line.split(':')[o].strip() == "root":
・・・

説明変数を使えば以下のように記述できる。

user_name = line.split(':')[o].strip();
if user_name = "root":
・・・

要約変数

if (request.user.id == document.owner_id){ ・・・}
if (request.user.id != document.owner_id){ ・・・}

request.user.id == document.owner_idは大きな式ではないが変数が複数入っているので考えるのに少し時間がかかる。
このコードではユーザが文書を所持しているかをチェックしているだけなので、要約変数を追加すればこの概念を明確にできる。

final boolean user_owns_document = (request.user.id == document.owner_id);
if (use_owns_document){・・・}
if (!use_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)

下記のような評価式があったとする。

if(!(file_exists && !is_protected))

この式の場合、ド・モルガンの法則を使って書き直した方が読みやすくなる。

if(!file_exists || is_protected)

9章 - 変数と読みやすさ

変数が多いと変数を追跡するのが難しくなる。
変数のスコープが大きいとスコープを把握する時間が長くなる。
変数が頻繁に変更されると現在の値を追うのが難しくなる。

変数を削除する

一旦値を保持しているだけの変数をなくす。

Python

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

nowはdate time.datetime.now()をより明確にしているわけではないので下記のように変更する。

Python

root_message.last_view_time = datetime.datetime.now()

変数のスコープを縮める

変数のスコープを縮めることによってコードを追いやすくなる。

１度だけ書き込む変数を使う

swiftだと初期値を代入してないletなど

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

コードの高レベルの目標に直接的な効果のないコードは下位問題を解決しているコード。
下位問題を解決しているコードはまとめて切り離す。

11章 - 1度に1つのことを

1度に1つのタスクを行う。

10章11章の例

位置情報を取得し、位置情報をもとに天気を取得するAPIにリクエストを投げ、帰ってきた処理をreult関数に渡す処理を書くとする。
下記の例では、位置情報を取得する。URLを作成する。APIから天気を取得する。という３つのタスクを１つのメソッド内に記述している。

swift

  func locationManager(_ manager: CLLocationManager, didUpdateLocations locations: [CLLocation]) {
    if let location = locations.first{
      let latitude = location.coordinate.latitude
      let longitude = location.coordinate.longitude

      // リクエストURLの作成
      let baseUrl = "http://api.openweathermap.org/data/2.5/forecast"
      let gpsOption = "?lat=\(location)&lon=\(longitude)"
      let modeOption = "&mode=json"
      let cntOption = "&cnt=14"
      let idOption = "&APPID=hoge"
      let requestUrl = baseUrl + gpsOption + modeOption + cntOption + idOption

      // APIにリクエストを投げて天気を取得
      Alamofire.request(requestUrl, method: .get)
        .responseJSON { response in

          let json = JSON(response.result.value!)
          result(json)

      } 
    }

これをタスクごとに分け、URLを作成する処理を抽出する。

swift

// 位置情報取得時呼び出される
  func locationManager(_ manager: CLLocationManager, didUpdateLocations locations: [CLLocation]) {
    if let location = locations.first{
      let latitude = location.coordinate.latitude
      let longitude = location.coordinate.longitude

      observeWeather(latitude: latitude, longitude: longitude)
    }
  }

  func observeWeather(latitude: Double, longitude: Double) {
    let requestUrl = getRequestUrl(lat: latitude, lng: longitude)
    Alamofire.request(requestUrl, method: .get).responseJSON { response in
      let json = JSON(response.result.value!)
      result(json)
    } 
  }

  func getRequestUrl(lat: Double, lng: Double) -> String{
    let baseUrl = "http://api.openweathermap.org/data/2.5/forecast"
    let gpsOption = "?lat=\(lat)&lon=\(lng)"
    let modeOption = "&mode=json"
    let cntOption = "&cnt=14"
    let idOption = "&APPID=hoge"
    return  baseUrl + gpsOption + modeOption + cntOption + idOption
  }

12章 - コードに思いを込める

ユーザーにドキュメントを回覧する権限があるかどうかを確認して、権限がないことを返す処理があったとする。

swift

  let document: Document? = getDocument
  let is_admin = is_admin_request(username: username)

  if let doc = document {
    if (!is_admin && (username != doc.username)){
      return not_authorized()
    }
  } else {
    if (!is_admin) {
      return not_authorized()
    }
  }

このロジックを単純化させたいと考えたときに、簡単な言葉でロジックを説明してみる。
権限があるのは2つ
管理者、ドキュメントの所有者
そのほかは権限がない。

これをコードに適応してみる。

swift

  let document: Document? = getDocument

  if (is_admin_request(username: username)){
    //権限あり
  } else if (username == document?.username){
    //権限あり
  } else {
    return not_authorized()
  }

このように書くとif文の中身が２つも空になってしまうが、コードは小さくなり、否定形がなくなったのでロジックが単純になった。
つまりこっちの方が理解しやすい。

13章 - 短いコードを書く

・既存のAPI、ライブラリ、メソッドで解決する。
・使っていない機能をプロジェクトから削除する。
・最も簡単に問題解決できるような要求を考える。

例えば既存の配列をもとに新しい配列を作る。要素の値を２倍し要素の後ろに要素0を入れる。
ということをしたかったときに下記のように書いて動作させた。

swift

    let array = [1, 2, 3, 4, 5, 6]
    var newArray = [Int]()

    for i in array{
      newArray.append(i*2)
      newArray.append(0)
    }
    print(newArray)    // [2, 0, 4, 0, 6, 0, 8, 0, 10, 0, 12, 0]

しかしflatMapを使って書いた方が実装が楽で、新しい配列も定数で宣言できるのでより良い。

swift

    let array = [1, 2, 3, 4, 5, 6]
    let newArray2 = array.flatMap { [$0 * 2, 0] }
    print(newArray2)    // [2, 0, 4, 0, 6, 0, 8, 0, 10, 0, 12, 0]

14章 - テストコードと読みやすさ

他のプログラマが安心して追加や変更ができるように、テストコードを読みやすくする。

テストの入力値

必要でない大きな値は使わずに単純な数値を用いる。
大きな数字を用いるのであれば「1e100」のように簡潔に記述する。

・悪い例
add(-500009, -3, 4, 1, -3333333, 87658976)

・訂正後
add(1, 2, 3, -1, 2)

テストの機能に名前をつける

テストコードの名前もわかりやすいものにしなければならない。Test1などは何のテストかがわからない。

Test_addのように機能名や関数名をつけるのが望ましい。

テストに優しい開発

開発をするときにテストコードを描くことを意識して処理を書くことで、振る舞いごとに処理を分けて書くこともできる。