🎉

結局「クソコード」ってなんなの？

2023/12/31に公開

クソコード

tech

年末なのにzennを読んでる勉強熱心なみなさんこんばんは

Twitterで「クソコード」が盛り上がりまくってますが、折角なので自分が考えるクソコードがなにかを書いておこうと思います

ちなみにこの「クソコード」には3つの観点があると思っていて、それぞれの視点で何がクソコードなのかを紹介します

※クソコードと言いつつ全てはトレードオフなので時間的制約などからそれを選択せざるをえない事は良くあります

1. コード的なクソコード

こちらはシンプルに目の前のエディタを叩いてる時に遭遇するクソコードで、多分一般的にクソコードと言われて想像しやすいものがこれだと思います

脳内メモリをフルに使うコード

よく題材に挙げられるのが三項演算子のネストやif文のネストなどですね

if a > 1
  if b > 1
    if c > 1
      if d > 1

let result = a > b ? c > d ? true : a > d ? true : false : a > d ? true : false;

その他にも個人的には以下のようなunlessを更に反転させたりelseを使ったコードは頭がこんがらがるので嫌いです

unless flag1 || flag2
else

こういったコードを読めるかは「その人の脳内メモリの量」に大きく依存します。僕は脳内メモリがクソ雑魚なので読めませんが読める人にとっては「何がクソコードなんだよ」と言われるタイプの辛いアレです

凡人からすると、「1923 * 1893 の計算を脳内でやってくれ」と言われてる感じですね。もちろんミスも発生しやすくなるので出来るだけ僕のような凡人のために安易に書いて頂けると嬉しいです

秩序を破壊しているコード

array = [123 ,456,   789  ]
flag      =true

こういう秩序の無いコードを見るとエンジニアは吐血して死ぬので秩序を保ってください

# なんでもいいけどリズムをあわせて
array = [123, 456, 789]
array = [ 123, 456, 789 ]

最近ではlintなどでルールを機械的にチェックしてくれる場合が多いのでこういったコードを見ることは少なくなりましたが、コピペでコードを書いている人などは消すべき部分がわかっていないのでとりあえずそのまま～というパターンで上記のようなコードが生み出される事が多いです

個人的にはjavascriptやhtmlなどでよく見かける気がします

<div><p>
	                                          テキスト
	</p>
                           </div>

その他にも

改行が全然ない
コンポーネントが無駄に分割されまくっている
コメントが全く無い

など色々あると思いますが、まあそんな感じで「理解し辛い」「一般常識から外れている」「脳のメモリを多く使う」「秩序が無い」あたりがキーになってくるのではないかと思います

※良いコードというのはある程度収束するのですが、クソコードというのはあの手この手で新種が出るので網羅することは難しいです

2. 運用保守の面から見たクソコード

ここらへんは視野が一段階上がり、一度書かれたコードを長期的に保守運用していく観点でクソコードと呼ばれるものです

最初に書いた時には意識されない場合が多いのである程度このフェーズを見据えて書ける人は経験値が高いエンジニアだと言えます

テストが無いコード

テストがある場合は何かしらの修正をした際に「このコードが書かれた段階で求められていた挙動をクリアできているか？」を確認することができます

テストがない場合は過去の資料を漁ったり既存の実装を確認して正しい動作が何なのかを検証しながら進めないといけないのでそれだけで大幅にリスクが上がるためそれだけで運用保守の観点からは「クソコード」と呼ばれます

スタートアップなどでは初期の段階ではテストを書く時間を開発に当てる！という方針を取るチームも多いので一番遭遇しやすいのがこれなのではないかと思います

変更の可能性を意識していないコード

よく上がるのが俗に言うマジックナンバーなどですね

下記のように同じ目的の同じ数字「3」をベタ書きで書いたりすると

if array.length > 3
  puts "1ページで収まりません"
end

page = array.lenght > 3 ? array.length / 3 + 1 : 1

例えば1ページの件数を「4」に変更しないといけない時に3箇所も変更が必要になりますし、メソッドが長いと見落とす可能性があり変更容易性が低いコードとなります

こういったものは最初に書いた時点では別に切り出す必要がない（ファーストcommitでは比較が1箇所だけだったなど）ことも多く、しばらく運用していく上で何箇所かに分散し次に数値を変えることになり修正漏れによるバグが発生して初めて気づかれるタイプのクソコードです

クソコードとは言いつつ上にも書いた通り経験しないと避けることが難しいのである程度こういう地雷を何度も踏んで経験をつんだシニアエンジニアは「あ、ここ将来バグに繋がりそうだな」と考えて避けて書いてくれます

スケールしないコード

「ユーザー数が増えてから遅くなってしまった」なんて話は良く聞くと思います。経験のあるエンジニアであればコードを書く際は「将来的に何人くらいのユーザがMAXで、どれくらいのデータ数を想定していますか？」などの話をします

例えば有名な話ですが、Twitterのフォローフォロワーのタイムラインを設計する場合にRDBで愚直にやると即破綻しますがみんながフォロー数1000くらいまでであれば騙し騙し使えるのでリリース時点ではサクサク動いても1年後くらいに致命的なまでに品質が落ちてしまう地雷となります

一般的に踏みやすいのが以下のようなDB起因のものですね

RDBに適切にindexを貼っていない
N+1問題を起こしている

