$nl
〜僕が見た"最高"のPHPコード〜
黒曜
@kokuyouwind
▲ スライドのリンクは常に表示されています
スライドめくりもリンクされます
$ whoami
- 黒曜 / @kokuyouwind
- 株式会社Misocaに転職して半年
- 現職: Ruby on Rails/React
- 前職: PHP/Scala
- 趣味: OCaml
宣伝
プログラマ向けの謎解きゲームを作りました
宣伝
- Githubで公開中
- http://kokuyouwind.com/b
- 「プログラマ 脱出ゲーム」でググると多分出てくる
本題
RubyKaigi2015
私が人生で見た
最悪のRubyコード
聞きに行った
感想
- 実際、割と酷かった
-
RubyKaigiのセッションページに動画がある
- 教訓もあり、良い発表だった
一つ思ったこと
自分が見た
PHPコードは
これより酷かった！
(主観)
$nl
〜僕が見た"最高"のPHPコード〜
黒曜
@kokuyouwind
構成
独自フレームワーク！
構成
構成
/core
/model - MVCのM
/logic - 共有ロジック
/for_xxx
/public - web公開ディレクトリ
/controller - MVCのC
/view - MVCのV
/template - Smartyテンプレート
構成
/core
/model - MVCのM
/logic - 共有ロジック
/for_xxx
/public - web公開ディレクトリ
/controller - MVCのC
/view - MVCのV
/template - Smartyテンプレート
controller
- MVCのC…ではない！
- 大体のメイン処理を書く場所
- 変数名が大体ひどい
- 後述する
- 例外駆動プログラミングという技法がフル活用される
- 後述する
- とりあえずひどい
- 後述する
構成
/core
/model - MVCのM
/logic - 共有ロジック
/for_xxx
/public - web公開ディレクトリ
/controller - MVCのC メイン処理を書く場所
/view - MVCのV
/template - Smartyテンプレート
view
- MVCのV…ではない！
- 謎
- そもそもテンプレートは別にある
- controllerから受け取った値を表示用に加工する？
- 大体は受け流すだけ
- たまにネストのレベルを変えてて悶絶する
- できることはControllerとほぼ一緒
- たまにすごい処理が書いてあってビビる
構成
/core
/model - MVCのM
/logic - 共有ロジック
/for_xxx
/public - web公開ディレクトリ
/controller - MVCのC メイン処理を書く場所
/view - MVCのV 謎
/template - Smartyテンプレート
model
- MVCのM…ではない！
- 配列を上手く扱うためのクラス
- ドメインモデルのデータはすべて配列で扱う
- (PHPの配列は連想配列)
- DBとの入出力も扱う
- PDOでクエリを直接投げる
- 独自のクエリビルダーが4種類くらいある
- どこからでも$this->getModel('Name')で取れる
model
<?php
class UserModel extends HogeFWModel
{
public function get($id, $ignore_delete = false) {
$table = $this->get_table_name($id); // ソフトシャーディング
$query = "SELECT * FROM $table WHERE id = ?";
if (!$ignore_delete) {
// deletedが未来のことがあるので、IS NULLだけではだめ
$query .= "AND (deleted IS NULL OR deleted <= '$(data('Y-m-d h:i:s'))'");
}
$records = $this->db->execute($query, $id);
if ($records === false) {
// 厳密にfalseならSQL接続エラー
throw new SQLException('SQL接続エラーです');
} elseif (!$records) {
// クエリは成功したが中身が空
throw new NotFoundException('ユーザーが見つかりませんでした');
}
$user = $records[0];
// 他に必要な情報を補完する
return $this->appendInfo($user);
}
}
model
// こんな感じで使う
// callはメソッドの呼び出し結果をキャッシュする関数
$this->cache->call('User', 'get', $id);
// callには配列形式で渡しても良い
$um = $this-> getModel('User');
$this->cache->call([$um, 'get'], $id);
// 返ってくるのは配列(というかハッシュ)
['id' => '758',
'name' => '黒曜',
'registered' => '2016-04-16 15:50:00', // 大抵MySQL形式
'last_login' => 1460789400, // たまにUnixTimeが入っている
'attributes' => [
'belongs' => [
'Misoca' => ['id' => '331'] // どんどんネストする
],
'is_penalty' => 'none', // なぜか真偽値ではなく文字列
],
'append_info' => [
// 3段くらいネストしたなんらかの情報
]
]
構成
/core
/model - MVCのM 配列の加工とDB処理
/logic - 共有ロジック
/for_xxx
/public - web公開ディレクトリ
/controller - MVCのC メイン処理を書く場所
/view - MVCのV 謎
/template - Smartyテンプレート
logic
- 共有ロジック…ではないものがほとんど
- 大体はcontrollerの中身を分割したもの
- バージョン移行の際に、controllerの処理を
まるまるコピーしlogicクラスができたりした
- バージョン移行の際に、controllerの処理を
- controllerでできることはlogicでもmodelでもできる
- たまにlogicやmodelから直接viewに渡す値を
設定していてビビる
- たまにlogicやmodelから直接viewに渡す値を
- どこからでも$this->getLogic('Name')で取れる
構成
/core
/model - MVCのM 配列の加工とDB処理
/logic - 共有ロジック メイン処理を書く場所2
/for_xxx
/public - web公開ディレクトリ
/controller - MVCのC メイン処理を書く場所
/view - MVCのV 謎
/template - Smartyテンプレート
"最高"のPHPコード
"最高"のPHPコード
- 単一のコンテンツを表示するページのController
- メイン処理のメソッドが3000行くらい
- 1メソッドで3000行
- うち2000行くらいがcatch節
- 5段くらいにネストした100項目くらいの配列を構築
- これが1つのコンテンツを表す
- とりあえずvar_dumpしてデバッグする
- 5環境くらいにforkして、それぞれ独自に進化している
2文字変数
変数名が酷い
<?php
class ContentController extends HogeFWController
{
public function execute() {
// 大体2文字変数
$cm = $this->getModel('Content');
$nl = $this->getLogic('Normalize');
$cont = $this->cache->call([$cm, 'get'], $this->getAttribute('id'));
$cont = $nl->norm($cont);
// 間に300行くらい挟まる
if($has_body) {
$cont = $nl->normBody($cont);
}
// 間に500行くらい
if ($need_next) {
// さっきの$nlを上書きする
$nl = $this->getLogic('NextContent');
$content['next'] = $nl->calc($content);
}
// ...まだ続きます変数名が酷い
// ...続き
// 間に300行くらい
// さっきの分岐を通ってると死ぬバグ
$content['append'] = $nl->normAppend($content);
//間に200行くらい
if ($need_extra) {
// 明らかに$nlじゃないけど$nlを上書きする
$nl = $this->getLogic('Extra');
$content['extra'] = $nl->compute($content);
}
// 間に300行くらい
// viewにcontentを受け渡す(Model, Logicからも可能)
$this->setAttribute('content', $content);
return;
}
}
例外駆動プログラミング
例外駆動プログラミング
<?php
class ContentController extends HogeFWController
{
public function execute() {
try {
$cm = $this->getModel('Content');
$content = $this->cache->call($cm, 'get', $this->getAttribute('id'));
if(!this->user) {
throw new NotLoginException();
}
// ログイン時、コンテンツありの処理が1000行くらい続く
} catch (NotLoginException $e) {
// 非ログイン時の処理
if ($cm->isAllowGuest($content)) {
// 非ログイン時、コンテンツ閲覧可の処理が800行くらい続く
// うち600行くらいはログイン時処理のコピペ
} else {
// view/content_error_viewを描画する
return 'content_error';
}
}
}
}
どうしてこうなったか
- そもそも2007年ごろから作られ始めたコード
- PHP5.2の頃で名前空間すらない
- モダンで実績のあるフレームワークもあまりない
- 初期は数名での開発だった
- レビュー文化のないカウボーイプログラミング
- テスト文化もない
- 新機能や企画優先で継ぎ足し実装が横行した
- 品質より納期を優先するスケジュール
- 追加された機能が消せず、膨れる一方
どうやって戦っていたか
- レビュー文化が根付き始めた時期の入社だった
- 当時はsvnでFisheye/Crucibleを使っていた
- PHPUnitも先輩が広めていた
- 複雑すぎてテスト不可能な部分が多すぎた
- 新規に書くコードはテストを書く、という共通認識
- 手作業確認によるリファクタリング(リスク高)
- grep力を高めて物理で殴る
- 最近はScalaで書きなおそうとしている(はず)
- 転職したため現状は謎
教訓
- 機能追加偏重の文化は保守不能なコードを生み出しうる
- 無論、ユーザーに価値を届けることは大事
- うまくバランスを取ることが重要
- レビュー文化、テスト文化で品質を保つ
- スピードを上げ過ぎない「足枷」としての機能
- 無理な納期から開発チームを守る防火壁が必要
- 機能もコードも「捨てられる」ように作る
- 犠牲的アーキテクチャ
- 機能テスト
予備スライド
他に面白かったコード
- バグにバグが重なって上手く動いているコード
- 1日が 60 * 60 * 60 * 24 秒ある
- 1週間が 8 日ある
- コメントと返り値が真逆
- 関数名は block
- 「過負荷ブロックされたらfalseが返ります」
- 実際は過負荷ブロック時にtrueが返っていた
- forが3つネストした中に「continue $n;」
- 100行くらいあるメソッド先頭に「return false;」
構成
/core
/model - MVCのM
/logic - 共有ロジック
/for_xxx
/public - web公開ディレクトリ
/controller - MVCのC
/view - MVCのV
/template - Smartyテンプレート
public
- webからのエントリポイント
- apacheでDirectoryとして指定される
- Controllerをkickするphpファイルが配置される
- URL routesの数だけファイルが置かれる
- Router? なにそれおいしいの?
- 画像/js/cssなどの静的ファイルもここに配置される
構成
/core
/model - MVCのM
/logic - 共有ロジック
/for_xxx
/public - web公開ディレクトリ
/controller - MVCのC メイン処理を書く場所
/view - MVCのV 謎
/template - Smartyテンプレート
template
- Smartyのテンプレートを置く場所
- ネストが非常に深い
- 3重ifの中身が数十行とかはざら
- ロジック分離？ なにそれ美味し(ry
- 多言語対応の影響でさらにカオスになった
- 日本語以外では出さない、みたいな分岐が
そこら中にある
- 日本語以外では出さない、みたいな分岐が