737コメント214KB

ヘタなコードの書き方

■ このスレッドは過去ログ倉庫に格納されています

0001デフォルトの名無しさん

2008/04/12(土) 23:20:23

基礎編

    * サブルーチン(関数)を長くする。数百行以上。
    * コピペ多用
    * ネストを深くする
    * フラグを多用する
    * マジックナンバーを使いまくる
    * 識別子(変数名、関数名)は意味不明に
    * グローバル変数つかいまくり

まとめサイト
http://www34.atwiki.jp/hetacode/

↓以下、応用編、上級者編

0002デフォルトの名無しさん

2008/04/12(土) 23:24:04

クラスの数だけインターフェースがある

0003デフォルトの名無しさん

2008/04/12(土) 23:31:14

if (success == true) ・・・

とか、bool値のリテラルとの比較があると、コードが素人くさくなる。
(if (success != false) みたいにfalseとの比較ならいいとかって話じゃなくてね)

0004デフォルトの名無しさん

2008/04/12(土) 23:37:29

/*************************************************
*
*
*
*************************************************/

サブルーチンごとに、先頭に、こういう罫線を囲んだコメントがついてるとか。
ひどくなると、サブルーチンの途中でもがんがん入れてある。

0005デフォルトの名無しさん

2008/04/12(土) 23:37:38

if not notNullCheck(notNullFlg) then

0006デフォルトの名無しさん

2008/04/12(土) 23:41:09

' <<<------------ 初期化 ------------>>>
：
：
' <<<------------ DB更新 ------------>>>
：

コメントにいらん飾りを入れる

0007デフォルトの名無しさん

2008/04/12(土) 23:43:25

基礎編
見れば分かるコメントを入れる

fp = fopen(path, "r"); // ファイルオープン

return hogeLength; // hogeの長さを返す

0008デフォルトの名無しさん

2008/04/12(土) 23:48:39

言語依存なものばかりだな

0009デフォルトの名無しさん

2008/04/12(土) 23:49:29

グローバル変数の応用編

size = **;
func(table);

配列は引数で渡してるのに、配列に入ってるデータの個数はグローバル変数で渡すとかってコードを見たときは、
鳥肌たった。

0010デフォルトの名無しさん

2008/04/12(土) 23:59:41

Java、C++ あたりのサブルーチンの途中でも変数宣言できる言語でも、宣言は全部サブルーチンの
先頭で行う。

0011デフォルトの名無しさん

2008/04/13(日) 00:10:50

引数が３０個以上ある関数

0012デフォルトの名無しさん

2008/04/13(日) 00:20:22

// ADD START M.YAMADA 2008/4/15
:
:
// ADD END

変更履歴みたいなコメントが大量にあるとか、変更前のコードとか削除したコードを消さずに
コメントアウトして残してあるとか。

0013デフォルトの名無しさん

2008/04/13(日) 00:25:06

>>12
それは言語の責任ではないしコードの問題でもない
バージョン管理システムに類するものの有無の問題だ

0014デフォルトの名無しさん

2008/04/13(日) 00:29:16

・暗黙の型変換(キャスト)
・省略可能な{}を省略してしまう(K&R)
・難解な演算子の使用法
　 (例) o = --o - o--;

0015デフォルトの名無しさん

2008/04/13(日) 00:31:47

int xxx = 0;
int xxxx = 0;
int xxxxxx = 0;
int xxxxxxx = 0;

↑こういうのじゃなくて

int xxx       = 0;
int xxxx      = 0;
int xxxxxx   = 0;
int xxxxxxx = 0;

↑こういういちいち桁を揃えてるほうが素人っぽく見える

0016デフォルトの名無しさん

2008/04/13(日) 00:34:57

>>13
コードの問題です。
コードの書き方と環境の問題は切り離せません。

0017デフォルトの名無しさん

2008/04/13(日) 00:42:26

・インデントがずれてる。
・関数が長い(制御のブロックが長い)
・削除せずにコメントアウトしてコードを残してる

のコンボで、どうしようもないコードをいじったことがある。

