はじめに
この記事を読む前に、よかったら以下も読んでください。
この記事の説明
「これはアンチパターンである」と、ハッキリとは言えないのですが、最近たくさんのソースを読んでいて「これ、困るなあ」「こうなっていると嬉しいな」と思ったものがあるので記します。
コード例は JavaScript のような、架空の言語で示します。if 文を持つ言語なら、だいたい当てはまるのではないかと思います。
※ ポイント以外、コードは省略しますのでサンプルコードは動きません。
（１）式の中に処理がある
例えば、以下のようなコードです。
if (registerUserProperty(obj) == true) {
console.log("登録処理は成功しました");
}
...と、こうではなく...
let successful = registerUserProperty(obj);
if (successful) {
console.log("登録処理は成功しました");
}
...というようにしてほしい。
理由としては、以下のようなものです。
- 「処理」と「判定」は別のもの。別々に書いてほしい。「このあたりで処理を呼んでるはずなんだけど...。妙だな、見当たらない。あっ！」となった。
- 関数の戻り値は、どうせデバッグ時に調べたくなる。「
trueかfalseを期待していたら実はnullだった」などということが、言語によってはしばしば起きる。 - 下記の算術関数と違って、判定内容を明確に示す識別子（関数名）になっていない。
以下はOK。処理ではなく、算術関数だから。意図が明確で読みやすい。
if (isNumeric(value)) { ...
if (productCode.startsWith("ABC-")) { ...
あと while の中の式とかも別にいい。以下のようなやつね。
while (reader.Read() == true)
{
Console.WriteLine(reader["NAME"].ToString());
}
（２）式が長い
下記のようなコードではなく。
if (user.profile.birthday.toAge().between(20, 30) && user.options.class.contains(CLASS_PREMIER)) {
// 処理 ...
}
...ではなく...
let isTargetGen = user.profile.birthday.toAge().between(20, 30);
let isPremier = user.options.class.contains(CLASS_PREMIER);
if (isTargetGen && isPremier) {
// 処理 ...
}
...としてほしい。これなら読んですぐ「ああ『ターゲット世代かつプレミア会員かどうか』を判定しているのね」とかわかる。
（VB とかの And と違って、&& とかはシンタックスハイライトしないエディタ多いこともあって、長くなると読みづらい）
上記のようなケースは、長くて読みづらいとはいえ仕様とコードが一致しているからまだ良いのだけれど、複数の値を確認して初めて一つの状態が決まるようなケース。例えば、製造ライン番号と時間帯から作っている製品がなんなのかを判定するようなケース。
if (lineCode == "A20" && timeRange <= 1200) {
// 処理 ...
}
こういったケースでは、判定結果の製品名と判定文の式がまったく結びつかないので、上記のような説明変数に入れるか、下記のように算術関数を作ってほしい。
function isSandwich() {
return (lineCode == "A20" && timeRange <= 1200);
}
if (isSandwich()) {
// 処理 ...
}
これなら「ああ、製品がサンドイッチかどうかを判定しているのね」とかわかる。
まとめ
短く書く要件があるような場合（ワンライナーだったり高速化のためだったりメモリの節約のためだったり）でなければ、保守性（読み易さ）を優先してほしいなあ。
メインのロジックだけで良いです。ビジネスロジック・仕様が読み取れるようになっていて欲しいのです。なんのための判定なのか、判定結果はどのような意味を持つのか。そういったことがわかると、ドキュメントや要件との照合もしやすい。
おわりに
どちらも「あ。やってしまったことある」でした。ソースコードを書く立ち場と読む立ち場で、変わるのだと思います。