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

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

この記事は、社内向けに書いた資料を公開の許可を得て加筆修正したものです。

記事中の具体例やサンプルコードはJavaScript/TypeScriptで書かれていますが、内容自体は言語にかかわらず使えます。

同僚の @shisama にも手伝っていただきました。

随時更新していきます。モチベーションが続けば。

はじめに

読みやすいコードは、コメントがなくても文章を読むように理解できます。

逆に、読みにくいコードはコメントがあってもさっぱり意味がわかりません。

文章を読むように理解できるコードを書くために普段気をつけていることや、コードレビューの際に重点的に見ているところをまとめました。

普段から読みやすいコードを心がけている方にとっては何も目新しい物はなく、当たり前に気をつけているであろうことばかりです。

特にプログラミングを始めたばかりの方は参考にしてみてください。

命名について

読みやすいコードを書く第一歩は、わかりやすい名前をつけることです。

わかりやすい名前とは、具体的には以下のようなものです。

• 役割がわかる名前
• 誤解を招かない名前

変数名・プロパティ名

以下のことがわかるような名前をつけましょう。

• どんな情報が入っているか？
  
  
  • ユーザーIDか？ユーザー名か？それらを含むユーザー情報オブジェクトか？
  • 入っている情報がわかれば、型も容易に推測できます。
• 単位は何か？
  
  
  • 例えば時間の場合、日か時間か秒かミリ秒か（それともYYYYMMDDの文字列かDateクラスのインスタンスかMoment.jsのインスタンスか）曖昧になりがちなので、はっきりわかる名前にしてください。

悪い例

• 
  data
  
  
  • あらゆる情報はデータなので、これだけでは何が入っているのかわかりません。
  • 一時的に使う、スコープが数行程度の変数（ソート用の比較関数の引数等）であればあまり問題にはなりませんが、処理の根幹に関わる変数がこんな名前だと混乱の元です。
• 
  user
  
  
  • どんなユーザー情報が入っているのかわかりません（ユーザーID？ユーザー名？{id: 1, name: "John Doe"}といったオブジェクト？）
  • 唯一許されるのは、Userクラスのインスタンスの場合のみです。
• 
  startTime
  
  
  • 時間をどんな形で持っているのかわかりません（Dateオブジェクト？YYYYMMDD形式の文字列？Unix時間？）
  • Unix時間だとしても、単位が秒なのかミリ秒なのかわかりません。

いい例

• 
  userId
  
  
  • ユーザーIDだとわかる
  • 重複のない一意の値だとわかる
  • 多分数値型じゃないかな
• 
  startTimeUnixMilliSec
  
  
  • Unix時間（数値型）だとわかる
  • 単位はミリ秒だとわかる

ただし、あまり細かくやりすぎると長くなるのも事実なので（上のstartTimeUnixMilliSecも長いですね）、文脈から明らかにわかる情報は削っても構いません。

（明らかにわかる情報という条件つきですが）

関数名

以下のことがわかるような名前をつけましょう。

• 何をしているか？
• 引数はどう使うか？
• どんな情報を返すか？
• 将来の拡張や関数の追加に対応できるか？

悪い例

• 
  user()
  
  
  • ユーザーをどうにかする関数だろうと推測できますが、単一ユーザーを取得するのか、複数ユーザーを取得するのか、ユーザー数を数えるのか、ユーザー情報を更新するのか、ユーザーを削除するのか、有効なユーザーかどうかを調べるのか全くわかりません。
• 
  findUsers(name)
  
  
  • ユーザーを絞り込む関数だろうと想像できますが、関数名だけでは「名前で」絞り込むことを推測できません。
  • 将来、「ユーザーを所属グループで絞り込む関数」を追加するかもしれません。名前で絞り込む頻度が他の絞り込み方法に比べて圧倒的に高いとかでない限り、findUsers()という「特等席」を与えるべきではありません。

いい例

• 
  countUsers()
  
  
  • ユーザー数を取得する関数だとわかる
  • 戻り値はユーザー数（数値型）だとわかる
• 
  findUsersByName()
  
  
  • ユーザーを名前で絞り込む関数だとわかる
  • 引数は絞り込む名前だとわかる
  • 戻り値はUserクラスのインスタンスだとわかる
• 
  user.isAvailable()
  
  
  • 有効なユーザーかどうかを返すメソッドだとわかる
  • 戻り値はboolean型で、有効であればtrueだとわかる
• 
  group.hasUser()
  
  
  • グループにユーザーがいるかどうかを返すメソッドだとわかる
  • 戻り値はboolean型で、ユーザーがいればtrueだとわかる
• 
  user.removed()
  
  
  • ユーザーが削除されているかどうかを返すメソッドだとわかる
  • 戻り値はboolean型で、削除されていればtrueだとわかる
  • 
    isRemoved()でも問題ありませんが、removed()だけでも通じます。
  • ただし、同時にremove()メソッドもあったりすると紛らわしいので取り扱い注意。

