【読書メモ】リーダブルコード

技術 読書

リーダブルコードを読んで

印象に残った部分だけかいつまんでまとめました。

汎用的な言葉は避ける

stopとかsizeとか。 短いループでの、後の再利用のない''temp''などは逆に分かりやすい。 Get()は何をしているのかよくわからない。Fetch()とかDownload()とかだとわかりやすい。

ループイテレータ

i,j,kなどでもいいが、users_i,members_iとか、何に対するイテレータか目印をつけると良い 。

単位のあるものは、変数名に記入

double delay→double delay_msや、int dataSize→dataSize_mbなど。 ただし、明示的にしなくても問題ない場合は短く記入。

必要な単語を加え、不要な単語は投げ捨てる。

名前の長さや言葉はさまざまな観点から選択する必要がある。 たとえば、スコープの短い変数（すぐそこで使って、その後は捨てるようなもの）は、nとかtempでよい。 ConvertToString()はToString()でも十分伝わる。逆にプロジェクト固有の単語は省略したり削ると危ない。

配列の中で、始端と終端の範囲を言葉によって分ける

array[0] array[1] array[2] array[3] array[4]
a b c d e 
print integer_range(start=2,stop=4);

これが印字するのは[c,d]?[c,d,e]? 包含的な範囲を示すなら、(終端を範囲に含むのなら)firstとlastを使う。

set.Printleys(first="a",last="d");

逆に、排他的(終端を範囲に含めない)選択では、beginとendを使う。

set.Printkeys(begin="a",end="e");

「59分59秒」を避ける

たとえば、日付指定でデータ内のイベントをすべて印刷したいとする。

PrintEventsInRange("OCT 16 12:00am","OCT 17 12:00am"); 

PrintEventsInRange("OCT 16 12:00am","OCT 16 11:59:59:9999pm");

どちらがわかりやすいかは明確。

「何がtrueで何がfalse」なのかを明確に

たとえば、次のような変数。

bool read_password = true;

パスワードをこれから読み取る必要があるのか？ パスワードをすでに読み取っているのか？ がわからない。 ブール値の変数では、is,has,ca,shouldを接頭語として使うと分かりやすい。 need_passwordとか、user_is_authenticatedなどでもよい。 あとは、なるべく肯定形で書く。

bool disable_ssl = false; 
bool use_ssl = true;

二重否定は混乱の元。

親しみやすさと軽量さは両立させる

GetやSizeなどの、標準ライブラリでサポートされている名前を含めて使う時は注意が必要。 関数の利用者は、その名前の親しみやすさから気軽に使ってしまう。

似ているコードは似せて見せる

シルエットを意識するとよい。 4つの引数を持つTcpConnectionSimulatorクラスを使って、3つのインスタンスを作るとする。

1.接続速度(KBps) 2.平均遅延時間(msec) 3.遅延時間(msec) 4.パケットロス率(percent,%)

public class PerformanceTester 
{ 
public statc final TcpConnectionSimulator wifi = new TcpConnectionSimulator( 
500, /*Kbps*/ 
80, /*msec latency*/ 
200,/*jitter*/ 
1/*packet loss %*/); 

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

public static final TcpConnectionSimulator cell = new TcpConnectionSimulator( 
100, 
400, 
250, 
5); 
}

「コーディングの方針として、80文字折り返し」というプロジェクトの場合、黄色の部分が他と違うため、 どうしても見にくくなってしまう。

たとえば、こうしてみる
public class PerformanceTester 
{ 
public statc final TcpConnectionSimulator wifi =  
new TcpConnectionSimulator( 
500, /*Kbps*/ 
80, /*msec latency*/ 
200,/*jitter*/ 
1/*packet loss %*/); 

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

public static final TcpConnectionSimulator cell =  
new TcpConnectionSimulator( 
100, 
400, 
250, 
5); 
}

でも、縦に長くなって見づらくなる欠点もある。 こういうときは、簡潔に書くことを念頭に置くべき。

public class PerformanceTester 
{ 
//TcpConnectionSimulator(throughput, latency, jitter, packet_loss) 
//                                       [kbps]        [ms]    [ms]   [percent] 

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

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

public static final TcpConnectionSimulator cell =  
new TcpConnectionSimulator(100, 400, 250, 5); 
}

