LoginSignup
117
70

More than 1 year has passed since last update.

@neonemoin株式会社ネオシステム

【精神崩壊】だからこのコードはクソなのだ！【主に俺の】

Last updated at Posted at 2021-02-11

INDEX

  • 背景
  • だからこのコードはクソなのだ！
  • あとがき

背景

Et60_3jVIAEAMQv.jpg1

かなり攻撃的なタイトルを付けたが、要はクソコードと言われるものを引き継いだり、
生産者からアドバイスを求められたりレビューをせざるを得ない状況に追い込まれ徐々に精神崩壊を起こしそうになった気がするので脳内妄想を吐き出してみることにした。つまりフィクションだ。いいね？

クソコードにも色々誕生秘話があって確かに大多数がクソ認定するコードもあるとは思うが、レビュアの理解が追い付かない事でクソコード認定されてしまう事もある。

はたまた、かつては輝いていたけど幾多の苦難2に飲まれ、多くの兵どもの夢の跡3を見守った結果、技術的観点からグリーフシード4のようにクソコードとなってしまった悲しい経緯があるかもしれん。
変に稼働実績があるからリファクタリング出来ないし、しない方が良い類のものでもある。5

まあ、そんなことはどーでも良い。この記事は

肩の力を全力で抜いて「あるある！」と笑えばいいのだ！6

だからこのコードはクソなのだ！

以下、グダグダと続く。後から何か気が付いたら追記するかもしれない。
ちなみにサンプルコードは主にJavaで書いているが基本的に言語に依存する話ではない。

1. インデントにスペースとタブが混在している

if (hoge != null) {
    for (int i = 0; i < hoge.length; i++) {
    \t  String str = hoge.get(i);
    \t  System.out.println(i + ": " + str);
    }
}
  • 何？お前、うち（レビュア）の事を騙したいの？
    • 人間は欺けてもコンピュータは欺けないので当然バグが入り込む可能性がある。
    • このコードだと、基本は4スペースインデントだが、forの中身に1段分タブインデントがある。あった…（Qiitaさんに削られたので\tと書く）
  • Pythonだと割と致命的。

2. インデントに全角スペースが混入している

if (hoge != null) {
    for (int i = 0; i < hoge.length; i++) {
        String str = hoge.get(i);
    　　System.out.println(i + ": " + str);
    }
}
  • 何？お前、うち（レビュア）の事を騙したいの？（再）
    • これでバグ出してみろ。お前はボスにも「先輩にも見てもらいました」って言い訳の報告をして、うちは連帯責任取らされるんだろ？…クソッ…クソッ…
    • このコードだと、println()の箇所に全角スペースがある
  • LL（Lightweight Programing Language: 軽量プログラミング言語）だと、実行時に初めて気づく場合も…
    • 要コンパイルな言語だったり、開発支援機能があるエディタなら事前にエラー等で分かる
    • JavaScript等言語によって使える場合もあるが、統一するならともかく混在は嫌だなあ…
      • メリーさんのパロディ7とかあるくらいだし。
  • Pythonはダメかもしれん。

3. カッコの位置がテキトー

if (hoge != null) {
    for (int i = 0; i < hoge.length; i++) {
        String str = hoge.get(i);
        System.out.println(i + ": " + str);}
    }
  • 何？お前、うち（レビュア）の事を騙したいの？（祭）
    • スコープが分かりにくいクセに動作はするもんだから、相手によっては検収通っちゃて後の祭になんだよ！
      • このコードの改修依頼とか来てみろ。うちが発狂する未来しか見えないから請けないからな！（ボスへの意思表示。頼む、頼む…）
    • っつーか、すんごいモヤモヤすんだよ。目が腐るから読む気しないんだよ。こんなん門前払いだよ。フォーマッター使えや！
  • そもそもお前、これどっからコピって来た？
    • 派生形（かどうかは知らんけど）で、インデント位置ガン無視もある。スコープって何？
  • Pythonだととても致命的。うちも致命的。