このように、関数は「何かをするもの」なので、原則として動詞を頭につけてください。

「●●かどうか」を返す関数（boolean型）は、上記のように三人称単数現在形や過去分詞を使いましょう。

便利な動詞たち

関数名によく使われる動詞です。

「その関数が何をやっているか、何を返すか」がわかりやすいものを選んでください。

取得系

• 
  get
  
  
  • 取得する
  • 万能感があります。安易に飛びつかず、以下のfetch / find / searchといった、より限定的な意味の動詞を使うことを検討してください。
• 
  fetch
  
  
  • （別の場所から）取ってくる
  • DBやネットワーク越しに何かを取得する場合に使います。
• 
  find
  
  
  • 探す
  • 複数の候補の中から絞り込む場合に使います。
  • 例: findUserById() - 複数のユーザーの中から、指定のIDに一致するユーザーを取得
• 
  search
  
  
  • 探す
  • 
    findは候補の中から絞り込むのに対して、searchは対象の中を探します（特定の名前を持つファイルを「複数のファイル」から探す場合はfind、「ディレクトリの中で」探す場合は search ）
• 
  list
  
  
  • 複数（0以上）取得する
  • 
    listを名詞として扱い、getListOfUsers()のような名前（ユーザー一覧を取得）でも間違いではないと思いますが、listUsers()（ユーザーを列挙する）のほうが簡潔に書けます。
  • listを使わずに、findUser()であれば単一のユーザー情報を、findUsers()であれば複数の情報を返すという方法でもいいと思います。
• 
  count
  
  
  • 条件に合うものの数を数える
  • 
    countを名詞として扱い、getCountOfUsers() のような名前（ユーザーの数を取得）でも間違いではないと思いますが、countUsers()（ユーザーを数える）のほうが簡潔に書けます。

作成系

• 
  create
  
  
  • 無から有を作る場合に使います。
  • 例: createUser() - 新規ユーザーを作成
• 
  build
  
  
  • 材料を組み立てて「構築する」場合に使います。
  • 例: buildQuery() - 引数として渡した条件等からSQL文字列を構築（queryという名前はSQLなのかURLのクエリストリングなのか曖昧な場合があるので、文脈上明らかな場合にのみ使ってください）

更新系

• 
  update
  
  
  • 特定のデータを更新する場合に使います。
  • 例: updateUserName() - ユーザー名を更新する
  • ↑ただの例なので、「関数じゃなくてメソッドにしろよ」というツッコミは勘弁してください…
• 
  save
  
  
  • データを永続化する場合に使います。
  • 例: saveUser() - ユーザー情報を（DB等に）永続化する

削除系

• 
  delete
  
  
  • データやリソースの削除に使います。
  • 例: deleteUser() - ユーザー情報を削除する
  • 一部の言語（JavaScriptやC++等）ではdeleteが予約語として存在しているので、delete()という関数名は作れません。
• 
  remove
  
  
  • データやリソースの削除に使います。
  • 例: removeUser() - ユーザー情報を削除する
  • 
    deleteとの違いは以下のとおりです。
    
    
    • 
      delete - 削除する。削除されたものはもはや存在しない。
    • 
      remove - （一覧から）取り除く。取り除いたものはまだどこかに存在しているかもしれない。
    • 例えば、jQuery.remove()やJavaのList.remove()は削除された要素を返します。つまり、完全に消滅したわけではなく、単にDOMツリーやリスト内から「取り除かれた」だけで、まだメモリ上のどこかに存在しています。これらのメソッド名がdelete()だったらネイティブからみて違和感を覚えるかもしれません。

動詞を使わない関数名

上で、このように書きました。

関数は「何かをするもの」なので、原則として動詞を頭につけてください。

原則には例外がつきもので、時には動詞を使わない関数名やメソッド名をつけることがあります。

JavaやJavaScriptの文字列にあるcharAt()やindexOf()などがその例です。

これらは関数名と引数とつなげてフレーズにしているという特殊な命名方法です。

• 
  charAt(0) は "char(acter) at 0" 、つまり「0番めの文字」
• 
  indexOf('a') は "index of 'a'" 、つまり「'a'のインデックス」

こういう名前を自然に作れるようになると玄人っぽいですが、一歩間違えると意味が伝わらなかったり誤解されたりするのでご利用は計画的に。

また、単なるプロパティや属性のgetter/setterでもわざわざ動詞をつけることはないかもしれません。

class Rectangle { 
    private width_: number; 
    private height_: number; 
 
    constructor(width: number, height: number) { 
        this.width_ = width; 
        this.height_ = height; 
    } 
    get width(): number { 
        return this.width_; 
    } 
    get height(): number { 
        return this.height_; 
    } 
} 