縦の線をまっすぐにする

CheckFullName("doug Adams","Mr.Doglas Adams",""); 
CheckFullName("Jake Brown","Mr. Jake Brown III",""); 
CheckFullName("No such Guy","","no match found"); 
CheckFullName("John","","more than one result");

わかりにくい。

CheckFullName("doug Adams" ,"Mr.Doglas Adams"   ,""); 
CheckFullName("Jake Brown"  ,"Mr. Jake Brown III"  ,""); 
CheckFullName("No such Guy" ,""                            ,"no match found"); 
CheckFullName("John"             ,""                           ,"more than one result");

わかりやすい・・・？ 要検討。

宣言をブロックにまとめる

これは、関数の宣言などで、void型はvoid型にまとめるとか、機能ごとにまとめるといった宣言方法をとったほうが良い。

コードを段落に分割する

具体的には、何か１つのアクションをするごとに1行開けるといった方法がよく用いられる。

下手にコメントを入れるよりも、関数名をわかりやすくしたほうがよい。

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

↓

void ReleaseRegistryHundle(Registry* key);

このほうが、分かりやすいし、混乱もない。

「監督のコメンタリー」を入れる

コメントは自分の考え、コードの悪い点、具体的な例を入力するとよい。後で聞かれそうなことを書く。

//このデータではハッシュテーブルよりもバイナリツリーのほうが40%早かった。

とか。

//このクラスは汚くなってきている。 
//サブクラスを作成して整理したほうがいいだろう。

とか。 コードの欠陥については、

記法 
意味 
TODO: 
あとで手をつける 
FIXME: 
既知の不具合がある 
HACK: 
あまりキレイじゃない解決策 
XXX: 
大きな問題アリ

といった感じでコメントを入れておいてもいい。プロジェクトによるかも。

定数にもコメントをつけるといい

なぜその値を持っているかの背景が存在しているなら、記入する価値がある。

NUM_THREADS = 8 #値は「<=2 * num_processors」

で十分。 逆に、

PI = 3.141592653589793 #PIは円周率

このコメントはいらない。

読み手のことを考えて、質問されそうなことにコメントをつける

コメントとは、プロジェクトを熟知していない人のためにある。（もちろん、1週間後の自分に対しても。）

//メールを送信する外部サービスを呼び出している（1分でタイムアウト） 
void Sendmail(string to,string subject,string body); 
//実行時間はO(タグの数*タグの深さの平均)なので、ネストの深さに注意 
def FixBrokenHTML(html):…

ファイルやクラスには全体像のコメントを書く

コードブロックにコメントをつけて概要をまとめる。関数単位でまとめられるならそっちのほうがいい。

入出力のコーナーケースに実例を使う

//'v'の「要素<pivot」が「要素>=pivot」の前に来るように配置しなおす。 
//それから、「v[i]<pivot」になる最大の'i'を返す（なければ-1を返す) 
int Partition(vector(int)* v,int pivot);

これだけでは、視覚化がしづらい。1文加えると良い

//実例：Partition([8 5 9 8 2], 8)の結果は[5 2| 8 9 8]で、1を返す。

よくわからない引数を説明するときのみ、名前付き引数コメントで、何を処理しているか明確化する。

Connect(10,false);

数値とブール値が渡されているが、何なのかよくわからない。

Connect(/* timeout_ms = */ 10, /* use_encryption = */ false);

if文の条件には基本的に肯定形を使う。「trueのとき」のことを念頭に置いて条件分岐を読み進めるのは気が散る。 一方、否定形でも短くて注意を払わせたいときもある。

if not file: 
#error logを出力 
else: 
…

三項演算子 「(条件)? a : b」 は、読み手が分かりやすくなるとき以外は使わない

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

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

三項演算子が分かりやすい数少ないパターン。基本的にはif/elseでよい。 do/whileをなるべく避ける。他のコードと違い、条件が下にあるため、読む流れが変わってしまう。 ほとんどの場合、while(条件){}で書き直せる。

あと、次に示す処理でどうなるのかの判断が難しい。

do{ 
continue; 
} 
while(false); 
//ちなみに、これは1回のみ実行される

ネストは浅く保ち、精神的なスタックを減らす

