冗長・不適切なプログラムの例

プログラミングの授業で、“冗長” や “誤り” と判定するプログラムのサンプルを集めました。

printf やキャスト

連続した printf 呼出し

冗長な形	適切な形
printf("答は "); printf("%d", x + y); printf(" と "); printf("%d", x - y); printf(" です。\n");	printf("答は %d と %d です。\n", x + y, x - y);

一行の出力は、普通は一回の printf の呼び出しで済ませましょう。

多すぎるキャスト

冗長な形	適切な形
int n1, n2; double d = (double)(n1 + n2) / (double)2;	int n1, n2; double d = (double)(n1 + n2) / 2;
	int n1, n2; double d = (n1 + n2) / (double)2;
	int n1, n2; double d = (n1 + n2) / 2.0;

部分式のどこかが double にキャストされれば、残りは暗黙的にキャストされます。

冗長な形	適切な形
printf("一辺は %f です、\n", (double)sqrt(10));	printf("一辺は %f です、\n", sqrt(10));

関数 sqrt の戻り値の型はもともと void なのでキャストは必要ありません。

不適切な変数名

不適切な形	適切な形
int l = 0, O = 1, I = 2;	int i = 0, j = 1, k = 2;

説明しなくても、わかるでしょう。

if 文

同じ条件式

冗長な形	適切な形
if (pt >= 60) { printf("pass!"); } if (pt < 60) { printf("fail!"); }	if (pt >= 60) { printf("pass!"); } else { printf("fail!"); }
if (pt >= 60) { printf("pass!"); } else if (pt < 60) { printf("fail!"); }

同じ（あるいは全く逆の）条件式を重複するのはよくありません。条件を変更するときに 2 箇所を同時に変更する必要が出てきます。

一部だけ違う出力

冗長な形	適切な形
if (x % 3 == 0) { printf("余りは 0 です。\n"); } if (x % 3 == 1) { printf("余りは 1 です。\n"); } if (x % 3 == 2) { printf("余りは 2 です。\n"); }	printf("余りは %d です。\n", x % 3);

出力の一部だけが変わるなら printf の書式指定を使います。

if と else で重複

冗長な形	適切な形
if (n < 0) { n *= -1; printf("%d", n); } else { printf("%d", n); }	if (n < 0) { n *= -1; } printf("%d", n);

if と else に同じ処理があるならば、外に出すべきです。

else のあとが空

冗長な形	適切な形
if (i > 0) { printf("みつかりました\n");　 } else { }	if (i > 0) { printf("みつかりました\n");　 }

単純に else 以降は必要ありません。

if のあとが空

冗長な形	適切な形
if (i > 0) { } else { printf("みつかりませんでした\n");　 }	if (i <= 0) { printf("みつかりませんでした\n");　 }

条件式を否定して if だけにします。

不等号の連鎖

誤った形	正しい形
if (0 <= i <= 9) { … }	if (0 <= i && i <= 9) { … }

これは冗長なのではなく、明確に間違いです。

コンマ演算子の不適切な使用

誤った形	正しい形
if (0 <= i, i <= 9) { … }	if (0 <= i && i <= 9) { … }

副作用のない条件式をコンマ演算子の左オペランドに使っても意味はありません。

繰り返し

余計なブレース

冗長な形	適切な形
for (…) { { ⋮ } }	for (…) { ⋮ }

ブレースが多すぎます。

ループの前の条件文

冗長な形	適切な形
if (no > 0) { for (int i = 0; i < no; i++) { .... } }	for (int i = 0; i < no; i++) { .... }

これは単純に if 文は必要ありません。 if 文がなくても no が 0 以下のとき、.... は一度も実行されません。

常に成り立つ条件式

冗長な形	適切な形
for (int i = 0; 0 <= i && i < no; i++) { .... }	for (int i = 0; i < no; i++) { .... }

この場所では 0 <= i は常に成り立ちます。

ちょっとの工夫でなくせる条件判断

冗長な形	適切な形
for (int i = 0; i < 100; i++) { if (i % 3 == 0) { printf("%d ", i); } }	for (int i = 0; i < 100; i += 3) { printf("%d ", i); }

最初から i は 3 の倍数だけを動くように書くべきです。

最初だけ特別扱い

冗長な形	適切な形
int m; for (int i = 0; i < 100; i++) { if (i == 0) { m = arr[i]; } else if (arr[i] > 0) { m += arr[i]; } }	int m = arr[0]; for (int i = 1; i < 100; i++) { if (arr[i] > m) { m += arr[i]; } }

最初だけ、特別扱いするなら、ループの前に出すべきです。

最後だけ特別扱い

冗長な形	適切な形
int m; for (int i = 0; i < 100; i++) { if (i < 99) { printf("%d, ", arr[i]); } else { printf("%d", arr[i]); } }	int i; for (i = 0; i < 99; i++) { printf("%d, ", arr[i]); } printf("%d", arr[i]);

最後だけ特別扱いするなら、ループのあとに出すべきです。

繰返しを制御する変数を他所で変更

不適切な形	適切な形
i = 0; for (k = 1; i < n; k *= 2) { ⋮ i++; }	k = 1; for (i = 0; i < n; i++) { ⋮ k *= 2; }

繰返しを制御する変数は for 文の丸括弧の内部で変更するべきです。さもないと繰返しがわかりにくくなります。

必要のない二重ループ

冗長な形	適切な形
for (int i = 0; i < n; i++) { for (int j = 0; j < n; j++) { if (i == j) { printf("%d ", a[i][j]); } } }	for (int i = 0; i < n; i++) { printf("%d ", a[i][i]); }

繰返し本体の最後に continue 文

冗長な形	適切な形
for (…) { ⋮ if (…) { ⋮ } else { continue; } }	for (…) { ⋮ if (…) { ⋮ } }

ループの最後の continue はなくても同じです。

do 文に固執

冗長な形	適切な形
if (n > 0) { do { ... } while (n > 0); }	while (n > 0) { ... }

ループ本体を実行しない可能性があるならば、 do 文ではなくて while 文または for 文を使うべきです。

未定義

誤った形	適切な形
v = v++;	v++;

これは冗長なのではなく、明確に間違いです。 v = v++ の動作は、同じ変数の値の変更の順序が不明確で未定義 (undefined) です。