魑魅魍魎クソコード初級編

イカれたメンバーを紹介するぜ！

今までに出会って困惑した魑魅魍魎、有象無象のクソコードたちです。
Java、VBAマクロ、Windowsバッチ、SQLといろいろ入り乱れてるのは許してほしい。
クソコードで胸やけしても大丈夫な準備ができたぜ！という方だけどうぞ！
誰が見たってこいつぁクソだぜ！となるようなものから、ほぼバグじゃんそれ、というものをネタがてら集めました。（全部本当にあったシリーズです）

1.無限ループって怖くね？

だって同じことが繰り返されるんだぜ

Excelシートの情報をもとに、文字列を連結していくマクロの処理がそんな重いはずないのに全然処理終わんねぇな、なんでだ？　と思ってデバッグしたらクソコードの罠が待ってたやつ。

VBAマクロ

' colBuffの変数の中にはセルから取得した文字列が格納されている
' 1行が35文字になるように半角スペースをパディング処理
Do Until Len(colBuff) = 35
    colBuff = colBuff & " "
Loop

ここがクソ！

VBAでは

VBAマクロ

Do Until 終了条件 Then
    繰り返したい処理
Loop

という使い方になる。
なので、colBuffが35文字以上だと延々に条件を満たすことはなくループし続けることになるのである。
セルからとってきた文字列がすでに35文字以上の時点で終わる。
運悪く何も知らずに35文字以上に該当して延々とループさせてしまった自分はいったい…
という気持ちになった。（大当たり）

これをこう！

• セルの文字が35文字以上ならエラーになる処理の追加
• 終了条件を35文字以上ならループしないようにする
• そもそもマジックナンバーを使うな

2.わかる。わかるよ。君は言われたとおりにコードを書いただけなんだよね…

設計書みたら「条件に当てはまる場合はAの処理、当てはまらない場合は何もしない」って記載があったわけですよ。
このコメント部分……いるのか？（諸説あるので異論は認める）

java

if (条件文) {
    ... Aの処理実装
} else {
    // 何もしない <-コメントのみ
}

ここがクソ！

何も間違ってはないんだけど、規約上そうしろ！という思想がない限りはできるだけ可読性を優先したい！

これをこう！

世の中にはこれでいいのだとする意見もあるらしいのだけど、自分は業務系システムのエンジニア寄りってこともあって

java

if (条件文) {
    // 処理
}

でよくね？と思ってしまう派閥。

3.何かが不安だったらしい

java

if (hogeList.size() > 0) {
    for (int i = 0; hogeList.size() > i; i++) {
        処理
    }
}

なんか過去に嫌なことでもあったんか。

ここがクソ！

記載しているfor文はListの要素数が0の場合はそもそもループ処理を行わないので、わざわざif文でネストする必要はないのである。
過去は何かさぞかし意味をもつif文だったのだろうと信じたい。

これをこう！

java

for (int i = 0; hogeList.size() > i; i++) {
    処理
}

ネスト深くなるの嫌い。

4.たどり着けそうで誰もたどり着くことはない

if (errCode.startsWith("AAA")) { 
    処理A
} else if ("AAA001".equals(errCode)) {
    処理B
}

Bの処理にどう頑張ってもいけねーのよ。

ここがクソ！

シンプルにデッドコード！（到達不能コード）
たぶんテストしてねぇんだろうなこれ感も否めない。

これをこう！

ユニークを先にチェックしませう。

if ("AAA001".equals(errCode)) { 
    処理B
} else if (errCode.startsWith("AAA")) {
    処理A
}

5.ちょっとそのパターンは初めて見た。

Sub gomi()
    ' 今はおそらく使われなくなったであろうちゃんと動作しないコード群
    
End Sub

どうした。何があった。

ここがクソ！＆これをこう！

ゴミなら！！！！！！！！！消してほしい！！！！！！！！
見たとき3度見しました。もちろんどこからも呼び出し機会はなかったけどそのネーミングはあまりにも酷い。

6.いつもいつでも俺は4月しか許さないぜ！！！という主張が強い変数名

java

Calendar aprilCalendar = new Calendar();

なお、こんなに4月を気取っているが、この変数には1月から12月のすべての月のデータが保持される可能性がある。
（実装していたタイミングが4月だったらしい。許さんぞ）

ここがクソ！

変数名がaprilCalendarってあったら、そりゃぁもうそれは4月のカレンダーであるはずやん。
ていうか動的に変わるCalendarの変数があるんだろうなと思ってコード覗いたのにこれがあったらびっくりするやん。
変数名には意味がちゃんと通る名前を付けようねって言ってるけど、動的な変数なのに静的な変数の顔をするな。

これをこう！

java

Calendar targetCalendar = new Calendar();

画面上から選択して送信された月毎のデータを格納するので、こういう名前でいいかなぁと。（ケースバイだけどaprilCalendarよりはいい）

7.意外！！それは文字列の返却！

Windowsバッチの終了処理です。

Windowsバッチ

SET ERRCODE=9
EXIT /b ERRCODE

……おわかりいただけただろうか。
さてはテストしてないなこれ！！！ということに気がついちまったねぇ……

ここがクソ！

シンプルにバグ！！！！！

これをこう！

EXIT /b %ERRCODE%

それはそう！

8.Oracle12->19になったことでの罠なのかな、と思ったらそもそもこんな構文どのDBMSにも存在しなかった

……ん？

sql

CREATE TABLE T_HOGE IF NOT EXITST
(
    field-definition,
    field-definition-2
    ...
);

これ、動かしたこと……ある？

ここがクソ！

IF NOT EXISTSはオプションで、指定した名前の表がストア内にすでに存在し、表の定義が指定した定義と完全に一致する場合、表作成をサイレントにスキップします。文の実行の結果としてエラーは返されません。
(引用元：https://docs.oracle.com/cd/E63846_01/html/GettingStartedGuideTables/tablecreationddl.html)

Oracle12cまではテーブル作成の時に利用できた「表がなかったらつくるね」のオプションです。便利ですよね。19c以降は使えないようですが。
ちなみにこやつは動きません。なぜなら位置がおかしいのです。
たぶんだけど、既存のCREATE文に後から気づいて慌てて後ろに付け足しちゃったから間違っちゃったのかな……と思うことにしました。

これをこう！

sql

CREATE TABLE IF NOT EXISTS T_HOGE
(
    field-definition,
    field-definition-2
    ...
);

これが正解です。PostgreSQL、MySQLなどでも同様。

コードを憎んで、人を憎まず

もしもこれらのようなクソコードに、あなたも出会うことがあったとしても、それはコードがクソになってしまっただけなのです。
もしかしたら何かやむを得ない理由があってそれらが生み出されてしまった悲しいコードなのかもしれません。
決してイケてないコードを書きまくっていたとしても、あるいは自分がクソコードを生み出してしまったとしても、コードが悪くても、その人は悪くないのです……

と、「まぁ、さすがにやらんか」というような内容が多めではあったけど、初級編はこのあたりで！
中級編は時間が許せば年内に書きます！

12月始まったばかりなので、「自分も供養したいクソコードがあるんだよな…」という方は短いものでもいいので、一緒にお焚き上げして、すっきりしましょう……