HowではなくWhatを

関数を使う側にとって必要なのは、その関数が「何をするか（What）」であって、「どうやってするか（How）」ではありません。言い換えると、内部実装のことまで関数名に含める必要はありません。

むしろ内部実装まで関数名に含めてしまうと抽象度が下がり、将来実装が変わった時に誤解されてバグの元になります。

// ユーザー情報 
class User { 
    // ... 
} 
 
/** 
 * 悪い例 
 * ユーザー情報をMySQLから取得するけど、使う側にとっては内部実装（どこから取得するか）はどうでもいい 
 * 将来取得先を変えた場合（PostgreSQLとかHBaseとか、あるいはマイクロサービス化して外部APIから取得とか）に整合性が取れなくなる 
 */ 
export function findUserFromMySQL(id: number): User | null { 
    // ... 
} 
 
/** 
 * いい例 
 * ユーザー情報を取得 
 * 内部実装の情報は関数名には含めない 
 */ 
export function findUser(id: number): User | null { 
    // たとえばMySQLから取得 
} 

もちろん、呼び出し元で内部実装を知っている必要がある場合は別です。

その場合でも、ほとんどが内部的に使われる関数に限られるので、exportする必要はまずないはずです。

// 内部実装を関数名に含める必要がある例 
class User { 
    // ... 
} 
 
/** 
 * ユーザー情報を取得 
 * キャッシュにあればそれを取得し、なければRDBから取得 
 */ 
export function findUser(id: number): User | null { 
    const user = findUserFromCache(id); 
    if (user !== null) { 
        return user; // キャッシュにあった 
    } 
 
    // キャッシュにないのでRDBから取得 
    return findUserFromRdb(id); 
} 
 
/** 
 * キャッシュからユーザー情報を取得 
 * 内部実装の情報（どこから取得しているか）が呼び出し側で必要なので関数名に含めている 
 */ 
function findUserFromCache(id: number): User | null { 
    // たとえばmemcachedから取得 
} 
 
/** 
 * RDBからユーザー情報を取得 
 * これも内部実装の情報が呼び出し側で必要なので関数名に含めている 
 */ 
function findUserFromRdb(id: number): User | null { 
    // たとえばMySQLから取得 
} 

条件式について

読みにくいコードにはまず間違いなく複雑な条件式があります。

プログラムの規模が大きくなると複雑な条件が出てくるのは避けられませんが、条件が複雑ということと条件式が複雑ということはイコールではありません。

条件が複雑でもわかりやすい条件式を書くことはできます。

条件式は関数化して「目的」がわかるようにする

例えば、以下のような条件式は何をしたいのか即断できません。

if (year % 4 === 0 && year % 100 !== 0 || year % 400 === 0) { // 何を判定したい？？ 
    // 何か処理 
} 

これはyearが閏年かどうかを判定しているのですが、=== !== && ||が入り乱れているので非常にわかりづらい条件式です。

ここで重要なのは、本来の目的はyearが閏年かを判定したいのであって、「yearが4で割り切れるが100で割り切れない、又は、400で割り切れるか」を判定したいのではない、ということです。

「4で割り切れるが…」は手段です。手段を条件式に書くと読みづらいコードになります。

ifの中には手段（どうやって判定しているか）ではなく、目的（何を判定したいのか）を書きましょう。

if (isLeapYear(year)) { // 閏年かどうかを判定したいとわかる 
    // 何か処理 
} 
 
function isLeapYear(year: number): boolean { 
    return year % 4 === 0 && year % 100 !== 0 || year % 400 === 0; 
} 

単に関数化しただけですが、単体テストもやりやすくなりますし、格段に読みやすくなったとおもいませんか？

複雑な条件は分解する

上のisLeapYear()は関数化して全体の見通しはよくなったものの、関数の中身はまだ読みづらいままです。

しかし、条件部分が関数化してあれば読みやすくするのも簡単です。

読みやすくするために、条件式を分解しましょう。

分解のコツは、真偽が確定したらその時点でreturnすることです。

function isLeapYear(year: number): boolean { 
    // 元の条件式: year % 4 === 0 && year % 100 !== 0 || year % 400 === 0; 
 
    if (year % 400 === 0) { 
        // 400で割り切れる年は閏年 
        return true; 
    } 
    if (year % 100 === 0) { 
        // 400で割り切れず、100で割り切れる年は平年 
        return false; 
    } 
    if (year % 4 === 0) { 
        // 400でも100でも割り切れず、4で割り切れる年は閏年 
        return true; 
    } 
 
    // それ以外は平年 
    return false; 
} 

これでだいぶわかりやすく、ロジックにバグがあってもすぐに気付けるレベルになりました。

関数

そのうち書きます。

コメント

そのうち書きます。

参考

• 
  リーダブルコード - 読みやすいコードを書くための必読書です。