4. スペルミスを気にしない

/**
 * 名前を取得する
 * @return 名前
 */
public String getoName() {
    return this.name;
}
  • いや、確かに呼び出している箇所全部でスペルミスのまま実装すれば動くけどさ…
    • こんなん検収通っちゃった日には恥ずかしくてしょうがないんですよ…
    • だってお前、どうせこのクラスのサンプルとして、うちが＠auther neonemo8とか書いてるクラスを丸ごとコピって来て、さらに＠autherは自分の名前に書き換えていないんだろ？
      • ハハッ、getすら書けない低能野郎だってこのコードを引き継いだ会社の社員とかたまたまソースを覗いたお客さんに思われるんだろうな…クソッ…クソッ…
  • Pythonに限らず恥ずかしいから止めてくれ…。うちは自ら発した言霊9により致命傷。

5. フォーマットの足並みを揃えない

Aさん他、大多数
/**
 * 名前を取得する。未設定時は"（未設定）"と返す
 * @return 名前
 */
public String getName() {
    if (StringUtils.isEmpty(this.name)) {
        return "（未設定）";
    }
    return this.name;
}
Bさん
// 名前を取得する。未設定時は"（未設定）"と返す
public String get_name()
{
    if(StringUtils.isEmpty(this.name)) return "（未設定）";
    return this.name;
}
  • 既存ソースを無視して、独自フォーマットを使い始めてしまう
    • たしかに動作に違いはないが、運用上パッチ当てがあるソースだったりすると汎用パッチじゃなくて専用が必要になるかもしれん
    • 正直ソース解析する時に、変に気を使うので止めて欲しいし、仮にフォーマッタの使用指示が無かったとしてもこの辺の感覚は養って掴んで欲しい。
    • むしろ気に入らない書き方でどうしても変えたかったら、その旨を上司に相談し、既存のフォーマットを自力で全部直して、チームメンバーに布教活動するくらいの気概を見せて欲しい。勝手に反乱軍10を作らないでくれ…頼む…orz
  • Pythonでもこういうのは嫌だろう。

6. 似た処理を行うのに不必要に複数の書き方をする

Aさん、他大多数
for (int i = 0; i < hoge.length; i++) {
    System.out.println(i + ": " + hoge[i]);
}
Bさん
int i = 0;
for (String v : hoge) {
    System.out.println(i + ": " + v);
    i++;
}
  • カウンタも出力したいんなら、カウンタ番号で回すfor文で良いだろ。
    • 他の実装箇所はカウンタで書いてるじゃないか。何故変えた！？
    • コレクション系のforEach()とかもかな。使いどころですよ。
  • いいんですよ。基本は自由で。必要があればなおさら。ただ、特に理由もなく同じプロダクト内なのに場所によって書き方が違うのはNGだ！レビュアがそのコードに対するレビュー内訓練が出来ず、結果解析速度が落ちてしまう。
    • もっとも、書いた本人だって1ヵ月もしたら読むの大変だと思う。細かな内容は忘れてると思うので。
    • いわゆるクセの一種だと思うが、統一する事で構文解析にレビュアの脳のリソースを割かずに済み、他のコード解析にリソースを割けるのでレビューの品質が上がる。かもしれない。
  • Pythonだと…どうなんだろ？

7. 周りと文字コードが違っている