制御ブロックの最後の } がどこに対応してるか、目で追うのが大変すぎるから、
エディタの対応する括弧に飛ぶ機能を使おうとしたら、

// if (hoge > 100) { // DEL M.YAMADA 2000/10/10
if (hoge > 200 { // MODIFY M.YAMADA 2000/10/10

みたいなコードが途中に何行もあって、それも使えないでやんの。

0018デフォルトの名無しさん

2008/04/13(日) 00:48:20

>>14
>　 (例) o = --o - o--;

これは、下手なコードと言うよりバグだろ。

0019デフォルトの名無しさん

2008/04/13(日) 00:48:24

>>17
それは自動整形に通せよ
インデントのズレ修正とコメント削除はできるだろ

0020デフォルトの名無しさん

2008/04/13(日) 00:49:12

>>17
コメント内の括弧を対応させようとする間抜けなエディタを使うのを止めたらいい。

0021デフォルトの名無しさん

2008/04/13(日) 00:50:08

Cで。
文字列はかならず memset() で初期化。
かつ NULL を使うと上級者。

memset(s, NULL, sizeof s);

0022デフォルトの名無しさん

2008/04/13(日) 00:53:13

>>20
素のviしかない環境で作業させられた。

つか、ふだんそういう機能が必要になる局面がないんで、つかってるエディタが、言語の
コメントを意識するかとか、考えたこともないけど。

0023デフォルトの名無しさん

2008/04/13(日) 00:57:11

>>19
しょうがないから、コメントとか、大量のデバッグprintはいったん削除して作業した。
勝手にコメントを削除していいような空気でなかったんで、動作確認したら、
変更箇所をもとのソースに書きもどしたけど。

0024デフォルトの名無しさん

2008/04/13(日) 00:58:03

>>22
それは作業環境の改善を要求するべきだったね。とりあえずvimかemacs入れてもらうとか。

少なくともSyntax Coloringするエディタは言語のコメントを意識してるかな。

0025デフォルトの名無しさん

2008/04/13(日) 00:58:36

>>22-23
それならばスレ違い

0026デフォルトの名無しさん

2008/04/13(日) 01:00:57

if (cond)
return true;
else
return false;

0027デフォルトの名無しさん

2008/04/13(日) 01:13:19

職場の年寄りがVB.NETで仕事してるけど、やっぱ変数名は intHoge とか lngHoge とかやってる。
打ち合わせで、ハンガリアンやめましょー、みたいな話がでて、そうしようみたいになってたと思うけど、
伝わってなかったか。

ほかの人も、MSのクラスライブラリの作成ガイドラインみたいな文章に沿いましょうみたいな話に
なってたはずだけど、なぜか定数値だけは、HOGE_MAX みたいな、Cのスタイルにこだわってたし。

0028デフォルトの名無しさん

2008/04/13(日) 01:44:16

>>27
ああ、あるな。

0029デフォルトの名無しさん

2008/04/13(日) 05:09:21

やる気の低下で1週間とか放置することがあるから汚いコードとは思いつつコメントだけは多めに入れてある

0030デフォルトの名無しさん

2008/04/13(日) 08:38:15

後で大変なことになるぞーと思いつつ、バカが考えたコーディング規約厳守だから俺にはどうにも出来ないorz

0031デフォルトの名無しさん

2008/04/13(日) 12:03:20

checkをchkとか、countをcntとか、単語を省略すると一気に素人くさいコードになるな。
とくにcntは、ネイティブはぜったいやらない省略。

0032デフォルトの名無しさん

2008/04/13(日) 12:10:19

じゃあこれらは何かの間違いか全部非英語圏の人間の書いたコードだな
ttp://www.google.com/codesearch?q=cnt&hl=en

0033デフォルトの名無しさん

2008/04/13(日) 12:15:04

contine なんだかcontrolなんだかcountなんだかワカンネーよ

0034デフォルトの名無しさん

2008/04/13(日) 12:16:00

continueだったorz

0035デフォルトの名無しさん

2008/04/13(日) 12:17:27

少なくとも1ページ目に出てるのは contine でも control でもなさそうだ

0036デフォルトの名無しさん

2008/04/13(日) 12:20:57

ならば問う、cntの正体とはは何だね？明智君

0037デフォルトの名無しさん

2008/04/13(日) 12:57:41

変数名が hogeInfo とか hogeData もヘタクソ臭が。

0038デフォルトの名無しさん

2008/04/13(日) 13:11:10

>>37
LCCなんて時代錯誤は捨ててアンダーバーで区切るべきだよな

0039デフォルトの名無しさん

2008/04/13(日) 13:35:48

if (...) {
　...
}

// なにやら
// 非常に
// 長ったらしい
// 説明と
// 空白行
// の後に...

else if (..) {
↑ってうぉ？！繋がってんのかよ！！！

0040デフォルトの名無しさん

2008/04/13(日) 14:40:09

>>38
infoとかdataじゃ分かりにくいって話じゃないのかこれ。

0041デフォルトの名無しさん

2008/04/13(日) 14:43:48

hogeってそのまま書いてあるのか。そりゃひどいな。

0042デフォルトの名無しさん

2008/04/13(日) 14:44:37

わかりにくいっていうか、変数なんてデータが入ってるもんなんだから、いちいちDataなんてつけないでいいだろってツッコミ。

0043デフォルトの名無しさん

2008/04/13(日) 14:47:05

こういうコードは頭痛が痛いな。

0044デフォルトの名無しさん

2008/04/13(日) 15:00:36

>>42
かと言って例えば受信データを入れる変数を Receive にするのはもっとまずいと思うが。

0045デフォルトの名無しさん

2008/04/13(日) 15:08:10

>>42
もしくはdataじゃなくてnameとかlengthとか
意味がありそうな単語を付けるとかかな。

0046デフォルトの名無しさん

2008/04/13(日) 15:19:58

だから言語実装依存の話なんかしたって意味ねーんだよ

0047デフォルトの名無しさん

2008/04/13(日) 15:24:38

名前や長さというのは名前ではなく所属クラス自体に持たせる情報だな

クラスがあればだが

クラスが無い場合は名前に情報を持たせるしかないだろう

0048デフォルトの名無しさん

2008/04/13(日) 16:15:02

>>46
自分でスレを立てたらいいよ。"言語に依存しないコードの上手下手"とかそんな感じで。
ム板にしてはかなり盛り上がっているのだし、吠えても意味ない。

0049デフォルトの名無しさん

2008/04/13(日) 16:19:04

>>48
言語や環境を特定せずに「コードが下手」と言っても意味がないという指摘なのでは
Javaの論理をVBに持ち込んでも仕方がないし、PHPの書き方をC++で使っても無為だ

0050デフォルトの名無しさん

2008/04/13(日) 16:19:16

コメントの書き方とか、変数名の話とか、どこが言語実装依存かよくわからんけど。

0051デフォルトの名無しさん

2008/04/13(日) 16:21:14

>8 >46
>7 や >11 辺りは比較的言語に依存しないヘタコードだと思うが。

n = 65; // n に 65 を代入する

ウボァー

0052デフォルトの名無しさん

2008/04/13(日) 16:23:12

言語によっては、マジックナンバーを使うべしとか、グローバル変数をどんどん使うべしってのが
お作法になってりするのか。
たしかにN88BASICとかグローバル変数しかないし、HSPとか構造化されてないコードとか当たり前みたいだけど。

0053デフォルトの名無しさん

2008/04/13(日) 16:50:01

定番の話題だけど、比較で定数を左にもってくるやつとか

if (100 == n) ・・・

0054デフォルトの名無しさん

2008/04/13(日) 17:03:03

= とミスする可能性は排除するには、有りだな

0055デフォルトの名無しさん

2008/04/13(日) 17:19:59

最近のコンパイラは警告してくれるけどな。

0056デフォルトの名無しさん

2008/04/13(日) 17:21:58

代入と比較を間違えたらコンパイラが警告出すから、定数値を左にもってくるような不自然な書き方はしないってのが主流。

0057デフォルトの名無しさん

2008/04/13(日) 17:30:58

google code searchで検索すると

==\s*0 lang:c
1,870,000

0\s*== lang:c
143,000

「定数が左」派は7%超えてるのか。
思ったより多いな。1%未満かと思ってた。

0058デフォルトの名無しさん

2008/04/13(日) 17:55:11

VB.netだけど
bFlag = i = 0
って式があって一瞬混乱したんだが、こういう場合には
bFlag = 0 = i
みたいに定数を左に持ってきて、条件式だって明示するのもいいかもしれないとふと思った。

0059デフォルトの名無しさん

2008/04/13(日) 18:23:13

おれは、

button.Enabled = (count = 0)

みたいに括弧でくくってるよ。

0060デフォルトの名無しさん

2008/04/13(日) 19:11:01

代入演算子と等値演算子が同じなんて、言語仕様がバグってる。

0061デフォルトの名無しさん

2008/04/13(日) 19:25:23

左辺と右辺が完全に独立しているから、混乱することはないので問題ない、
それよりも、式のど真ん中で代入できたり、代入がない式でもエラーが出ない方が異常じゃないかと思いつつC++を15年使ってる。

0062デフォルトの名無しさん

2008/04/13(日) 20:15:16

「あとで(自分を含め)誰かが読んで何やってるかわかりにくいのがへたなコード」ってことでおｋ？

・相変わらず昔の名残で変数の宣言をメソッドの先頭でやって、後の方で使う
・同じ変数を、違う場面で(違う意味で)使いまわす

0063デフォルトの名無しさん

2008/04/13(日) 20:18:53

・同じ変数を、違う場面で(違う意味で)使いまわす
これ最悪だな

0064デフォルトの名無しさん

2008/04/13(日) 20:35:13

関数の中のコードをほんの数行変えてfunc2とかfunc3とかを量産するのはホントやめて欲しい。

0065デフォルトの名無しさん

2008/04/13(日) 21:19:10

>1 の条件すべて満たすコードばっかりのところで仕事してる。
関数が長いのに加えてその中に長大な#ifdef～#endifが無数にあって、一部はネストになってる。
もう人間が読めるようなものではない（がんばって読んでいるけどね）。

この仕事何年やってんだよお前ら。

0066デフォルトの名無しさん

2008/04/13(日) 21:30:33

>>60 きもちわるいよね

0067デフォルトの名無しさん

2008/04/13(日) 21:39:53

>>62
> ・相変わらず昔の名残で変数の宣言をメソッドの先頭でやって、後の方で使う

C++やC#で、これプラスして、変数は必ず初期化するってルールでやってるもんだから、適当に0とかNULL
で全部初期化してたりするところもあるな。

コンパイラは未初期化の変数を参照したら警告出してくれるけど、それが台無しになってるっていう。

VB.NETは、初期化を省略したら勝手に初期化するって仕様だから、デフォでこの状態になってるけど。

0068デフォルトの名無しさん

2008/04/13(日) 21:57:39

VB.NETにもスコープ用の{}みたいなのが欲しいな。
スコープを意識するような文化が根付けば
メソッドの先頭で変数宣言することも減るかもしれない。
ついでにスコープごとでの処理の抽出もしやすくなり、
自然と各メソッドも短くなるかもしれない。

0069デフォルトの名無しさん

2008/04/13(日) 22:06:16

goto使いたくないからという理由で do {} while (0); でくくる。
キモいよ、キモすぎ。

0070デフォルトの名無しさん

2008/04/13(日) 22:10:53

・省略可能な{}を記述してしまう
・三項演算子を使わない

0071デフォルトの名無しさん

2008/04/13(日) 22:19:42

３項演算子は、書くときはすごい簡潔になってスゲーと思うんだが、他人が書いたのを読まされるときはさっぱり意味がワカンネエと感じる。

0072デフォルトの名無しさん

2008/04/13(日) 22:24:48

省略可能な {} ってなに？
if (a) {b = 1;} else {b = 2;} → if (a) b = 1; else b = 2; みたいなやつのこと？

0073デフォルトの名無しさん

2008/04/13(日) 22:27:20

>>69
それってdo whileの中で複数判定があって、そこでbreakさせるってこと？
別関数にしてreturnしちゃったほうが手っ取り早そうだな。

0074デフォルトの名無しさん

2008/04/13(日) 22:30:11

>>72
それは3項演算子を使わないのほうじゃないのか？

0075デフォルトの名無しさん

2008/04/13(日) 22:33:23

if ( a )
　b = 1; //省略可能
else
{
　b = 2; //省略不可能
　c = 1;
}
こういうことか

0076デフォルトの名無しさん

2008/04/13(日) 22:35:45

そうなんですよ、いろんな箇所でbreak。
モチベーションとしては呼び出し元にreturnするのは、そのサブルーチンの最後ってことにしたいらしい。
コード自身のクオリティーがそもそも高ければ、エラー処理程度のgoto文は普通にOKだと思いマッスル。

0077デフォルトの名無しさん

2008/04/13(日) 22:36:19

>>69
あるある。マイクロソフトのドライバのサンプルコードにそれが。

0078デフォルトの名無しさん

2008/04/13(日) 22:38:25

if (cond) {
n = 0;
}

こういう、一行でもカッコをつけるべしってスタイルの人は、このスタイルが絶対的に正しいって
信じ込んでる場合が多いですな。

0079デフォルトの名無しさん

2008/04/13(日) 22:40:27

>>78
はい、そのスタイルは絶対神であります。

0080デフォルトの名無しさん

2008/04/13(日) 22:40:53

>>75さんに言うわけじゃないけど
if、elseに限らず、たとえ省略可能であっても {} は省略しないほうがいい。
動作チェック後、たった1行の修正しようとした新人PGがこれにはまったことがあった。
省略可能とはいえ、その後のメンテを考えればつけるべき。

0081デフォルトの名無しさん

2008/04/13(日) 22:42:51

if ( a )
　b = 1; //省略可能
　d = 1;
else
　d = 2;
{
　b = 2; //省略不可能
　c = 1;
}

気にしない。気にしない。

0082デフォルトの名無しさん

2008/04/13(日) 22:45:57

有名プロダクツのソースとか、定番の書籍とかでも、省略してるのはよくあるよ。

追加したときに、{}を付け忘れてハマるから省略すべきでないってのが理由だけど、
そんなので嵌るのって、10年プログラムやってて、一回あるかどうかだと思われ。

0083デフォルトの名無しさん

2008/04/13(日) 22:50:12

>>82
うん、Linux kernelソースなんかは全然ついてないね。

でも、プロプライエタリを複数人で開発してると、ソースの寿命ってそんなに短いわけじゃないから、
いつ新人が入ってきていじりだすか分からない。
事故は未然に防ぐという意味は十分あると思う。

0084デフォルトの名無しさん

2008/04/13(日) 22:51:42

>>82
そんな理由じゃなく、神には叛けないからだろ。

0085デフォルトの名無しさん

2008/04/13(日) 22:55:27

if ( a )
　b = 1;
これみたいに1行ならいいけど

for (i = 0; i < 100; i++)
　if (aaa (i)) {

　}
こんなのは外側のも囲わないと気持ち悪く感じる

0086側近中の側近 ◆0351148456

2008/04/13(日) 22:55:52

>>82
(っ´▽｀)っ
>10年プログラムやってて、一回あるかどうか
かなり高確率じゃないか。
こういうバグで数百億の損害を出すことだってあるんだよ。

0087デフォルトの名無しさん

2008/04/13(日) 22:55:57

俺は１行か２行以上か考えて変えるのが面倒くさいという理由で全部に付けてる。
変更で後から付けるのも面倒だし、変更で後から外すのも面倒だ。
とにかく頭を働かせるのが面倒だ。俺のチャンクは他人より少ないんだぜ。

0088デフォルトの名無しさん

2008/04/13(日) 22:59:44

2行を1行に修正した時に {} をとるってのもまぁ、律儀といえば律儀なんだろうけど変な気がする。
今のところそんな diff にはお目にかかったことがありません。

0089側近中の側近 ◆0351148456

2008/04/13(日) 23:03:09

(っ´▽｀)っがリーダーで、if文に{}をつけないメンバがいたら、

お前は今後の保守において、if文が2行になった時、
{}を付け忘れる奴がいないことを保証できるのか？

と問い詰めるね☆

0090デフォルトの名無しさん

2008/04/13(日) 23:04:00

>>85
おれは、制御されるコードが一行のときだけ{} 省略ってルールでやってる。
複数行の場合は省略したら、さすがに見難いと思った。
K&Rはガンガン省略してたけど。

>>86
まあ、10年に一度ってのは、適当だけど、linuxとかやってる人は影響ないって思ってるみたいだね。

>>83
MISRAとか、ループをbrackで抜けるの禁止とか、安全側に振ったら、いくらでも厳しくなっちゃうだろうね。

たとえば{}の位置論争とか、たいがいの人はどっちでもいいじゃんと思うだろうけど、{}の省略は、なぜか
省略しない派の人は、ぜったい自分が正しいと思ってる場合が多いって話。

0091デフォルトの名無しさん

2008/04/13(日) 23:06:29

> 複数行の場合は省略したら、さすがに見難いと思った。
具体的にどんなのよ？

0092デフォルトの名無しさん

2008/04/13(日) 23:08:00

>>91
>>85 のようなコード

0093デフォルトの名無しさん

2008/04/13(日) 23:09:47

>>90
自分が正しいと思ってるかどうかじゃなくて、実際に起こった事故の同じ轍は
二度と踏ませないという意味で説得力があるって話。

0094デフォルトの名無しさん

2008/04/13(日) 23:09:57

インデントで見やすくしとけよ。

0095側近中の側近 ◆0351148456

2008/04/13(日) 23:10:26

(っ´▽｀)っ
{}の省略のメリットってなんだろう☆
2文字減らせるってことかな☆
デメリットは2行になった時に{}を付け忘れても普通に動くってことだよね
テストで気づけよと思うかもしれないが、将来直す人に対して言うことはできないよ
{}を付け忘れるなとコメントを残す？それだったら{}つけよう☆

0096デフォルトの名無しさん

2008/04/13(日) 23:10:43

>>91
if (・・・) {
if (・・・)
n = 0;
}

こういう場合は内側のifは{}省略。外側は省略しない。

0097側近中の側近 ◆0351148456

2008/04/13(日) 23:12:29

(っ´▽｀)っ
と言いつつ(っ´▽｀)っも
if(条件A){
}
else if(条件B){
}
else{
}
ってやるけどね。
厳密に{}をつけるルールなら
if(条件A){
}
else{
　　if(条件B){
　　}
　　else{
　　}
}
だね☆

0098側近中の側近 ◆0351148456

2008/04/13(日) 23:13:48

(っ´▽｀)っ
だって、見やすいんだもん☆
条件が増えると
if(条件A){
}
else{
　　if(条件B){
　　}
　　else{
　　　　if(条件C){
　　　　}
　　　　else{
　　　　　　if(条件D){
　　　　　　}
　　　　　　else{
　　　　　　　　if(条件E){
　　　　　　　　}
　　　　　　　　else{
　　　　　　　　}
　　　　　　}
　　　　}
　　}
}
って感じで深くなっていく・・・

0099デフォルトの名無しさん

2008/04/13(日) 23:14:18

>>93
でも、じっさいでかいコードを書いて、実績上げてる人でも、いらねって思ってる人もけっこうな数いるわけで。
そうでないひともいっぱいいるし。
正直「どっちでもいい」レベルの話だとしか思えない。

>>95
メリットはコードが見やすくなる。

0100デフォルトの名無しさん

2008/04/13(日) 23:14:51

>>95
全然関係ないが、俺は未来にメッセージを送信できる装置を考案したぞ。
完成したら将来直す人と連絡するために使ってみるか？

0101デフォルトの名無しさん

2008/04/13(日) 23:15:55

>>97 >>98
厳密うんぬんの方はぶっちゃけどうでもいいんで、脳内で繰りひろげてください。
ただただキモいです。

■ このスレッドは過去ログ倉庫に格納されています