古のコーディング規約
古のコーディング規約
古のコーディング規約がすべて問題なわけではありませんし、多くは有用なのですが、今その規約を守るとかえってバグの原因になったり、可読性の低下を招くものもあります。若いプログラマーには理由がわからないものも多いと思います。
基本はそのような規約を撤廃した方がいいのですが、どうしても守らないといけない場合、関数の抽出で対処できることもあります。その他利を含めて解説します。
return は一つまで
「関数一つにつき return は一つまで」というルールは昔はよく見ました。調べると、「出口がたくさんあるとわかりにくいから」という意見を見かけます。しかし、私は違うんじゃないかなあと思います。
C# のコードですが、ファイルを開いて読み込んで解析して閉じるっていうやつです。
var stream = File.Open(...);
try {
var header = stream.ReadLine()
if (!header.Contains("...")) {
return 1;
}
// ...
return 0;
} finally {
stream.Close();
}
(C# では using があってそっちを使った方がいいんですが、説明のしやすさのため try を利用してます)
解析中にエラーになろうがストリームは必ず閉じなければなりません。
ところが C 言語はこの finally に当たるものがないんです。(ないですよね?) ありがちなのが、次のコードのようにエラーの時の fclose() を忘れてしまうものです。
FILE *fp = fopen(...);
if (...) {
// エラー
return 1;
}
fclose(fp);
return 0;
これではエラーの時にファイルが閉じられていません。後始末を忘れないようにしないといけません。
FILE *fp = fopen();
if (...) {
// エラー
fclose(fp); // return の前に必ず
return 1;
}
fclose(fp);
return 0;
忘れんなよ? って話ですが、結構忘れます。特に他人のコードを後から修正するときによく忘れます。
忘れがちなので、出口を一つにしましょうというやつです。
FILE *fp = fopen(...);
int result = 0;
if (...) {
result = 1
} else {
// ....
}
fclose(fp);
return result;
今では finally (もしくはリソースの自動クローズ) が使える言語がほとんどなので、この規約を守る理由はありません。C 言語で書いていた当時はこの規約を守るようにしていましたが、今は全然守ってません。finally 万歳!
C 言語でも、処理部分を関数化することで後始末忘れを回避することができます。
FILE *fp = fopen();
int result = process(fp);
fclose(fp);
return result;
int process(FILE *fp)
{
if (...) {
return 1;
}
// ...
return 0;
}
if 文のネスト
関数で return を一つだけとすると、どうしても if 文がネストします。ループも絡んでくるとかなり厄介になります。
int result = 0;
for (...) {
if (...) {
result = 1;
break;
}
}
if (result != 0) {
if (...) {
if (...) {
result = 2;
} else {
result = 3;
}
}
}
return result;
悪役扱いされる goto 文を使った方がはるかにましになります。
int result = 0;
for (...) {
if (...) {
result = 1;
goto FINAL;
}
}
if (...) {
if (...) {
result = 2;
goto FINAL;
}
result = 3;
goto FINAL;
}
FINAL:
return result;
細かく関数に分けていく方法もあります。
int result = 0;
result = process1(...);
if (result != 0) {
result = process2(...);
}
return 0;
int process1(...) {
int result = 0;
for (...) {
result = process11(...);
if (result != 0) break;
}
return result;
}
// 以下省略
いずれにせよ、return 文は一つまでというルールを守るより、早期 return を活用した方が可読性は上がると思います。
変数は関数の先頭で宣言すること
昔の C 言語はそもそも変数の宣言は関数の先頭でしかできませんでした。とはいえ、これは規約じゃなくて制限です。
私が最初に触ったプログラム言語は N-BASIC でして。変数の名前は先頭二文字までしか認識しない仕様です。そういう制限がなくても昔は変数名は省略していることが多かったように思います。変数名からは何を意味しているのかが分かりにくかったです。
ソースコードも印刷されたものを読むことが多かったです。変数表があればそれを片手にコードを読んでました。変数表の代わりに、関数の先頭にコメント込みで宣言しといた方が分かりやすかったです。
今は統合開発環境とかでコードを読むので、変数の型や使っている場所がすぐにわかります。先頭にまとめて宣言する理由は薄くなってます。その代わり、変数を使っているスコープが短い方がいいという風潮になってます。
1,000 行の関数に変数が 20 個あったとして、先頭で宣言されていると常に 20 個の変数を意識しなければなりません。しかし、1,000 行のうち 20 個の変数を同時に利用している部分などないはずです。その部分で必要な変数のみを認識していればいいわけです。
const a = 10;
let b;
let c;
// 実際には 10 行くらいの処理
// a と b しか使ってないけど c のことも考えないといけない
if (a % 2) {
b = a * 2;
} else {
b = a;
}
// 実際には 10 行くらいの処理
// b と c しか使ってないけど a のことも考えないといけない
c = '[' + b + ']'
// 続きの 10 行くらいの処理
次のように処理ごとに関数に分けることをお勧めします。自然と変数の宣言は先頭付近に集まります。そのうえ、一つ一つの変数のスコープが短くなります。
const a = 10;
const b = process1(a);
const c = process2(b);
余談ですが、関数の抽出をうまく使うと、多くの変数が不変になります。値が変わらないため、可読性も向上します。
抽出した関数で早期 return を使えば、一時変数が不要になり、変数宣言すら不要になることもあります。
function process1(a) {
if (a % 2) return a * 2;
return a;
}
// 以下省略
変数の宣言で必ず初期化すること
多くの言語は変数は宣言文だけで規定値に初期化されます。
例外的なのが C 言語 (C++ も) です。
int i;
printf("%d", i);
変数 i は初期化されていませんので、i が割り当てられたメモリの値になります。このコードは表示しているだけなので害はありませんが、通常はバグの原因になります。
次の例では SEASON_WINTER の時に i が未初期化になります。
void process(int season) {
int i;
if (season == SEASON_SPRINT) {
i = 1;
} else if (season == SEASON_SUMMER) {
i = 2;
} else if (season == SEASON_AUTOM) {
i = 3;
}
// i を使った処理
}
何らかのバグが発生しますが、値が不定ということで毎回症状が変わるバグになります。このようなバグは原因の特定が難しくなります。
バグはバグとしてしょうがないけど、症状がなるべく変わらないように、初期化漏れになるくらいなら初期値を設定しておいた方がまだ症状固定のバグになることが多いということでこの規約があるのだと思います。
今どきはコンパイラーや静的解析ツールが未初期化の変数の利用を検出して警告ないしはエラーを出力します。
次のコードは会員か非会員かを表示しますが、非会員の実装を忘れてしまいました。
string s = "";
if (isMember)
{
s = "会員";
}
Console.WriteLine(s);
宣言時に初期化しなければ、未初期化の変数を利用したとしてエラーになります。初期化を忘れていることにすぐ気が付きます。
string s;
if (isMember)
{
s = "会員";
}
Console.WriteLine(s); // Error CS0165 : 未割り当てのローカル変数 's' が使用されました
なお、こういう場合も関数に分けることをお勧めします。そうすると、宣言時に初期化するルールも守れます。
var s = GetMemberDisplayText(isMember);
Console.WriteLine(s);
string GetMemberDisplayText(bool isMember)
{
if (isMember) return "会員";
return "非会員";
}
JavaScript の var
以前の JavaScript (ES5) までは変数の宣言は var のみでした。
var text = 'global';
function hello() {
console.log(text); // undefined ... (1)
{
var text = 'local'; // ... (2)
console.log(text); // 'local' ... (3)
}
}
hello();
このコードを実行すると、(1) は undefined と表示されます。これは (2) の var text の変数宣言が関数の先頭に巻き上げられるからです。
実際に実行されるコードのイメージはこんな感じです。
var text = 'global';
function hello() {
var text;
console.log(text);
{
text = 'local';
console.log(text);
}
}
hello();
このようなバグを避けるために、JavaScript では var 宣言は関数の先頭に書くべきでした。ただし、静的検査ツールにかければ問題を検出してくれますし、今は巻き上げを起こさない const や let 宣言があるので不要になりました。
最後に
コーディング規約はルールと同時にその意義を学びましょう。意義は時代が変わってもほとんど変わりませんが、プログラム言語やツールの進化でルールは変わることがあります。
定期的に見直すことをお勧めします。
Discussion