Shift_JIS「もしかして・・・・・・」 UTF-8「私たち……」
二人「蜈･繧梧崛繧上▲縺ｦ繧具ｽ橸ｼ?ｼ」
  • 5ch（https://hawk.5ch.net/test/read.cgi/livejupiter/1514990323/）より引用11
  • 菴包ｼ溘♀蜑阪€√≧縺｡�ｼ医Ξ繝薙Η繧｢�ｼ峨′隱ｭ繧√↑縺�縺ｨ縺ｧ繧よ€昴▲縺溘�ｮ�ｼ滂ｼ医ラ繝､繧｡�ｼ�
  • 指定された開発環境を使っていなかったり、どこぞからコピーして来たファイルをそのまま入れた場合にまま起きる。主な副作用は次の通り。
    • ビルドが通らない（UTF-8前提の所にSJISが混入してマルチバイト誤認したりとか）
      • Windows版Maven等で日本語のバリデーションテストをする時に良く落ちる。
    • プログラムから吐き出すテキストが文字化けしている。
    • 間違った文字コードでエディタを開いているだけならまだしも、その状態で保存なんかした日にゃ正しいマルチバイドを復元する事は極めて困難だろう。
      • 設定ファイルとか設定ファイルとか設定ファイルな！そういう意味ではネイティブなpropertiesファイル12は優秀かもしれん。
    • つーか、それライセンスとかバグとか平気？中身理解してコピってる？
  • Python縺倥ｃ縺ｪ縺上※繧ゅム繝｡縺�繧阪≧縲�

8. 指定の改行コードから別のものに変えてしまう

image.png

  • 何？お前、うち（レビュア）に目grep13でもしろと？（眼精疲労）
  • Gitで改行コード変換無しで運用するってのは割とあると思う。上記のスクショはGitHubにて改行コードのみを変更した場合のバッチファイルの差分である。
    • LFと言われているのにデフォルトのCRLFで作業していました等。
      • ファイル全体が差分として検出されるね！やったねタエちゃん！！
      • つーか、プログラム不具合起こされても嫌だから直してコミットしなおせ！
  • これもPythonとか関係なしにダメだろ…。

9. 設計通りではないが一見すると要件通りではある

サンプルなし
  • 何？お前、うち（レビュア）とボスに、次はテストフェーズだって時期に仕様と設計を変えさせてくださいとお客さんに謝罪と説得に行けと？（胃痛）
    • かなりの衝撃と笑撃（笑うしかないので…）が走る事案。心労がかさむね。
    • で、設計書に書いたクラスとI/Fを担うメソッドが存在していないのに、画面上は要件を満たしたように動いてる…
      • えっとつまり全部オリジナルって事？！
      • ある意味スゴいんだが、今はダメぇー！うちの画面から呼び出せない14
      • ああ、なるほど。JUnitメソッドに@Ignore15付けてたのか…。orz
  • Pythonとか関係なｓ（ｒｙ

FINAL. でも、動いているからOKですよね？

サンプルなし
  • だからこのコードはクソなのだ！
    • そんなわけあるか！動いているからOKってのは、障害復帰等のスピード重視な暫定対応とかの特殊な場合だけだ。初期開発や恒久対応でそれはない。…ないよな？
    • こういうプログラム改修で生産者がギブアップしてしまい、うちの所に回って来た場合、身内なら嫌がるし外部からならゼロベース開発を模索してしまうと思う。
      • 命令されたらやるとは思うけど、「このクソコード書いたの誰だよ！」16って脳内でディスる未来が見える。
  • Python（ｒｙ

Ex1. ブーメラン

image.png

  • 2021/02/13追記
  • @sohsatohさん指摘ありがとうございます。
  • 誤:auther ⇒ 正:author
  • 案の定、うちもクソでした。そんな気はしてた…知ってた…知ってた…orz
    • フリのつもりはなかったんだけどなあ。やっちまったな！
    • 元々ボケ多めなのに自分でオチまでつけてしまった。ネタ記事ですし直すのもあれなのでそのままにしておきます。

Ex2. Pythonのタブとスペースの混在ってエラーじゃないの？

image.png

  • 2021/02/13追記
  • マンガの方のツッコミがTwitterの方にあったのでネタ晴らしをすると、2.7では、1タブが8スペース換算で動きます。3.8は混ぜるな危険と怒られますがががｇ
    • TabError: inconsistent use of tabs and spaces in indentation

あとがき