if(user_result == true) 
{ 
    if(permission_result != true) 
    { 
        reply.WriteErrors("error reading permissions"); 
        reply.Done(); 
        return; 
    } 
} 
reply.WriteErrors(""); 
reply.Done();

このネストを削減するにはどうすればいいだろう？ 解決策としては、早めにreturnをしてネストを削除する。

if(user_result != true) 
{ 
    reply.WriteErrors(user_result); 
    reply.Done(); 
    return; 
} 
if(permission_result != true) 
{ 
    repry.WriteErrors(permission_result); 
    reply.Done(); 
    return; 
} 
reply.WriteErrors(""); 
reply.Done();

for文などで、容易にreturnを使えない場合はcontinueを使う

巨大な式は飲み込みやすい大きさに分割する。 分割する際は、説明変数を用いる。

if line.split(':')[0].strip() == "root"; 
…

↓

username = line.sprit(':')[0].strip() 
if username == "root"; 
…

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

if(!(a && !b))

↓

if(!a || b)

変数のスコープscopeを縮めることは、いつだっていい事だ

というか、人間はメモリ容量が少ない。

ブロックスコープをうまく使う

変数は使う直前で宣言する。そして、何度も同じ変数に代入するのはやめる。 以下はC++のコード。

if(...) 
    { 
    int x = 1; 
    } 
x++; 
#コンパイルエラー

逆にJavaScriptなどでは関数全体にこぼれ出たりするので注意。 定義の位置を下げる。 これはレガシープログラマがやりがちなことと被る。 （昔のCでは変数の宣言が先頭にある必要があったので。） ふつうは使う直前でのみ宣言して、使ったら早々に放棄する。 C#などではガベージコレクションが管理をしてくれるので放置。

変数は1度だけ書き込む

できれば1度だけ書き込み、永久に変更されない変数が使いやすい。

static const int NUM_THREADS = 10;

この変数に対して多くのことを考える必要はない。 では、どうすれば1度だけ書き込むように構成するのか？ 少しずつ何度もコードを改良する。 いっぺんに変数の動きを減らすのは難しい。

まず、パッと見で不要な変数を削除する（どうでもいいbool型の変数とか）

次に、whileやfor文中で何度も変更されている変数に着目する。 for文中でさらにif文がネストされていて、 書き換えて判定→書き換えて判定→書き換えて判定... といったものを繰り返しているのであれば、while(true)やfor(var i = 0;true;i++)などで ループ内の文章を減らすことで可読性が向上する。

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

機能を分離してメソッドを作るときの一番の指標となる。 1.関数やコードブロックを見て「このコードの高レベルの目標は何か？」を自問する 　→たとえば、「与えられたデータから一致する人を割り出す」など 2.コードの各行に対して、「高レベルの目標に直接効果があるのか、あるいは、無関係の下位問題を解決しているのか」について自問する 　→たとえば、「与えられたデータがバイナリであった場合、テキストに変換する」など 3.無関係の下位問題を解決しているコードが相当量あれば、それらを抽出して別の関数にまとめる

なぜこのようなことをするのか？理由は二つある コードが独立しているため、改善やテストがしやすい 他のプロジェクトで再利用しやすい

この手法は多くのプログラマが実行できていない。 一番の理由は、「やりすぎ」の境界線がよくわかってないから。 関数を追加すると、若干ではあるが確実に読みにくさのコストが発生する。 そのコストを上回るようなコスト削減を見込めないのであれば、それは追加すべき関数じゃない。

一度にひとつのことを

これは無関係の下位問題を抽出するときにも効果を発揮する。 複数の下位問題を抽出しているのであれば、機能単位でさらに分割してしまえばよい。 そして、プログラムのデフラグをする。 １つのプログラムの中でAをやってBをやって、またAに飛んで・・・・というのはわかりにくい。 A→B→C→Dという流れを作っておく。 機能を分割するにはどうすればうまくいく？

言葉でプログラムを説明してみる

1行がそのまま関数になる。

コードは小さく保つこと

プログラマが思っているより、運用はめんどくさい。

書く前にライブラリを調べろ

いま実装しようとしている機能の大半は必要ない。 なにか（主に、ログなど）を出力させるプログラムは、ターミナルやコマンドプロンプトに丸投げしよう。