新しく配属された開発現場で、コードを読みはじめてみると漂ってくる不穏な空気。
コードの節々から漂う悪臭。それが、いわゆる「コードスメル」(コードの匂い) です。
コードから嫌な匂いを感じた場合、それはチーム内でのコードレビューが不足していたり、SIer から納品されたコードを無条件で受け入れていたり、古いコードをまったくリファクタリングしていないない場合があり、何らかの不具合が潜んでいる可能性があります。例えば、コードのコピペが多い、開発終了しているライブラリに依存している(しかもそのライブラリがリポジトリに含まれている)など、リファクタリングが現実的に不可能な状況に陥っていることも少なくありません。
私が今まで携わったコードの中で見てきた、「こんなコードは大抵ヤバい」という語句を12こ紹介します。
1. checkXxx() (メソッド)
checkナントカ というメソッド名。前後関係から、おそらくブール値を返すメソッドのようなので、つまりバリデーションをしてるっぽいのですが、何を返すのか、それとも何も返さずに内部状態を変えるものなのか、実態はコードを読むまでさっぱりわかりません。
真偽値を返すことを期待して読み進めていくと、突如現れる return -1。return 0xFF だから超 true ってこと?
このチェックは成功しているのか？失敗しているのか? 成功とか失敗とか関係ない世界なのか?
メソッドの存在意義自体をぼやかす、 checkナントカ っていう名前、やめてくれ。
validateXxx という名前にすればいいのか、というとそうでもなく、依然として何がおこるのかわかりにくいままです。
たまに、バリデーションのメソッドで、通常時は return は何も返さず、バリデーションに失敗すると例外を投げる設計の時もあります。どちらかというとこっちの方が、真偽値を返すより個人的にはしっくりきます。
もし isValidXxxx という名前なら、真偽値を返すメソッドで true なら問題無いことがわかりやすいので、悩まずに読めたのに。 throwIfInvalidXxxx とかでもいいかもしれません。
2. xxx_flag
日本の SIer さんが書いたコードに、かなり見かけます。特に DB のフィールド名にわんさかいます。真偽値のサフィックスで _flag をつけないと査定に響くぞ、とか上司から言われてるんですかね。
ビットフラグならわかりますよ。
ATTACK_BUFF = 0x02
DEFFENCE_BUFF = 0x04
buff_flag = ATTACK_BUFF | DEFFENCE_BUFF
ああ、これならフラグだなって感じです。実際、PHP の json_encode や error_reporting でしこたま使います。ただ DBにビットフラグ入れるのかっていう。
xxx_flag 、0, 1 ならまだわかるけど たまに 2 とか入ってる。 delete_flag = 2。なんやねん。
おそらく1と排他な状態なんだな…とは思いますよ。でも delete が 2 って。delete = 0 をアクティブだと思い込んでいた先入観さえ揺らがされます。
さらに混乱させられるのが、xxx_flag = 3 以上になった時。
ぱっと見ただけだと、1 とも 2とも排他な状態なのか、それとも 0b01 | 0b10 ってことなのか、判断できない。
そしてたまに見かける adult_flag = 'YES' 。文字列って。たしかにわかりやすいけど。その YES は定数化しないんですかね。
定数化したところで、 define YES = 'YES', NO = 'NO'。そこには虚無しかありません。
3. t_テーブル名, m_テーブル名
テーブルの先頭につく謎の t_ m_ のプレフィックス。どうやら、太古には世の中のテーブルをすべて「トランザクションテーブル」と「マスターテーブル」に2分し、管理するという教えがあったそうです。マスターは、なんか固定したデータを扱うテーブル、トランザクションテーブルは処理のたびに増えていくテーブルだそうな。
では、ECサイトで「商品」を扱う場合、それはトランザクションなのかマスターなのか? 「ユーザー」は? ブログのページは? それらを日々議論して区別しなければいけないそうです。くだらなすぎる。
4. 顧客番号
まさかの DB フィールド名に日本語。 WHERE 顧客番号 = ? 。勇ましい。バックアップ作る時の文字コードが不安で仕方がない。 WHERE kokyaku_banogu = ? よりはまだマシかも知れない。そんなこともないか。どちらも私の目の前からさっさと消えてほしい。自分で直すのはめんどくさすぎる。
5. regist
レジスト。何かに抵抗するのか? と思いきや、何かを登録する処理らしい。なぜ素直に register と書かないのか。エディターでスペルチェッカーが反応して警告を出しまくるのでうざいことこの上ない。すべての感情を捨て去り、そっとエディタのスペルチェッカーに登録する単語。1週回って反社会的でかっこいいのかもしれない。
6. custom
カスタム。こっちはスペルチェッカーが反応しない。何かをカスタマイズする処理なのかと思って追ってみると、 logout(custom) というコードが出てくる。 customer のことだった。なんで er とっちゃった。er を grater とか richer とかの比較級だと思って er とったのか。それとも phper みたいなもんだと勘違いしたのか? 商品購入結果の order テーブルのレコードに custom_id って入ってて、顧客IDだと一発で理解できる人がどれだけいるんだろう。
7. if (xxx_id == 9999) { // 無効な場合
何かのリレーションをしてるテーブルの ID の 9999 を無効とか未設定で扱うプロダクトがたまにある。汚なすぎる。どっからその4桁はきたんですかねえ... しかも DBに ID = 9999 のレコードがができちゃった後で、オートインクリメントで ID 10000 以上もできちゃってるし。そして、プロダクトコードの仲の至るところに != 9999 コードがいるし。しかも数箇所 != 9999 判定しなきゃいけないところが入ってないし。だれが直すんですかね。
無効なIDを 9の連続で表現する手法。この手法に名前をつけたい。
8. z-index: 10000
z-index インフレ問題。このエレメントの上にさらにエレメントを載せようとして、さらに新参の方が z-index: 100000000 っていうコードを書いて平気でインフレしちゃう。もはや桁数だけしか意味を持たない。全部 log10 すればいいのでは。
z-index を新たに書く前に、コードを読め。コメントして規約を作れ。そんで新しいプロジェクトで z-index を 10 で収めようとしてうまくフロントを設計した後、突如外部ライブラリから注入される z-index: 10000 にブチギレです。
9. 検索結果のページが http POST
これも古いプロジェクトでよくあったりする。検索のフォームが POST で動くもの。URL を他の人と共有できない。 http POST ってそもそもんなんのためにあると思ってるんだ。
昔、POSTで検索フォームを作ってる開発者に「なんで GET じゃないんですか」と聞いたら「GET、ダサいじゃん」と言われたことがあって、すべてどうでもよく感じるようになった不思議な体験をしたことがあります。かっこよさってむずかしいものですね。
10. 検索結果のページがセッションデータ
POST で受け取ってセッションに詰め込んで、それを検索ページの条件にするやつ。別タブでの操作内容が他のタブに影響しまくる。デバッグがかなり難しく、新しいコードをデプロイすると既存利用者の環境が壊れて、問い合わせがめっちゃくる。そして開発側で再現できない。その後出てくる「セッション内のデータをバージョン管理する」という解決策。地獄。その労力をもっと別のところに向けられないんですかねえ。HTTPメソッドをイチから勉強してこい。
11. <img src="/img/spacer.gif">
スペーサージフ。テーブルレイアウトと並んで HTML コーディングを最悪なものにしてくれるモンスター。開発者が新しいHTMLを書く時は直しますが、この教えが運用スタッフに深く根付いている場合があるのが厄介。
「先輩に、こう書けっていわれまして…」魔法のiらんどかよ。その先輩はキリ番ゲッターですか。
しかしテーブルレイアウトは管理サイトの大枠ページに根強く残ってる場合もある。解消するには管理サイトを作り直すしかないけど、事業が成功しないとお金がないので直せない。つらい。
ところで、10年後には、「うわーこのサイト flex 使ってる! 臭い! 」って言われるようになるのかな…。
12. <font color="red">
非推奨タグ。運営チームのおねーちゃんがなんかやたら使ってくる感じのやつ。「え？だめなんですか？なんでですか？」って言われた時の返しがすごく疲れるやつ。
元をたどれば、HTMLを直接書き込む運用が良くないのはわかってる。でもそういう現場も多い。
WYSIWYG エディタは「思い通りにならない」っていう苦情が来まくる諸刃の剣。
じゃー markdown でいくか、っていうと、「また新しいの覚えないといけないんですか」と言われる。まあ、わかるが、覚えてよ。