どうだっただろうか。
今回はコード解析やレビュー時に脳内ツッコミしている点などを書いてみた。
やり玉に挙げているのは業務ロジックの話まで行けない門前払いなソースである。

最近はレビューやプロダクト引継ぎとして他人のソースを読むことが圧倒的に多くなった。
まあ、引き継いだ方はしょうがないので解析するが、レビューの場合、本来は業務ロジックとかそういった方に力を注ぎたいのだが、ソースが読みにくい事が多くてそっちに気が回らない。集中が分散するので、レビュー品質が落ちてしまう。

そんな鬱憤が溜まってて爆発したのがこの記事でした。orz

とりあえず、初心者を自覚している人はコードフォーマッタを使う所から始めると良いかと思う。
Visual Studio CodeやEclipse等にもフォーマッタが組み込まれているしカスタマイズも出来る。
人間はテキトーだからね。エディタで手動調整するくらいなら先人の知恵が入ったツールは使いこなせばいい。

それにしてもこんな記事書いて怒られないかな…（ドキドキ
（2021/2/14追記）やべぇ…デイリートレンドに載っていいねが増えまくってしまい、タグランキングがクソコードの弊社みたいになってる。ボスに目を付けられるかも…やべぇ、やべぇ…。17

以上！

  1. 誤:printf(f'～') ⇒ 正:print(f'～') 記事を推敲しないと修正だらけになる良い反面教師 ↩

  2. 経営戦略とかそういうの。スマホの料金プラン辺りをイメージするといいかも。 ↩

  3. この場合は経営戦略的無理難題吹っ掛けられた結果、仕様が複雑化したシステムに立ち向かうエンジニアというニュアンス。 ↩

  4. マドマギのあれ。 - グリーフシード - ピクシブ百科事典 ↩

  5. そうは言っても引き継ぐ方は溜まったもんじゃないので何とかならないもんか…。 ↩

  6. これ（こんなログは嫌だ！ - Qiita）と同系統の記事だしね ↩

  7. 私全角スぺース、今あなたのソースの中にいるの - このページ（https://corobuzz.com/archives/43674 ）のNo.8。 ↩

  8. JavaDocの一種で作成者を表すもののつもり。文中はtypoで本当は@authorが正しい ↩

  9. 神「authorの綴りが間違えているのはツッコミ待ちですか...?」俺「ああああああッ！」 ↩

  10. 「のばら」「きさまら　はんらんぐんだな！」って帝国兵に襲われるあたり帝国に情報漏洩しているのは明らかなので反乱軍の情報セキュリティのリテラシーは低い。 ↩

  11. Shift-JIS⇔UTF-8間の文字化けのパロディ。実は文字化けしている方にも意味がある。興味があるなら文字化けを解消したものを調べてみて欲しい。消えた情報自体は戻らないが正しくデコードできれば読めるようになる事もある。手練れてくると「糸偏が多いから元がSJISでUTF-8にしちゃったか？」等の勘が働くようになる。 ↩

  12. Javaのプロパティファイル（*.properties）は、ASCIIで書かれるので基本的に文字化けしない。 ↩

  13. diffやgrepコマンドが役に立たない時に発揮するいぶし銀の業。老眼にはキツイ。目grepとは？ - https://atuweb.net/201808_about-me-grep/ ↩

  14. 要件通りじゃなかった箇所orz ↩

  15. JUnitはJavaのテストツール。@Ignoreはテストケースを一時的に実施しないようにするアノテーション。 ↩

  16. そしてそれは往々にしてブーメランにもなる。なった（記事内で実証済み） ↩

  17. …とか書きつつ、こういうイレギュラーな奴もある程度取り込める組織なので居心地良いですよ。 ↩

117
70
16

Register as a new user and use Qiita more conveniently

  1. You get articles that match your needs
  2. You can efficiently read back useful information
  3. You can use dark theme
What you can do with signing up
Sign upLogin
117
70