ブログトップ 記事一覧 ログイン 無料ブログ開設

Strategic Choice

2014-09-18

[]制御フロー:ネストを浅くする

どういうこと?

ネストの深いコードは理解しにくいので、できるだけネストを浅く保ちます。

どうして?

ネストが深くなると、読み手は「精神的スタック」に条件をプッシュしなければなりません。閉じ括弧(})を見てスタックからポップしようと思っても、その条件が何だったのかうまく思い出せません。

以下のコードは、比較的単純な実装です。それでも、コードを読んでいる自分を顧みると、コードを読みながら、自分がどの条件ブロックにいるのかを常に確認しているのがわかります。

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

最初の閉じ括弧を見つけたとき、「permission_result != SUCCESS が終わるんだな。次は、permission_result == SUCCESS か。でも、これはまだuser_result ==SUCCESS の内部だな」と考えます。

つまり、「user_result」と「permission_result」の値を常に覚えておかなければいけないということです。それに、ifブロックが終了するたびに、覚えておいた値を反対にしなければいけません。

さらに、このコードはSUCCESSとSUCCESSの否定が交互に登場しているので、よりタチが悪いものになっています。

どうすれば?

ネストが深くなるメカニズムを理解し、そうならないように実装します。

上述のサンプルコードは、実は修正が重ねられた後のコードで、最初は以下のような単純なコードでした。

if (user_result == SUCCESS) {
	reply.WriteErrors("");
} else {
	reply.WriteErrors(user_result);
}
reply.Done();

このコードなら完璧に理解できます。エラー文字列を決めてから、reply を終了しているだけです。

しかし、ここに新しいコードが追加されていきます。

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

この変更は妥当です。最も簡単にコードを挿入できる場所がここでした。これを書いたプログラマにとって、新しく追加するコードは新鮮で「関心を引く」コードです。そして、この変更の「差分」は何も間違っていません。簡潔な変更です。

しかし、あとで誰かがこのコードを見たときには、こうした文脈はすべて失われてしまいます。こうやって、最初に見たコードが出来上がったというわけです。

大切なのは、「変更するときにコードを新鮮な目で見るようにする」ことです。一歩下がって全体を見るのです。この観点で、例のコードを改善します。ネストを削除するには「失敗ケース」をできるだけ早めに関数から返します。

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

これでネストの深さが2レベルから1レベルになりました。精神的なスタックから「ポップ」する必要がなくなりました。すべてのif ブロックは「return」で終わっています。

ただ、早めに「return」する技法はいつでも使えるわけではありません。例えば、ループ内部でネストしたコードを見てみます。

for (int i = 0; i < results.size(); i++) {
	if (results[i] != NULL) {
		non_null_count++;

		if (results[i]->name != "") {
			cout << "Considering candidate..." << endl;
			...
		}
	}
}

早めに「return」するのと同じようなことをループ内部で行うには、「continue」を使用します。

for (int i = 0; i < results.size(); i++) {
	if (results[i] == NULL) continue;
	non_null_count++;

	if (results[i]->name == "") continue;
	cout << "Considering candidate..." << endl;
	...
}

「if (...) return;」が関数のガード節になるのと同じように、「if (...) continue;」文がループのガード節になっています。

ただ、「continue」はわかりにくくなることが多くあります。「goto」と同じように、ループを行ったり来たりするからです。ただ、この例でのループは1つだけなので、continue が「この項目は飛ばす」意味だとすぐにわかります。

2014-09-17

[]制御フロー:gotoを避ける

どういうこと?

言語でサポートされていたとしても、gotoは使用しません。

どうして?

gotoを使うと、コードについていくのが難しくなり、たちまち手に負えないようになります。gotoの飛び先が複数になり、経路が交差していたら、もう解読不能です。特に、gotoで「上に飛ぶ」のは本物のスパゲティコードになります。

goto は、現状ほぼ必要ありません。同じことをする方法が、他にあるからです。

どうすれば?

gotoを使用するのは控え、普通のループに置き換えます。

しかし、今でもC言語のさまざまなプロジェクトでgotoは使われています。例えば、Linux カーネルがそうです。分析すると、確かに、gotoが許される場面もあります。

