LoginSignup
87
66

自分が関わったコードの中で最凶のクソコード

Last updated at Posted at 2023-11-21

最凶コード

論より証拠、そのコードを見てみよう。そのコードとは、JavaScriptで書かれたコードである。

Original.js
t = 1234.56
   :
if(n==0){
    var a, b, c, d, e, f = t,
        w = self._iFirst,
        x = self._iSecond,
        y = self._iThird,
        z = self._iFourth;
    z.innerHTML = "0",
        f > 100 ? (a = f, b = self._getRound(a),
        d = "",
        y.style.width = "234px",
        w.innerHTML = "120",
        x.innerHTML = "240"
        )
        : (c = self._getRound(f), e = self._getRound(f / 2),
        y.style.width = (mmpix * 2 * Math.pow(2,4)) + "px",
        w.innerHTML = "120",
        x.innerHTML = "240"
        )
}
else if(n==1){
  // if(n==0)と似たようなことをしているため省略
   :  
    z.innerHTML = "0",
    y.style.width = "234px",
    w.innerHTML = "240",
    x.innerHTML = "480"
   :
}
else{
  // if(n==0)と似たようなことをしているため省略
   :
    z.innerHTML = "0",
    y.style.width = "234px",
    w.innerHTML = "480",
    x.innerHTML = "960"
   :
}
  • (注1) 題材とした最凶コードは、約130行ある
  • (注2) 変数名、関数名、メンバ名や、定数は適宜変えている
  • (注3) 変数名a~f,t,w~zは、元々違うアルファベットではあるものの1文字の英小文字が振られていた
  • (注4) tの導出もいろいろ計算をしているが結局定数になるため割愛している

お分かりだろうか。ツッコミどころしかない。というか、何から突っ込めばいいか辟易するレベルである。

リファクタリング後のコード

結論から言うとこのコードは以下のことしかやっていない。

AfterRefactoring.js
if(n==0){
    self._iFirst.innerHTML = "120";
    self._iSecond.innerHTML = "240";
}
else if(n==1){
    self._iFirst.innerHTML = "240";
    self._iSecond.innerHTML = "480";
}
else{
    self._iFirst.innerHTML = "480";
    self._iSecond.innerHTML = "960";
}
self._iThird.style.width = "234px";
self._iFourth.innerHTML = "0";

ツッコミどころ

では、箇条書きであげつらうこととしよう

  1. 変数名が英小文字1文字のみで多用している
  2. そもそも var a, b, c, d, e は定義してあろうがなかろうが、undefinedだから無意味
    • (訂正)グローバルの同名変数に影響を与えないというメリットがある
  3. 使わない変数が定義されている
  4. 使わない変数のための処理(例:c = self._getRound(f))をしている
  5. 三項演算子(? :) の2項目、3項目に複数の文を処理している
  6. f = t と同値で置き換えるため、f > 100t との比較だと気づきにくい
  7. 結局 t が定数で f も定数なので、三項演算子の2項目の処理しかしない
  8. 使ってはいないけど、a = t もあり、aft を使い分ける理由が不明確
  9. カンマ(,) を文の区切りに多用している
  10. self._iThirdself._iFourthif-else節の中で処理する必要がない
  11. Math.pow()を 2 の累乗計算で使っているのに、別で2をかけている
  12. varは変数ならlet、定数ならconstが好ましい

きっと、まだまだ言えるし、省略した部分にも不可思議なコードであふれている。

三項演算子の速度(歴史的経緯)

で、1つよくわからないことがあるのだが、三項演算子の方が if節より早いとかあるのだろうか?

検索で実験した事例を見たが、それほど変わらないという記事がほとんど。ただ、IE全盛期のJavaScriptでは三項演算子が重宝されたとかあるのかな? いずれにしても無駄な処理をしているから、速度が出ないコードには変わりない。

これ以下のコードには出会えないだろう

およそ、職業プログラマとは思えないコード。先に変数を定義するC言語の作法が顔を見せたかと思えば、JavaScriptのようなカンマ区切りの文も現れる。

言語仕様の理解も、このシステムの外部仕様の理解も何もかもが不十分、いや理解していないコードである。

レビューの体制がないというか、立ち上げにど素人が1人で進めたようなコードは、なぜか執筆現在でも運用されている。

いかがだっただろうか。これ以上のコードがあったら是非教えてほしい。

87
66
12

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
87
66