5
1

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?

More than 1 year has passed since last update.

限界開発鯖Advent Calendar 2023

Day 9

クソコード鑑賞しようぜ!!!

Posted at

スライド

これは今日のLTで喋ったものなので

はじめに

  • Done is better than perfect
    • きれいな設計はいい、あるにこしたことは無い
    • けど動かないコードは意味がない
    • どんなに汚いコードでもお金になったり、有名になったりする
    • もちろん、動いてかつきれいなコードが素晴らしい

クソコードって何?

結局主観によると思います。最近は「保守しづらいコード」がある程度その範疇に入るのでは?と個人的には考えています。
具体例で言うと以下のようなコードでしょうか。

string sql = 'select hoge from huga where 1=1'

if(condition1) sql += 'AND con1 = ' + form_param_con1;
else if(condition2) sql += 'AND con2 = ' + form_param_con2;
//...
exe(sql);

(実際のコードではないので文法は雰囲気で)

このコードですが、真っ先に思いつくのはSQLインジェクションの可能性があったりとか、条件分岐でゴリ押しでかいけつしたりとまぁ色々あるとおもいます。

動いているクソコードは正義でもある

ただ、僕もプログラミングやりはじめには誤解していましたが、動いていれば正義です。
どんなに机の上の紙で書かれた素晴らしい構成図があっても、実際に動かないんじゃクズ紙です。なので動いている時点ですごいので、まずはそこを抑えた上で、動いているクソコードを見て、クソコードたるところを感じてくれたらと思います。
「動いてるからすごいね、糞だけど!」の精神でお願いします。

鑑賞会場

この作品のプレイはこのリンクからできます。ただWebGLサポートされてるブラウザ必須なのでご注意ください。

僕が思ってないだけでクソコードポイントたくさんあると思うので、思いついたらコメントで書いていただけるとこの記事がアンチパターン集になってWin-Winなので書いてくれるとすごく嬉しいです。

クソコードポイント

一つのコンポーネントに全部詰め込んでる

ハッカソンの作品なんですけど、いくら時間がなかったとはいえ710行のコンポーネントは流石にまずくないですかね
image.png

このRepoは3人称弾幕STGなのですが、せめて

  • ゲームの環境設定
  • UI
  • エージェント
    • 共通処理
    • 自機
      • ボス
  • 描画関連

のように分けたほうが良かったんじゃないですかねぇ

例えばVCS目線で言うとログの概要を見るときに全部「hoge.tsx」の更新としか書かれていないため、ちゃんとDIFF表示させるまでどこが変わるのかがわかりにくいです。

固定長単位、やめない?

一部抜粋ですが

canvas.tsx
            const congrats =new Image("congrats", "/Congrats.png");
            congrats.width = "800px";
            congrats.height = "284px";
            congrats.top = "-334px";

            const hpBar = new Rectangle("hpBar");
            hpBar.width = "800px";
            hpBar.height = "35px";
            hpBar.top = "300px";
            hpBar.background = "green";
            advancedTexture.addControl(button1);
            advancedTexture.addControl(logo);

            //dialog box
            const dialogBox = new Rectangle("dialog");
            dialogBox.width = "60%";
            dialogBox.height = "30%";
            dialogBox.background = "rgb(0, 15, 69)";
            dialogBox.thickness = 2;
            dialogBox.color = "white";
            dialogBox.alpha = 0.3;

ユーザーによってブラウザの画面幅違うのに、この人自分のブラウザだけでプレイする前提で自機でしかデバッグしてないからこんな固定長単位でやるくせになぜか割合単位も使ってますね、滅んだほうが良くないですか?

ブラウザ系で表示するならemとかv*を使うのがいいと思います(とはいえフロントちゃんとやってないので補完してほしい有識者に)

WhatとHowを混ぜるな

ゲームにおける描画とロジックがごっちゃになっており、これはさっき書いたファイル分割してないことにも繋がりますが「適切な役割分け」が改良したりするときに便利になるのでちゃんと分けましょう。

役割が違うものが交互に書かれていたりすると、変更するときにし辛いです。なので、一つのファイルに書かれるとしてもよほどの理由がない限りは混ぜるように書かずにするべきでしょう

待った!!その技術、本当に作品に適していますか!?

様々な要因でVercel+Next.js+Babylon.jsで作りましたが、そもそもWebGL系とNext.jsは相性が悪いように感じました
というかゲームとApp Router、ちゃんとわかってないと相性が悪くなる気もします。
"use client"してないとかのせいで開発時に2hくらい溶かしたので本当に選定に関しては吟味しましょう。

どうして仕様がないんですか?

ハッカソンの性質上ある程度は仕方ないんですが、これを継続させるための作品として考えると、仕様がどこにも残ってないので誰かが改良しようとしたときに信頼できるソースがこのクソコード1つだけになるというかなりまずい状況です。(というか久しぶりにコード見て絶望しました 700行も見れないですよ私())
なので仕様は書こうね!!!!!!!!!!!!!

5
1
0

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
5
1

Delete article

Deleted articles cannot be recovered.

Draft of this article would be also deleted.

Are you sure you want to delete this article?