ソースコードの混沌はどこから這い寄るのか

プロローグ

エンジニアとして働いているなら、誰もが混沌に満ちた闇の深いコード、カオスコードに遭遇したことがあるだろう。
何をしているかわからない、何故こんな書き方をしているかわからない、しかし正常に動作しているらしいというブラックボックス。そして往往にしてそのようなカオスコードは密結合になっていて一度手を加えればどこにどのような影響を与えるかもわからない。故に放置され、時を経てさらなる闇を生み出す。そうなるともはや我々人類は立ち尽くすしかないのである。

しかし考えてみてほしい。悪意を持ってカオスコードを生み出すものなど存在しないはずである。
誰もが光差すコードを書こうと腐心しているはずである。
それなのに今日もどこかで新たな闇が生み出されている。
それは何故なのか。

実装した者のスキルや意識の低さのせいにして「クソがぁ!」と叫ぶのは簡単である。
しかしそれでは何の解決にもならない。
我々は知性をもって物事を見極めなければならない。
事実を冷静に受け止め、分析することこそがカオスコードの闇を払う一筋の光となるのである。

…と気取った文章から始めましたが、私が実際に目にしたカオスコードをいくつか挙げつつ、そのカオスコードが何故生まれたのかを実装した人の気持ちを想像しながら分析することで、カオスコードにならないためにどうしたら良いか若造なりに考えようという試みです。
新人プログラマの方には反面教師にしてください。
あくまでディスるのが目的ではなく、なぜそうなったかを推測することで、そうならないようにするにはどうするか考えるのが目的であるということはご理解いただきたく思います。

各章のタイトルは格言っぽいこと書いてますが、私が適当につけただけですのでググったりするのは厳禁です。

※上げているコードは実際に目にしたものから、内容が大きく変わらない範囲で一部改変・省略してあります。

第1章 汝 自らの言葉をもって語らうべし

これ、何してるかわかりますか?

これはHighChartsを使ったグラフのツールチップの表示に関する処理のなかで見つけたものである。
なお、実際のコード内では特に補足的なコメントは無かった。

khaos_1.js

//this.point.zはグラフで表示する数値
var num = String(this.point.z).replace(/,/g, "");

while(num != (num = num.replace(/^(-?\d+)(\d{3})/, "$1,$2"))); //!?

// whileの()の中には条件式を入れるものだと思っていた時期が私にもありました...

while文の()の中で代入...だと...!?
正規表現に強い人ならすぐに理解できるのかもしれないが、そうでなければ理解するのは楽ではない。

検証の結果、数字の3桁ごとにカンマを入れているコードであることが判明した。
(1000 => 1,000のような変換である)

せめてこうして欲しかった...(これでも自分には少し辛いけど)

example.js

//this.point.zはグラフで表示する数値
var num = String(this.point.z).replace(/,/g, "");

while(num != (num.replace(/^(-?\d+)(\d{3})/, "$1,$2"))){
    // ループ処理は{}の中でやるとばっちゃが言ってた
    num = num.replace(/^(-?\d+)(\d{3})/, "$1,$2");
} 

考察

私は以下のように考えた。

1. このようなロジックを頭の中で組み立てた場合、愚直に文字数分のfor文でループを回すか再帰関数を用いるのが一般的だろう
2. whileを使うにしても()の中で代入処理をするという発想は真っ先に出るようなものではない
3. よって、このコードを実装者が自分で考案したとは考えにくく、どこかからコピペした可能性が高い
4. そして実際見つけた: コピペ元と思しきページ

結論：やはりコピペであった。

教訓

プログラミングをしていればネットでサンプルを探すことはよくあるし、それを参考にすることもあるだろう。
しかし、参考にするコードの良し悪しはしっかりと評価する必要があるし、取り入れるならばちゃんと自分の中でかみ砕いてから使うべきである。

ここから得られる教訓は

• そのままコピペ、ダメ絶対
• 自分が書いたコードには説明責任を持とう

第2章 新天地を目指す者よ　その手にある一切を捨てよ

当該システムの変遷

カオスコードを見る前に、カオスコードが発見されたシステムの変遷をざっくり追っておきたい

• もともとはユーザーごとのオンプレのLinuxサーバにインストールして稼働させていた
• 2年ほど前にプラットフォームをAWSに移行、SaaS化
• それに伴い、ELBを使った冗長化構成を採用するようになった

それではこちらをご覧いただこう

khaos_2.php

/**
  *  設定ファイル書き換え
  * @func write_setting
  * @param array     $setting_data  入力データ
  * @return true/false
**/
function write_setting($setting_data){
  //出力先設定
　// EACH_CONFIG_ORGは登録された組織ごとの設定ファイルを格納するディレクトリ
  $output_path = EACH_CONFIG_ORG . "/setting.inc";
  //改行
  $lfChar = "\n";
  $data = null;

  //内容作成
  $data .= '<?php' . $lfChar; // !? "<?php"で始まる文字列!?
  /** 省略 **/
  $data .= $lfChar;
  $data .= '?>' . $lfChar;

  //ファイルを開く（w=書き込みモード、なければ作成あれば上書き）
  $setting_fp = fopen($output_path, "w");
  //書き込み
  fputs($setting_fp, $data);
  fclose($setting_fp);

  return true; // (本筋からそれるけど)常にtrueを返す...だと...!?
}

