本当にあった怖いプログラム（クソコード事例集）

プログラマーはクソコードが大好きです。プログラマーが仕事を進める上でクソコードは本来全力で回避するべき対象であるにも関わらず、プログラマーはクソコードの話題が大好きです。みんな張り切って自分が経験してきたクソコードの話で盛り上がります。

昨日こんなツイートをしました。

プログラマーとして数年経験していれば誰でもクソコードの１つや２つ見たことあると思いますが、今まで「これはクソ」と思ったコードにはどんなものがありましたか？リプ欄で共有いただければ幸いです。

— 米村歩@日本一残業の少ないIT企業社長 (@yonemura2006) April 26, 2018

そうしたら出るわ出るわ。皆さんのクソコード事例集があっという間に集合知として蓄積されていきました。エンジニアであれば誰しもクソコードの話題では盛り上がらざるを得ないわけですが、今回上記のツイートを私はある意図があって投稿しました。

その意図とは、多くのエンジニアが経験してきたクソコードの事例を体系化してまとめることによって、その貴重な経験・知識をこれからプログラマーとして成長を目指す経験の浅いエンジニアのスキルアップに活用することができるのではないかと考えたからです。

今回数え切れないほどのクソコードの事例を情報としてご提供いただいた皆様、本当に助かりました。ありがとうございます。あまりにも数が多すぎてうまくまとめられているかどうかちょっと自信がありませんが、クソコードの種別ごとにグルーピングしてみました。

経験が浅くてこれからプログラマーとして成長を目指すエンジニアの皆さんは、ぜひこのクソコード事例集を参考としていただきまして、「こんなプログラムソースコードを書くとクソコード認定されるんだ」と知っていただき、今後のエンジニアライフにお役立ていただければ幸いです。

また新人プログラマーを育成するポジションにいる先輩プログラマーの皆様におかれましても、やってはいけないクソコード事例集として、後輩プログラマーの育成にお役立ていただければ幸いです。

命名規則に関連するクソコード

クラス名、メソッド名、変数名などのネーミングを誤るとクソコード認定されてしまいます。会社やプロジェクトごとに多少のルールの違いはあるにせよ、どこに行っても漏れなくクソコード認定されてしまうネーミングパターンのご紹介です。

ネーミングが「記号＋番号」

クラス名や変数名はわかりやすい名称にしましょう。ネーミングを見て内容を推測できるようになっていることが重要です。「記号＋番号」ではそれを見るだけでは何のプログラムであるかを推測することは不可能です。

ネーミングに日本語、英語、ローマ字が混在

プロジェクトによってクラス名や変数名のネーミングルールは異なりますので、何がダメだというわけではありませんが、自由すぎるネーミングを行うのはやめましょう。きちんとプロジェクトでルールを統一することは重要です。

またにクラス名や変数名に日本語を使用することは言語仕様上可能とはなっておりますが、アルファベットを使うことが慣習となっていることと、日本語だとIDEの補完機能がうまく機能しないことがあって非効率化の原因となりますので、避けた方が無難です。

ネーミングにスペルミスがある

ネーミングでスペルミスがあると、後でソースコードから文字列で該当箇所を検索する時に検索にヒットせず、改修漏れの原因にもなります。正しいスペルと間違ったスペルが混在していたりするともう最悪です。スペルミスのないように気をつけましょう。

ネーミングに個人名が使われている

ネーミングはプログラムの中身がわかるような名前にするという観点からも、プログラムの中に自分の名前にすることは適切ではないのでやめましょう。

またソースコードレビューの時に思いがけず恥ずかしい思いをすることになるかもしれません。私は新人の時に「yonemura.sh」という名前で自分用に作ったシェルが他社に買い取られることになってしまい、他の会社のエンジニア２０名くらいの前で「よねむらシェルとは・・・」と説明会で大きな声で読み上げるはめになって大変恥ずかしい思いをしたことがあります。

個人で使うプログラムでもプログラムの中身を表した無難なネーミングにしておくことを強くお勧めします。

ネーミングに番号やアルファベットの連番が使われている

クラスや変数のネーミングに、１からの連番やaからの連番を使うと、クラスや変数の中身を推測することが不可能になってしまうのでやめましょう。こういうことをすると後でそのプログラムをメンテナンスする人に、一々プログラムの処理を細かく解析することを強いることとなり、「このクソコード書いたやつまじで氏ね」と言われてしまいますのでやめましょう。

可読性に関連するクソコード

プログラムは後でメンテナンスするためにも、読みやすく書くことが非常に重要です。処理の内容だけ見ると読みやすくても読みにくくても実行される内容は同じかもしれませんが、読みやすいソースコードは改修の工数を下げますし、バグが混入するリスクも下げてくれます。

ネストが異様に深い

ソースコードの中にネストが何重にもなっている箇所があると可読性を下げてしまいます。ネストを何重まで許可するかはプロジェクトによって異なりますが、個人的には３重か４重くらいまでにおさまるようにコーディングするよう心がけていました。

