本当にあった怖いAS3
以前見かけた怖いAS3のプロジェクトの話です。
一年ばかり前につとめていた会社で開発されていたすごいActionScript3のコードについて書いてみようかと思います。倒産したので文句言ってくる人もいないはずだし。反面教師としてお役立てください。
ちなみにプロジェクトのリリース後から入社したので、これらのソースコードの制作には私は関わっていません。
4000行あるクラス
ある日、隣の席の同僚Aとの会話。
A:「もうこれのデバッグ嫌なんですよ」
私:「なんで?」
A:「このクラス、4000行もあるんですよ?」
私:「4000行?1行目に書いたことが4000行目に効いてきたりするんやろう?そんなん人間の力で管理できるかいな。誰がかいたんやそんなソース。外注の人?」
A:「Kさんです」
Kさんは私の目の前に座っている上司でした。もちろんこの話も聞いています。
そんな都市伝説みたいなベタなオチに巻き込まれるとはね。
上司は度重なる仕様変更のせいでそうなったと言ってましたが、どう考えても外部swfを読み込んで初期化するだけのクラスが4000行になるまで肥大することはまともなプログラマならあり得ません。
しかし、部下に自分のコードをデバッグさせる上司は何を考えてるんでしょう。無能力さを見せつけた上にと尻ぬぐいをさせては、絶対その部下はその上司のこと尊敬してくれなくなるだろうに。
仕様書
仕様書を書いて上司にチェックしてもらった。「仕様書ってこう書くんですね!」って言われた。
謎のコメント
人のソースをデバッグしてたらこんなコメントを見た。
/*▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲*/
/*▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲▼▲*/
書いた本人に意味を問いただしたかったが思いとどまって見なかったことにした。
謎のコメント2
// かつおちゃんてきとーばーじょん
というコメントをリリース済みのソースコードのある関数の前に見つけた。
とりあえず継承
リリース終わって一ヶ月経った頃、共通ライブラリを見ていたら、MovieClipやSpriteの機能を一つも使っていないのにMovieClipやSpriteを継承しているクラスを大量に見つけた。
共通ライブラリなのに全部private
共通ライブラリのクラスを継承しようとしたらほとんどの関数がprivateだった。
だいたいstatic
どうも作った人がオブジェクト指向を理解していないようで、共通関数の多くはstatic関数でした。継承できないのでどうにもならない。しかもstatic関数ばかりなのにクラス自体は前述のようにMovieClipとか継承している。もちろんfinalとか使わない。
コンストラクタにベタうち
共通ライブラリのサーバー接続クラスを継承して、テスト時にはサーバーに接続しないようにしようとしたら、接続部分がコンストラクタにベタうちだった。
カスタムイベント
共通ライブラリでカスタムイベントを使わす、コールバック関数ばかりでしかもそれをクリアする手段を用意していないから、mcがremoveされたり、swfをアンロードしたりした後にイベント発生するとnullエラーでまくるとか。
Main大好き
ほぼすべてのソースのドキュメントクラスが「Main」と言う名前でした。しかもデフォルトパッケージで。というか、クラスの大半がデフォルトパッケージ。swfを互いに読み込んだりするのに。名前問題とかどうなってたのだろう。
AMF
サーバーとの通信の一部はAMFを使っていました。
AMFは送受信データにバイナリが使えて、クラスマッピングでクラスのインスタンスがそのまま送れるところが良いところですね。
まあ、私が見たソースでは、なぜかすべての数値データをテキストに変換して、100項目以上あるデータをクラスマッピングせずにオブジェクト型で送ってましたが。
100項目設定するのに5050回比較する
100項目ぐらいのデータを設定するのに、なぜかループとcase文を使って
for(var tmp:Stirng in data){
switch(tmp){
case “A”:
s001_txt.text=data[“A”];
/*以下一〇〇個ぐらいのcase*/
}
}
とやっているのを見た。たった100項目設定するのに等差級数の和の公式から(100)*(100+1)/2 = 5050回の比較が行われる事になる。しかも変数名通番だから1つでも減ったり増えたりすると飛びが出て妙なことに。
テキストフィールド名を変数名+”_txt”とでもしておけば
for(var tmp:Stirng in data){
this[tmp+”_txt”].text=data[tmp];
}
と3行で終わりなのに。
荒く分けてから
switch(x){
case “A”:
case “B”:
case “C”:
case “D”:
/*以下数十個のcase*/
function01(x);
break;;
case “1”:
case “2”:
case “3”:
case “4”:
/*以下数十個のcase*/
function02(x);
break;;
}
といった処理があったので飛び先の関数を見てみると
function function01(x):void{
case “A”:
/*処理A*/
break;
case “B”:
/*処理B*/
break;
case “C”:
/*処理C*/
break;
case “D”:
/*処理D*/
break;
}
}
となっていた。最初のswitch caseはいらない。
MVC
MVCを使おうとしているけど、なんでこれがviewでこれがcontrol?というような分け方で、しかもそれ以前に機能による分割が出来てないから、全てのデータがmodelに集中したりとかして、それぞれのソースが肥大して訳のわからないことになっていた。
スレッド?
- フラグを用意する
- enterFrameでそのフラグを監視する
- イベントが発生するとリスナー関数でフラグを立てる
- 監視していたenterFrameでフラグをとらえて処理を行う
という処理を見た。なぜ
- イベントが発生するとリスナー関数で処理を行う
としなかったかは謎である。
直値
ここまで来れば言うまでもないかも知れませんが、だいたい定数は定数用クラスなんて用意せず直接ロジックに書き込んであります。
なんでもObject型
パラメータ用にクラスを用意したりしません。必要なときはObject型に全部入れます。もちろん直値でアクセス。
data[“A”]=0;
こんな感じで。何の型が入ってるかは実行してみるまでのお楽しみ。
日常業務
もちろん、普段の業務の中心はデバッグです。
その後
会社がつぶれたので、社員の方々は他の会社に転職したはずです。彼らは今あなたのとなりや取引先にいるかもしれません。
でも、AS3に限らなければもっとひどいソースも見たことあるから世の中こんなものなのですかねえ。
おしまい。