こちらはなんと、PHPのソースコードを文字列連結で生み出して設定ファイルとして保存しているという現代の黒魔術であった。
勘のいい方なら「あれ、冗長化構成にしてるのにサーバ内に設定ファイル置いてるの？動的に書き換えられるのに?」と思ったであろう。

その点は解決済みであった。
設定ファイルを保存しているディレクトリから別のディレクトリにシンボリックリンクを張り、リンク先のディレクトリとS3のバケットを同期させるという超絶技巧で

何を言っているかわからねぇと思うが(ry

考察

このケースでダイレクトに響いているののは、元はオンプレだったが後にAWSに移行したという事情であろう。
恐らく、当時の実装担当者の思考は以下のような流れをたどったと思われる。

1. オンプレでは特に問題なかった
2. AWSへの移行により、内部に大幅な改修を加える必要がある
3. しかし、移行に期限が設定されている(おそらくあまり猶予は無かった)
4. そうなると作業量はできるだけ減らしたい
5. よって、可能な限り既存の仕組みを残して、最小限の改修でAWSに持っていきたい

教訓

できるだけ既存のコードに手を付けたくない、作業量を減らしたいという気持ちはよくわかる。
場合によっては正解でもあると思う。

しかし、プラットフォームの移行など、どうせ大幅な改修が必要なら思い切ってとことんやった方が良いのではなかろうか。
(時間的な制約でそうもいかない場合もあるだろうが...)

ここから得られる教訓は

• どうせやるなら徹底的に
• いかに既存の仕組みを残すかを考えるなら、いかに新しい仕組みに変えるかを考えた方が建設的
• Dockerとか使おう

第3章 歩みを止めたとき、後退が始まる

こんなコードを見たことないか?

khaos_3a.php

// $data => DBからとってきたデータ
// $processed_data => 画面表示用に成型したデータ
// $search_key => データの検索条件
//グラフ用にデータを加工
$processed_data = null;
process_data($data, $processed_data);

$processed_data["data_type"] = $search_key["data_type"]; //!?

!? $processed_dataはnullのはずなのに、なぜいきなりdata_typeというプロパティが!?
鋭い人ならもうタネがわかっていると思うが、一応process_dataというメソッドの中身を見てみた。

khaos_3b.php

/**
 * データを表示用に加工
 * @func process_data
 * @param array     $data                   DB取得データ
 * @param array     &$processed_data        成型後のデータ
 * @return
 * */
 function process_data($data, &$processed_data)
{
  /** 省略　返り値は無い **/
}

参照渡しである。
PHPerでこれを使う人は少数派だろう。

考察

Cのような言語では関数にサイズの大きな変数を渡すとメモリが食いつぶされるため、参照で渡すことがあった。
そう、数多のプログラミング初心者の心を折ったポインタである。

恐らくこの実装者もCと同じ感覚で参照で渡すことでメモリを節約しようとしたのだろう。
なので以下のような流れを辿ったのだろう。

1. 一昔前はメモリ節約などのため参照渡しは割と行われていた
2. 時を経て、ハードウェアの性能が向上
3. さらにクラウドの普及でもはやハードウェア的な制約を気にすることはなくなった
4. でもこのままで動いているし手を加える必要ないじゃん
5. 今に至る

と思っていた時期が私にもありましたが、しかし

いつから参照渡しがメモリを節約すると錯覚していた?

念のためPHPの参照渡しについて勉強し直したところ、残念ながらPHPの言語仕様上パフォーマンスの向上は期待できない。
その解説はこちらの記事にて詳細に行われている。
どうりで全然使われないわけだよ...

しかしCの経験があったがゆえに「参照渡しはメモリを節約できる」という先入観を持ってしまい、
CとPHPの言語仕様の違いに考えが及ばなかったのだろう。
また、実装者の中では「参照渡しはメモリを節約する」というのが自明の理であったため、ベンチマークをとって比較するという発想すらなかったのだろう。

そして、幸か不幸かとりあえず正常に動いていたため問題視されないまま今に至り、PHPで参照渡しなんてやったことのない私がそれを見て驚愕したのである。

教訓

前はこうだったから、今もこうだろうと決めつけるのは危険である。
新しく言語を学ぶことはあるだろうし、同じ言語を使い続けていてもバージョンアップなどで挙動が変わることもよくある。
そのときに**「今までこうだったから」を続けていると取り残され、闇に沈むのである。**
それを防ぐためにも、常に情報を収集し、前に進み続けなければならない。

よってここから得られる教訓は

• 「今までこうだったから」という先入観を捨てる
• 日頃から真面目に勉強する
• 誰もやってないことには、誰もやりたがらない理由がある

エピローグ

少しでも楽な道を選ぶこと自体は何も間違っていない。技術の進歩というのは楽をしようという気持ちから生まれるのだから。
大事なのは、それが数歩歩くと途切れる道なのか、それとも長く続いている道なのかをよく考え選択することである。

長期的に楽になる方法ほど短期的には負荷が大きくなる傾向にあるし、常に時代に取り残されないようにするのはかなりの労力を要するであろう。
しかし我々は安易な道に甘んじてはならない。
光は煉獄の先で待っているのであり、そこから目を背けたものが混沌に飲まれるのであるから。

...と偉そうに締めましたが、自分を追い込むためと解釈してください...頑張ります...
この文章が少しでもこの世から闇を払う助けとなれば幸いです。

参考

• JavaScriptで数値を桁区切り (カンマ区切り) にする方法
• PHPが糞言語なのはどう考えても参照をポインタだと思っているお前らが悪い - なんたらノート第三期ベータ