データ量が増えた先の世界はもう何もかも地雷になるので、これもある程度大きなサービスを触った経験が無いと予め想定するのは不可能に近いので経験が物を言う世界です

チーム開発を意識していないコード（環境）

最近ではVS Codeにremote containerを入れてDockerを用意すれば新入社員もgit cloneして1分で環境構築が終わるというお手軽な世界になっているのですが一昔前は

rbenvを入れて
installして
ライブラリを入れて
環境特有のエラーが出て

と、環境構築するだけで3日とか下手したら1週間なんて場合もありました

他にもlintが整備されていなかったり、チーム開発が意識されていないコード（環境）はそれぞれのエンジニアのパフォーマンスが出しづらく本質とは全く違う余計なところ（環境依存など）で時間を取られるのでクソコー……どう頑張ってもコードではないですね。はい。まあよろしくない環境だなーと思います

3. チーム開発や社内政治を考えたクソコード

ここらへんまで行くとCTOとかテックリードとかそういう人が求められる視点だと思うので駆け出しの方などはあまり意識することは無いかもしれません

もはやクソ”コード”なのか？と思いつつ折角なので書いておきます

新しいエンジニアの採用を著しく難しくする技術を使うクソコード

GOが流行ってるので使いたいです！
Railsのデフォルトはイケてないので最高のアーキテクチャ考えてきました！
あんまり使われてないけど海外では著名なxxxってフレームワーク使いたいです！

エンジニアというのはしょっちゅう転職します。転職しなかったとしても1人のエンジニアに依存した状態は避けるべきなので常にバッファを持たせたいです

そんな時に「転職市場にエンジニアが溢れていない技術」を用いてしまうと、普通の技術で普通にやっている会社の方にエンジニアは流れます

他より高い給料を出すか、逆に先鋭化させて「他では使えない技術を使えます！」とアピールするか、何にしろエンジニアの確保が非常に大変になりますのでそれだけでクソコードと呼ばれます

上層部が責任を取らないといけないクソコード

～なぜ人はSaaSを使うのか？それは責任を取りたくないから～

決済部分や顧客管理部分などのコアな機能でSaaSを使う企業がなぜこれほどまでに多いかと言うと、なにか問題が起こった時に責任を取りたくないからです

クレジットカード情報が流出した
個人情報が流出した
セキュリティの穴があって攻撃された

そんな時に「外部のxxという会社のツールを使っていたのですがそちらの会社が～」と責任逃れ出来る（し、割とそれで許されてしまう）ので保身のためにSaaSを使いたがる上層部も一部存在します（それが別に悪いことだとは別に思いません）

そんな時に注意しないといけないのが「Auth0なんて聞いたこともない怪しいサービスを使ったら俺の責任になるだろ！」と怒られるので、聞いたことあるか上場してるような会社か他社で採用事例のあるSaaSを使いましょう

もはや何の話？と思うかもしれませんがそういう知識があると技術選定の段階で自分で書くという選択肢以外の選択肢を常に意識することができます

ベンダーロックインされるクソコード

よくあるのがパフォーマンスが落ちたことによるRDBの切り替えとか、安くするためにAWSからGCPへの移行とか、社長が喧嘩してマーケティングツールを切り替えるとか、まあ色々社内政治だったりコストを下げるためだったり、やむを得ない理由で切り替え作業が発生する事があります

そういう時に「特定のベンダーに依存した処理」が多いと当然ながら移行し辛いです

例えばRDBで言えばデータをランダムに取得する場合MYSQLならRAND()ですが、PostgreSQLならRANDOM()です。ORMで抽象化されている.random()のようなものを使っていれば書き換えなく移行できますがそこらへんを意識せずに書いてしまっていると移行したら動かないという地雷になります

もちろんある程度は仕方ないのですが、AWSを使っていても抽象化されたTerraformをコストをかけても使うとかそういう事を考えながら書くとフットワークの軽いコード？が出来上がります

まとめ

なんか途中から自分でもコード？？って感じになってましたが、それぞれの視点によってクソコードと言っても全然ちゃうよなーとのりで書いたので許してください

ちなみに僕の言う「クソコードの方が早く書ける」というのは

他のエンジニアが触ることを想定してきれいに書かないで良い
将来性とか考えず自分が使いやすい技術だけを使って良い
テストも書かずエラーが出ても、出てから治すという場当たり対処で良い（将来困るけどクソコードだからヨシ！）
他のエンジニアには受け入れられない「日本語変数名」をガンガン使える（脳内メモリが大幅に節約できる）
ライブラリなどで古い書き慣れたバージョンを使って良い（企業開発なら常に出来るだけ最新版を入れる）

とか、そこら辺が早く書ける理由だと思ってて「普段なら他人に触らせたり保守されていく事を想定して気をつけている部分を全く気にせず書いて良いから早い」という感じです

もう少し実例を上げると

erbではなくhamlで書く（非標準技術はエンジニア確保が難しくなるので採用しづらい）
tailwindを入れてオレオレCSSをパイルオン（他の人が使うならドキュメントを書かないと行けない）
svelteでパーツを書きなぐる（実務でsvelteはエンジニア採用に響くから使えない）

とかですかね。本当に自分が書きやすい/作りやすいだけを意識した構成で他人の事を考えなくて良いので早い！が、自分よがりなそれを人はクソコードと呼ぶので仕事ではできないわけです