小さくコードを保つ指針

この記事はRelux Advent Calendar 2017 18日目の記事です。

qiita.com

Webアプリケーションのフレームワーク上で小さくコードを保つ指針的なものを自分へのメモとして記載した。
昨今の横文字は一旦忘れる、問題は大体そこじゃない。

前提. コードの歪みは仕様の歪み

要望は要望であって鵜呑みにすると痛い目に合う。考えられていない前提を持つ。仕様とコードの依存関係は密であって、コードが複雑化しているのは仕様だったというケースがある。仕様を精査する過程で、代替となる機能を発見できたり、そもそも作らないという選択が出来ることもある。実装前にまずは精神を落ち着かせて、仕様に向き合うこと。何かしらの要因で向き合うことが出来ていない状態であれば、根本の課題を探り、その解決に向けて動く必要がある。

メソッド名に迷ったら

「これお願いね!」
「いやいや俺じゃないって」
「お前か!」
「ガハハハ」

オブジェクトに対して話しかけるメッセージングが不自然になってないかを考える。実装しようとしてるパブリックのメソッドが既存にあるオブジェクトにあると違和感を感じたり、他に適切なオブジェクトが閃いたら、積極的に新規でクラスを作っていくのも手の一つ。メソッド名が長くなってしまったり、適切な名前が浮かばなかった時は、そのクラスの責務ではない信号かもしれない。

請求書の封筒に貼るラベルをPDFとして一括ダウンロードする機能があったとする。

// app/Controller/InvoiceLabelsController.php
class InvoiceLabelsController extends AppController
{
     public function index()
     {
         // 一覧を取得してPDFとしてレンダーする処理
     }
}
// app/Model/InvoiceLabel.php
class InvoiceLabel
{
     private $somethingA;
     private $somethingB;

     // ラベルに必要なオブジェクト
     public function __construct(
         SomethingA $somethingA,
         SomethingB $somethingB
     ) {
         $this->somethingA = $somethingA;
         $this->somethingB = $somethingB;
     }

     public function listAll()
     {
         //ラベル取得に必要な処理をゴニョゴニョ書く
         // $this->somethingA->doSomething();
         // $this->somethingB->doSomething();
     }
}

既存のコントローラーやモデルに書く形でも勿論問題ないが、これ以上太らせちゃいけないものもある。コントローラーやモデルとなるクラスを新規に抽出していくことで、既存クラスに影響を与えず、小さい責務のクラスを作っていくことが出来る。大体後から追加仕様が入ってくることを念頭におく。

一つのクラスに依存するオブジェクトが多い時

上記モデルの実装例だと、二つのオブジェクトへ依存していることが分かる。 追加の要件で封筒に入れる送付状も合わせPDF出力したいという要望があった場合、どう実装していくべきか。その送付状へ記載する内容は別のオブジェクトが必要だと仮定する。

// app/Model/InvoiceLabel.php
class InvoiceLabel
{
     private $somethingA;
     private $somethingB;
     // 追加で必要となったオブジェクト
     private $somethingC;

     // ラベルや送付状に必要なオブジェクト
     public function __construct(
         SomethingA $somethingA,
         SomethingB $somethingB,
         SomethingC $somethingC
     ) {
         $this->somethingA = $somethingA;
         $this->somethingB = $somethingB;
         $this->somethingC = $somethingC;
     }

     public function listAll()
     {
        //ラベルや送付状に必要な処理をゴニョゴニョ書く
        // $this->somethingA->doSomething();
        // $this->somethingB->doSomething();
        // $this->somethingC->doSomething();
     }
}

既存コードを流用すると上記のようなコードを書いてしまいそうだ。オブジェクトの依存関係に着目する。送付状という別のモデルを抽出して、送付状に必要なデータ取得のメソッドを生やす形か、そもそも分割を行ったクラスが誤った依存関係を生み出してしまってないかを検討する。請求書のモデルに対して、ラベル取得、送付状取得のメソッドを生やし、オブジェクトのリレーションを辿る方法もある。難しく考え過ぎていると感じたら、既存にあるオブジェクトでその責務を果たせないかも検討する。

その他念頭に置くこと

  • 歴史を振り返る。
  • チームの状態はどうだろうか?他に優先して潰すべき課題はないだろうか?技術だけで解決できない問題があることを知る。
  • やりすぎると大体失敗する。小さく積み上げることを意識する。
  • 時にはスピードも意識する。ただ、最低限守るべき事項をチームで明示化し、厳守する。
  • 機能の改修でリファクタリングの必要性を感じた時は、しない形でまずは実装を行う。実装後に、リファクタリングのブランチを切って、積み上げる。何故なら大体深みにはまってしまうからである。
  • 既存コードがさっぱり分からない時は、捨てるという選択肢を視野にいれる。一から作り直す方がコストが小さく済む場合もある。
  • フレームワークのレールから外れることを検討したとき、その役割を明確にし、チームで共通の認識を持つことが必須である。説明機会から、クラス設計・コードへのレビューコストをかける、また、今そこにいない仲間の受け入れがすぐ可能な状態にすることも必要である。その前提を受け入れて、本当に導入すべきか、今一度考え直す必要がある。大体破綻する。

いつもよく見返している資料

最後に

自戒を込めて