皆さんに質問なのですが、「よりよいソースコード」、
すなわち、「可読性・保守性に優れたソースコード」とは、
どのようなソースコードだと思いますか?
私には、自分なりのプログラミングスタイルというものがあり、
それが自分にとっても万人にとっても見やすいと思っています。
しかし、会社では私のスタイルは受け入れてもらえず、
会社の定めた規則に従ってプログラミングしていました。
その規則とは、次のようなものです。
・if文には必ず{}をつけろ
・マルチステートメントにするな
・goto文、continue文は使うな
・比較演算子の「>」は使うな
・ソースコードの右側にコメントを記述するな
対して、自分は、以下のようなスタイルを持っています。
・if文の{}は、前後とのバランスなどに応じて、つけたりつけなかったり。
・似た処理が続く場合は、マルチステートメントにする(特にswitch文のcase処理)。
・不必要にネストが深くなるのを避けるために、continue文を使う。
・例外処理を簡潔化するため、場合によってはgoto文も使用。
・変数と数値を比較するときは、変数を必ず左側に書く。
・ソースを左側、コメントを右側と分けることで、印刷時の可読性を向上。
特に、最大の違いは、行数です。
会社の規則に従うと、縦に間延びしたソースコードとなり、
スクロールしないと全体を見渡せません。
対して、自分は
「1行には『1つの意味』を持つコードを記述」
することを心掛けています。
以下に、会社の規則に従って記述した「ソースコードA」と、
自分流のスタイルで記述した「ソースコードB」を書き込みさせていただきました。
皆さんは、どちらのソースコードがよりよいと思いますか?
率直なご意見をお聞かせいただければ幸いです。
■ ソースコードA:会社の規則スタイル ■
001|#include stdafx.h
002|
003|#define DATASIZE 8
004|#define DATAMAX 256
005|
006|bool Function(void)
007|{
008| FILE *fp;
009| char *Buf;
010| int Op, No1, No2, Ans;
011| int Cnt1, Cnt2, ErrCnt;
012| int i, v;
013|
014|
015| ////// メモリ確保/ファイルオープン(処理a)
016|
017| Cnt1 = Cnt2 = ErrCnt = 0;
018|
019| // メモリ確保
020| if ((Buf=(char*)malloc(DATAMAX*DATASIZE)) == NULL) {
021| return FALSE;
022| }
023|
024| // ファイル有無チェック
025| if (FileExist(file.dat) == FALSE) {
026| free(Buf);
027| return FALSE;
028| }
029|
030| // ファイルオープン
031| if ((fp=fopen(file.dat,rb)) == NULL) {
032| free(Buf);
033| return FALSE;
034| }
035|
036| // ファイル読み込み
037| if (fread(Buf, DATASIZE, DATAMAX, fp) < DATAMAX) {
038| fclose(fp);
039| free(Buf);
040| return FALSE;
041| }
042|
043| ////// メイン処理(処理b)
044|
045| for (i=0; i<DATAMAX; i++) {
046|
047| ////// 演算子、数値1,2取得(処理c)
048| if (DataGet(Buf, i, &Op,&No1,&No2) == FALSE) {
049| fclose(fp);
050| free(Buf);
051| return FALSE;
052| }
053|
054| ////// データによる処理の分岐(処理d)
055|
056| // データ読み飛ばしでないなら処理を実施
057| if (Op != 0) {
058|
059| // 0除算チェック
060| if ((Op != '/' && Op != '%') || No2 != 0) {
061|
062| // 数値1,2ともに正ならCnt1加算
063| if (0 < No1 && 0 < No2) {
064| Cnt1++;
065| }
066| // 数値1,2ともに正でないならCnt2加算
067| else {
068| Cnt2++;
069| }
070|
071| ////// 演算処理(処理e)
072| switch (Op) {
073| case '+':
074| Ans = No1 + No2;
075| break;
076| case '-':
077| Ans = No1 - No2;
078| break;
079| case '*':
080| Ans = No1 * No2;
081| break;
082| case '/':
083| Ans = No1 / No2;
084| break;
085| case '%':
086| Ans = No1 % No2;
087| break;
088| }
089|
090| ////// 結果表示(処理f)
091| if (AnsPut(i, Ans) == FALSE) {
092| fclose(fp);
093| free(Buf);
094| return FALSE;
095| }
096|
097| ////// その他の処理(処理g)
098|
099| DataGetMaking(Buf, i, &v);
100| if (v) {
101| DataSetMaking(v, Ans, 300, 128);
102| }
103|
104| DataGetComp(Buf, i, &v);
105| if (v) {
106| DataSetComp(v, No1, 210, 85);
107| }
108|
109| DataGetAdjust(Buf, i, &v);
110| if (v) {
111| DataSetAdjust(v, No2, 108, 255);
112| }
113|
114| DataGetCheck(Buf, i, &v);
115| if (v) {
116| DataSetCheck(v, Op, 153, 32);
117| }
118|
119| DataGetListUp(Buf, i, &v);
120| if (v) {
121| DataSetListUp(v, 0, 4, 11);
122| }
123|
124| }
125| else {
126| ErrCnt++;
127| }
128| }
129|
130| }
131|
132| ////// 後処理(処理h)
133| if (KekkaSyori(fp, Cnt1, Cnt2, ErrCnt) == FALSE) {
134| fclose(fp);
135| free(Buf);
136| return FALSE;
137| }
138|
139| fclose(fp);
140| free(Buf);
141| return TRUE;
142|}
■ ソースコードB:自分流スタイル ■
(本来は、ソースコードの右側にコメントがあるのですが、
折り返し発生防止のため割愛してあります)
01|#include stdafx.h
02|
03|#define DATASIZE 8
04|#define DATAMAX 256
05|#define FileClose(a) if (a!=NULL) { fclose(a);a=NULL; }
06|#define MemFree(a) if (a!=NULL) { free(a);a=NULL; }
07|
08|bool Function(void)
09|{
10| FILE *fp = NULL;
11| char *Buf = NULL;
12| int Op, No1, No2, Ans;
13| int Cnt1, Cnt2, ErrCnt;
14| int i, v;
15|
16| // メモリ確保/ファイルオープン(処理a)
17| Cnt1 = Cnt2 = ErrCnt = 0;
18| if ((Buf=(char*)malloc(DATAMAX*DATASIZE)) == NULL) goto ErrEnd;
19| if (FileExist(file.dat) == FALSE) goto ErrEnd;
20| if ((fp=fopen(file.dat,rb)) == NULL) goto ErrEnd;
21| if (fread(Buf, DATASIZE, DATAMAX, fp) < DATAMAX) goto ErrEnd;
22|
23| // メイン処理(処理b)
24| for (i=0; i<DATAMAX; i++) {
25|
26| // 演算子、数値1,2取得(処理c)
27| if (DataGet(Buf, i, &Op,&No1,&No2) == FALSE) goto ErrEnd;
28|
29| // データによる処理の分岐(処理d)
30| if (Op==0) { continue; }
31| else if (Op=='/' && No2==0) { ErrCnt++; continue; }
32| else if (Op=='%' && No2==0) { ErrCnt++; continue; }
33| else if (No1 > 0 && No2 > 0) { Cnt1++; }
34| else { Cnt2++; }
35|
36| // 演算処理(処理e)
37| switch (Op) {
38| case '+': Ans = No1 + No2; break;
39| case '-': Ans = No1 - No2; break;
40| case '*': Ans = No1 * No2; break;
41| case '/': Ans = No1 / No2; break;
42| case '%': Ans = No1 % No2; break;
43| }
44|
45| // 結果表示(処理f)
46| if (AnsPut(i, Ans) == FALSE) goto ErrEnd;
47|
48| // その他の処理(処理g)
49| DataGetMaking(Buf, i, &v); if (v) DataSetMaking(v, Ans, 300, 128);
50| DataGetComp (Buf, i, &v); if (v) DataSetComp (v, No1, 210, 85);
51| DataGetAdjust(Buf, i, &v); if (v) DataSetAdjust(v, No2, 108, 255);
52| DataGetCheck (Buf, i, &v); if (v) DataSetCheck (v, Op, 153, 32);
53| DataGetListUp(Buf, i, &v); if (v) DataSetListUp(v, 0, 4, 11);
54|
55| }
56|
57| // 後処理(処理h)
58| if (KekkaSyori(fp, Buf, Cnt1, Cnt2, ErrCnt) == FALSE) goto ErrEnd;
59|
60| FileClose(fp);
61| MemFree(Buf);
62| return TRUE;
63|
64|ErrEnd:
65| // エラー発生時処理(処理i)
66| FileClose(fp);
67| MemFree(Buf);
68| return FALSE;
69|}
> DataGetMaking(Buf, i, &v); if (v) DataSetMaking(v, Ans, 300, 128);
1行に複数の命令を入れるのは避けてもらいたところです。
デバッグ時に追いづらくなるので。
ソースコード2
if ((Buf=(char*)malloc(DATAMAX*DATASIZE)) == NULL) goto ErrEnd;
確保できていないメモリ解放に行きますよね
コード1ならなにもせづ戻るのだから問題少ないよね
呼び出し元にエラーコード返したいよね
どんなコードまで処理して戻ってきたのか?
if (FileExist(file.dat) == FALSE) goto ErrEnd;
ファイルオープンしていない、ファイルポインタをクローズしに行こうとしますよね
エラー処理が、エラー誘発コードになるのは避けたいですね
共通
Cnt1 = Cnt2 = ErrCnt = 0;
他言語的考え方では好ましくありません
Cnt1 = 0;
Cnt2 = 0;
ErrCnt = 0;
必ず実数を明示代入させます
アドレスが代入されるバグを減らすためです
よのなかいろいろと変な規則は存在するね。
面倒な思いするならgoto使ってもいいんじゃね?っとか。
・if文には必ず{}をつけろ
賛成
ソース修正とかブレークポイント置くとか考えると
1行にif((r = func()) == v) a = b;というのは好まない。
・goto文、continue文は使うな
gotoは使わないに越したことないと思うが強制はないな。
continue使わないとかかえってへんなコード書くことになるだろ
あたま悪い奴が自分のために考えたルールだな?
・比較演算子の「>」は使うな
あたま悪い奴が自分のために考えたルール?
・ソースコードの右側にコメントを記述するな
?
>会社の規則に従うと、縦に間延びしたソースコードとなり、
俺はこれよりさらに縦伸び派
可読性をあげるためのルールが
やがてルールを守れとかルールを守らないお前が気に入らないって話になって
可読性を下げてまでルールを守るというおかしなことはよくあるよね。
総合的にみると俺は会社のルールの方がいいと思う。
部分的にみると会社のルールに気に入らないものがある。
受け入れないとは細かくルールでなおさせられるってことか。
多少は直した方がいいと思う。
おれは細かくルール強制はしない。
だって俺がその時その時気分的に書きやすく書くアバウトだから。
Functionの前に
// FALSE エラー
とか書いてないのが気になる。
常識的にエラーはFALSEだろみたいなのはあるかもしれないけれど。
すぐ下のmallocでわかるから
// メモリ確保
ってコメント必要?
俺の今の好みはswitch caseは同じインデント。
switchのないcaseなんてないし
複数のswitch caseが連続している場合を除けばれば
caseの上の行を見ればすぐにswitchを見つけられるし
無駄に左に空白があいて横スクロールが必要になる。
C/C++にtry finallyがあればgoto ErrEndなんていらないのにね。
独断と偏見でいえば
私にとってはソースコードAのほうがよい
ソースコードB 読む気もしない
ひょっとしたら ひとりよがりの考えに凝り固まって
いらっしゃるのではないでしょうか
ぱーっとみたところ
ソースコードA のほうがよみやすい ソースコードB のほうはつかれる
ただし会社規則ってやつの
・goto文、continue文は使うな
・比較演算子の「>」は使うな
・ソースコードの右側にコメントを記述するな
なんじゃこれ?って感じ
他の人とほぼ同じで基本的にはAの方が見やすいです。
・if文には必ず{}をつけろ
他の方も言われていますが、デバッガで追う時のことを
考えるとこっちの方がステップ・バイ・ステップで
追えるので有利です。
・goto文、continue文は使うな
gotoはともかく、continueに関しては反対ですね。
かえって読みにくくなります。
あと、gotoに関しては使った方がリーズナブルであれば、
良いかなと考えています。特に逆戻りしないgotoならば。
但し、gotoを使うことでバグを誘発するようなコードに
なるなら使うべきじゃないです。
どうしても使わざる得ない時だけですね。
・比較演算子の「>」は使うな
・ソースコードの右側にコメントを記述するな
この二つに関しては意味不明ですね。
理由がきちんと説明されていれば、そういう意味かと思えるかもしれませんが。
まあ、コメントの入れ方に関しては色々有りそうなので
ここに書かれている内容だけでは何とも言いかねます。
で、個人に取って読みやすいソースが万人にとって読みやすいかは
かなり難しい話なのでこれなら誰が読んでも解りやすいと言うのは
難しいかなと思います。これだと大半の人は読みやすいと言うのなら
ありえそうですけれど。
会社のコーディングルールは画一化することでソースによって見方を
変える必要はなくなると言う意味では有用です。
なので、リーズナブルな理由があるのであれば採用されてしかるべきですし、
逆に意味不明なものなら意味があるものに変更するのは一向に構わないと
思います。この辺はコーディング規約の改正が出来るかどうかと言う話に
なりますね。
但し、改訂するにしても万能のコーディングルールと言うのは多分ありえない
と思うので柔軟な発想が必要だと思います。
追記
・マルチステートメントにするな
一般的にマルチステートメントは可読性が落ちるので
止めましょうと言うのが多いと思いますよ。
あとは、トリッキーなソースより素直な記述が良いとか
ありますね。
ソースをメンテナンスする人材がいつもベテランとは限りませんから
不必要にソースを難解にするのはメンテナンス性を落とす事になります。
> C/C++にtry finallyがあればgoto ErrEndなんていらないのにね。
C++ならデストラクタあるので、個人的にはfinallyはなくてもいいのですが、
Cは…許されるならばもう書きたくない…
というか、bool使ってるのはCでなくC++なんでしょうか。(afx…だとC99ではなさそう?)
であれば、mallocして各returnでfreeとかやってる時点で「ない」なぁ。>どっちも
と言うのが率直な感想です。
実は別に「(C++なのに)例外処理禁止」とかいう謎ルールがあったりするとか。
# でも、こういうルール作るC言語屋さんに限って、
# MFCとか使うのにCExceptionとか考えてなかったり、
# ひどいところになると、catchすら許してくれなくて「何それ?」とかいいそう…。
此処には触れて無さそうだけど…
>003|#define DATASIZE 8
>004|#define DATAMAX 256
#define DATASIZE (8)
#define DATAMAX (256)
と、自分は括弧を意図的につける様にしています。
(例えば…)
#define DATA1 8
#define DATA2 DATA1 - 1
#define DATAALL DATA1 * DATA2
この時()をつけるか否かで異なる値になります。
1.まず、A、B共に十分見やすいコードだと思います。
次に、可読性、保守性は、それを見る人の能力や、
使用するツールによるので何とも言えません。
2.コーディングルールは、多分今後は意味がなくなるので、あまり
気にしすぎる必要は無いように思います。
今は、VC2008のエディタ等の様に、コメント、関数本体、
引数等の表示非表示の選択機能ぐらい付いて無いと
ダルくてやってられません。単純なテキストエディタなどは
もう、長い間使ってません。
おそらく今後は、この様な専用コーディングツール化の方向に
進んでいくと想像でき、コード時のスタイルと、テキストへの
出力時のスタイルの分離が進むのでしょう。
ですから、あまりスタイルなどに気をとられずに、良い仕組み
の創造に注力していったほうが良いと考えます。
「いずれ意味の無くなることに気をとられるな」ということです。
まぁ個人的な意見言い出したらきりがないですが、
基本的にコーディング規約というのは、「みんなで同じにすると他人のコードが読み
やすいですよ」というものなので、決めごとでしかないなら、上司の言うとおりにす
るというのが私の姿勢です。
その視点で見ると、以下の3つは決めごとなので従えばよいのではないでしょうか。
・if文には必ず{}をつけろ
・マルチステートメントにするな
・ソースコードの右側にコメントを記述するな
以下のやつは意味不明。
・比較演算子の「>」は使うな
不等号の向きを全社統一しようという試みでしょうか?そういうことなら決めごとに
すぎないので、やはり従えばよいと思います。
以下のやつだけ反対です。
・goto文、continue文は使うな
これは決めごとではないからです。goto文、continue文を使わないと、ソースコード
の構造が変わります。しかも、本当にgoto文、continue文が有効な場面では、汚い方
へ構造が変わるからです。
私もAが見やすい
そのソース・仕事内容を知ってる人ならBのほうが見やすいが
Aなら、まったく知らない人でも、慣れた人でも大差なく見れる
って思います
>「1行には『1つの意味』を持つコードを記述」
Aは、コンピュータの処理が一行に一つの意味
Bは、プログラマーがやらせたい処理が一行に一つの意味
という感じになってますな
最初に書きましたが、
やらせたい処理が見る人に伝わってるならBのほうがいいが
そうでないなら
「一行分のやらせたいこと」を把握する必要がある分やっかいかと
>・比較演算子の「>」は使うな
たとえば
x > 0 && x < 10 より
0 < x && x < 10 のほうがやらせたいことがわかりやすいとかかな