if (p == NULL) goto exit;
...
exit:
fclose(file1);
fclose(file2);
...
return;

gotoでも、関数の最下部に置いたexitと一緒に使う場合、単純で害がありません。

2014-09-16

[]制御フロー:関数から早く返す

どういうこと?

「ガード節*1」を使って、関数から早く抜け出します。

どうして?

関数から早く返すことは、読みやすさを向上させます。

まれに、関数で複数のreturn文を使ってはいけないと思っている人がいます。「クリーンアップコード*2」のため、関数の出口を1つにしなければならない、という意図です。

しかし、これは誤りです。たとえば、以下の実装は、「ガード節」を使わずに実装すると、非常に不自然になります。

public boolean Contains(String str, String substr) {
	if (str == null || substr == null) {
		return false;
	}

	if (substr.equals("")) {
		return true;
	}
	// ...
}

どうすれば?

ガード節を積極的に利用します。

「クリーンアップコード」については、現代の言語では、それを確実に実行する仕組みが、より洗練された形で提供されています。

言語クリーンアップコードのイディオム
C++デストラクタ
Javatry ... finally, try-with-resource
Pythontry ... finally, with
C#using

*1:関数の上部で、単純な条件を先に処理する節です。

*2:リソースのクローズなどのことです。

2014-09-12

[]制御フロー:do/whileループを避ける

どういうこと?

読みやすさを考慮し、do/whileループの使用は極力避けるようにします。

どうして?

言語によっては、「do {式} while (条件)」ループを書くことができます。式の部分は最低1回は実行されるということです。しかし、これは読む順番からすると、不自然なコードです。

以下に例を挙げます。

// 'name' に合致するものを'node' のリストから探索する。
// 'max_length' を超えたノードは考えない。
public boolean ListHasNode(Node node, String name, int max_length) {
	do {
		if (node.name().equals(name)){
			return true;
		}
		node = node.next();
	} while (node != null && --max_length > 0);

	return false;
}

「do/while」ループが変わっているのは、コードブロックを再実行する条件が下にあることです。「if文」「while文」「for文」の条件は、コードブロックの上にあります。コードは上から下に読んでいくので、「do/while」は少し不自然です。コードを2回読むことになってしまいます。

どうすれば?

do/whileループは、whileループで書き直します。whileループにすれば、コードブロックを読む前に繰り返しの条件がわかるので、読みやすくなります。

上述例の場合、以下のように変更できます。

public boolean ListHasNode(Node node, String name, int max_length) {
	while (node != null && max_length-- > 0) {
		if (node.name().equals(name)) { 
			return true;
		}
		node = node.next();
	}
	return false;
}

こうすれば、max_length が0 でも、node がnull でも動くようになります。

do/while を避ける理由は他にもあります。それは、内部にあるcontinue文がまぎらわしいからです。例えば、以下のコードは、何をしているのか明確ではありません。

do {
	continue;
} while (false);

ループは永久に続くのか、それとも1 回だけなのか、多くのプログラマは、立ち止まって考えることになります*1

ただし、コードを重複させてまで、do/whileを削除することはありません。

// 擬似do/while(やってはダメ!)
/* 本体 */
while (condition) {
	/* 本体(2度め) */
}

*1:ループは1 回だけになります。

2014-09-11

[]制御フロー:三項演算子を避ける

どういうこと?

条件式には、基本的に「if/else」を使います。三項演算子は、それによって簡潔になるときにだけ使います。

どうして?

「条件 ? a : b」という条件式が書ける言語があります。これは、「if (条件) { a } else { b }」 を簡潔に書いたものです。

読みやすさの点から言うと、どちらが良いかは議論の余地があります。支持者は、複数行が1行にまとまるのでいいと主張します。反対者は、読みにくいしデバッガでステップ実行するのが難しいと主張します。

三項演算子が読みやすくて簡潔な例を挙げてみます。

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

これは、単純な2つの値から1つを選ぶようなものでありません。それなのにこんなコードを書くというのは、「何でも1行に収めたい」と思っているからです。

どうすれば?

「行数」を短くするよりも、「他の人が理解するのにかかる時間」を短くすることを重視します。

上述例であれば、きちんとif/else 文を使えば、コードがより自然になります。

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