これとセットで「１行の文字数は８０文字まで」みたいなコーディング規約があるとさらにカオスな感じになってきます。ネストが１０階層＋１行８０文字までとか、考えただけでも嫌になりますね。

インデントがずれている

今どきエディタが良い感じにインデントしてくれるのに、まさかインデントがずれているソースコードなんて存在しないと信じたいところですが、昔作られたソースコードだとそういう化石みたいなクソコードにお目にかかることはあるようですね。

カッコの閉じ位置のインデントがズレていたりすると、著しく可読性を下げますし、コードの解析を誤るリスクも増えてしまいます。こういうことをすると漏れなくクソコード認定されてしまうでしょう。

１つのメソッドが異様に長い

たまに１つのメソッドが異様に長いソースコードにお目にかかることがあります。私の個人的な感想だと某国にオフショア開発に出されてウミガメのように日本に帰ってきたソースコードにそういうメソッド分割の概念が消失してしまったかのようなソースコードが多いように思います。

１つのメソッドの長さが数千行にも及ぶような男前なソースコードにバグが混入してしまい、解析及び改修をしなければならなくなった時には絶望するしかありませんね。

保守性に関連するクソコード

クソコードであること自体が保守性を著しく低下させているわけですが、その中でも特にプログラムの保守性を低下させる事例を集めました。保守性が下がると後でバグ改修や機能追加の工数が大幅に上昇してしまい、新たなバグ混入のリスクも高めてしまいます。

マジックナンバーが使われている

プログラムを書いた人にしかわからない謎の数値がプログラム内に直書きされているものをマジックナンバーと言います。後でプログラムを見た時に何の数値か見ただけでわかるように、わかりやすい定数名を作成してそこに値を入れてプログラム内で使用しましょう。

その時に「ONE = 1」とかは結局意味がわからないのでやめてくださいね。「JANUARY = 1」のようにネーミングには意味を持たせましょう。

絶対に実行されないコードが残っている

プログラムを開発・メンテナンスしていると、開発の途中で一度は作ったけども、結局使われなくなってしまったコードが出てくることがあります。使われないコードを消すのが面倒くさかったり、いつか使うかもしれないと思って残しておくことは有害ですからやめましょう。

使われないコードを残してしまうと、何かの改修や調査が必要になった時に、使われないコードまで調査の対象となってしまい非効率です。どこからも使われず、実行されないコードは速やかに消しましょう。

Viewとロジックが分離されていない

どんな構成でシステムを構築するのかについてはプロジェクトの方針によっても異なりますが、例えばよくある一般的なMVCモデルでプログラムを作っていく場合、View部分に複雑なロジックを混在させることはクソコードの鉄板となります。

各モジュールには役割があり、モジュールごとに役割分担させることによってプログラムの保守性が上がり、品質も向上させることにつながります。間違ってもViewにSQLを書くような神をも恐れぬクソコードを書くのはやめましょう。

ログに役立つ情報が出力されていない

優れたシステムというものは、ログに必要な情報がスマートに出力されるように構築されているものです。そのようなシステムでは万が一システムに障害が発生したとしても、障害原因を最短で速やかに特定することができ、システムの保守性が格段に上がります。

これがダメなシステムだとログにまともな情報が何一つ出力されていないことがあります。システムエラーが発生したのに「log.error(“エラー”)」とだけログに出ていた時には軽く心の奥底からこみ上げてくるものがありますよね。

ログはシステムを運用していく上で非常に重要なものですのでまともなログ出力を心がけましょう。

再利用性に関連するクソコード

プログラムを書く上で「同じことを２度書かない」という原則は重要です。また機能追加になった時でも、既存のコードに手を入れないように既存のコードを再利用できるように作ることは重要です。

コピー＆ペーストがフル活用されている

既存のプログラムをリスペクトしてコピペすることは別に悪いことではないのですが、プログラマーである以上は「同じことを２度書かない」という原則は念頭に置くべきです。コピペするということは同じコードが複製されるということです。同じコードが２つになるということは、そこに改修が入った場合も２箇所改修が必要で、それに伴って試験も多く必要になるということです。

コピー＆ペーストする時は、本当にコピペして良いケースかどうかを良く考えましょう。何も考えず安易にコピペするような人はプログラマー失格と言っても過言ではありません。

共通処理が共通化されていない

共通処理については一箇所にまとめて共通部品化するようにコードを書くべきです。面倒くさいからとか、既存のコードに触れたくないからとか、そういうプログラマーの怠慢によって共通処理なのに共通化しないということが繰り返されると、１つのシステム内に同じ処理が何箇所にも書かれているということが起きてしまいます。

同じ処理が何箇所にも書かれているということは、そこにバグが発生した時に改修する箇所も何箇所もあるということです。試験しなければならない箇所も何箇所もあるということです。１箇所だけ改修したけど、他の箇所では改修漏れとなるリスクが生まれるということです。

共通モジュールの仕様がイケてない

共通モジュールを作成することは良いことですがその仕様がイケてないと、そのモジュールを使わなければいけない人達に多大なる迷惑をかけることになってしまうかもしれません。よく使う共通モジュールであればあるほど、その設計には細心の注意を払って使いやすくなるように心がけましょう。

共通モジュールを呼び出すために必要以上に余計な処理が増えてしまうので、誰もその共通モジュールを使わずに独自実装してしまうなんて笑えない話もあります。共通モジュールの仕様がイケてないと、プロジェクトメンバーからの大きな反感を買ってクソコード認定されてしまうでしょう。

コメントに関連するクソコード

直接プログラムの動作とは関係がないとは言え、今回ソースコードのコメントに関連するクソコード情報が数多くありました。動作とは直接関係なくても、プログラムを改修したりメンテナンスしたりする上でコメントは非常に重要なものだということです。たかがコメントだと思って適当に書いているとクソコード認定されてしまうので気をつけましょう。

コメントもドキュメントも無い

コメントの内容が適切ではないとかそういう問題の前に、コメントが一切記載されていない男前なシステムも世の中には存在するようです。趣味でやるならともかく仕事としてチームでシステム開発を行うのであれば、適切なコメントを残すことはプログラマーの重要な仕事と言えます。

趣味で１人で作るようなプログラムであっても、後で自分で見た時に「あれ？これ何だっけ？」となることは結構よくありますので、１人開発の時もコメントは適切に残すことをお勧めします。

見ればわかるレベルのことがコメントで書かれている

プログラムにコメントを残すことは大切ですが、わざわざコメントにしなくても良いようなことをコメントにすればそれはたちまちノイズとなってしまいクソコード認定されてしまいます。

例えばfor文の前に「//ループ処理」みたいなコメントなんて書かなくても、誰が見てもループ処理してることなんてわかりますのでそういう無意味なコメントを書くとゴミとなるだけなのでやめましょう。

他社のコピーライトが書いてある

他社からシステム保守を引き継いだとかそういう理由でもないのに、突然自社で開発したはずのシステムに全く関係ない会社のコピーライトが出現するような事例があるようです。クソコードとかそういう話以前に、普通に法に触れる行為である可能性が高くて草も生えません。

不要になったコードがコメントアウトで残っている

不要になったコードがコメントアウトで残されているケースは多々あるようです。バージョン管理システムが使われていなかった時代であればともかく、今はそういうことをしても邪魔になるだけですので、不要になったコードは速やかに削除しましょう。

修正履歴がコメントで管理されている

これについても今は修正履歴は細かくバージョン管理システムで管理されますのでコメントで残す必要は全くありません。邪魔なだけです。自分のところのプロジェクトではバージョン管理システムが導入されていないけど？みたいな人がいたら、それは別の意味で色々ヤバイです。

コメントに嘘が書いてある

適切に残されたコメントはシステムのメンテナンスに役立ちますが、どういう経緯かわかりませんが時々嘘のコメントが書かれていることもあるようです。有害すぎて即クソコード認定される事案ですのでやめましょう。

コメントに個人的な愚痴が書かれている

プロジェクトが炎上したりすると色々と辛いですよね。気持ちはわかりますがプログラム内に個人的な愚痴を書くのは後々まで履歴も残りますしやめておきましょう。愚痴を吐きたければTwitterあたりでやると良いのではないでしょうか。ｗ

コメントが全てフランス語

フランス語や中国語など、自分には読めない言語でコメントが残されていることがあるようです。日本のプロジェクトであれば日本語でコメントは残してもらいたいですし、せめて英語ですよね。フランス語や中国語のコメントを翻訳サイトに通すよりもコード読んだ方が早いかもしれません。

プログラマーを震え上がらせるコメント例

いくつか世にも恐ろしいコメントの具体例をご紹介します。恐ろしいですね。

とりあえず動いているので触らないこと
TODO 余裕ができたら作る（←本番稼働中のプログラム内）
もうどうしようもできない
動いている理由は不明
エラーが出ているのでコメントアウト
ここ何しているか不明

その他のクソコード

既にご紹介した以外のクソコードの事例をご紹介します。

常にtrue（またはfalse）にしかならないif文

何のためのif文なのかわかりませんね。こういうコードを書いてしまう人はif文の意味から勉強し直す必要がありそうです。

defaultしか書かれていないswitch文

これもなぜswitch文を使ったのか意味不明なので、プログラムの基本文法から学び直す必要がありそうです。

例外を全てキャッチして握りつぶしている

せっかく適切に例外をスローしてくれているモジュールを呼び出しても、呼び出し元でこういう理不尽なことをされると全部水の泡です。例外は適切に処理しましょう。

ゲッターでグローバル変数を変更している

プログラム言語仕様上はそういうことが可能だからと言って、ゲッターやセッターには意味があります。呼び出す人はまさかゲッターを呼び出しただけで値が変わるなんて思いませんから、そういうバグの温床としかならないようなことをやるのはやめましょう。これと類似で、toStringで値を書き換えるというクソコードもあります。