条件分岐禁止とは言ってもガード節は書きたい?
PHPerKaigi本編で 設計力を上げる!バリエーションの見極め術 を発表した直後、懇親会であるPHPer茶会で「条件分岐禁止について」という枠が急遽設けられて、駆けつけて話した際に「条件分岐禁止の趣旨はわかるがガード節は書きたい」という意見をもらいました。
ガード節とは
ガード節とは、引数がそのメソッド・関数の処理する対象として合っているかどうかをメソッド・関数の先頭でチェックするif文です。通常、マッチしない場合は例外を投げるか、early returnにより処理から抜けます。
ガード節は一般に良いプラクティスとされており(防御的プログラミング)、質問者の方もガード節を良いものという前提に立った時、私の言う条件分岐禁止はそれに反しないか気にされたのでしょう。
ガード節と条件分岐禁止
さて、PHPerKaigiの発表スライドではほぼ割愛しましたが、私が想定している条件分岐禁止を守って書いたコードは下記のような感じです。
※ スライド中では良い例としてappのコードのみ示し、詳細は 昨年の@hidenorigotoさんのスライド を見てください、と口頭で言いました。
appのコード
$targetDate = new \DateTimeImmutable('2019-05-02');
echo $wareki = $warekiProviderResolver->resolve($targetDate)->provide($targetDate); // 令和1年5月2日
Resolverのコード
<?php
class WarekiProviderResolver
{
/**
* @var WarekiProviderInterface[]
*/
private $providers;
// ...
public function resolve(\DateTimeInterface $targetDate): WarekiProviderInterface
{
foreach ($this->providers as $provider) {
if ($provider->supports($targetDate)) {
return $provider;
}
}
throw new \LogicException(sprintf('No matching WarekiProvider found for date %s', $targetDate->format('Y-m-d')));
}
}
そして令和のためのWarekiProviderがこんな感じ。(平成、昭和、大正…それぞれの元号についてWarekiProviderInterfaceを実装したクラスを作ります)
<?php
class Reiwa implements WarekiProviderInterface
{
public function supports(\DateTime $targetDate): bool
{
return $targetDate->format('Ymd') >= 20190501;
}
public function provide(\DateTime $targetDate): string
{
// ★ここにガード節書きたい?★
$year = (int)$targetDate->format('Y') - 2019;
return sprintf('令和%d年%d月%d日', $year, $targetDate->format('n'), $targetDate->format('j'));
}
}
先ほどのガード節は必要だと主張された質問者の方は、おそらく下記のような使い方がされることを危惧されているのでしょうね。
<?php
$targetDate = new \DateTimeImmutable('2018-04-30'); // 平成の年月日
$warekiProvider = new Reiwa();
echo $warekiProvider->provide($targetDate); // 令和-1年4月30日
質問された方には、「各クラスResolverでresolveする使い方を想定していると周辺のクラス構成からわかるときは、別にガード節を書いておかなくても誰も変な使い方をしないだろうから、ガード節は書かなくても問題ない」とお答えしました(契約による設計)。
少なくともカルテットでは文化としてこの書き方が浸透していますし、必ずコードレビューをするので、ここにガード節が必要とは考えたこともなかったです。
が、このような変な使い方をしてしまうメンバーがいて、それをレビューで止めることができない環境であれば、ガード節を書くこともやむを得ないのかもしれない、と考えていました。
nazonohito51/dependency-analyzer
PHPerKaigiのLTで @nazonohito51 さんが nazonohito51/dependency-analyzer というライブラリを発表されました。
dependency-analyzerは静的解析を用いてプロジェクト内のクラス間の依存グラフを出力したり、ルールに沿った依存のみが書かれているかバリデーションしたりすることを通じて、単方向依存を実現しやすくする・設計の意図をプロジェクト全体で共有することを目的としたツールです。(詳細は @nazonohito51さんのスライド をご覧ください)
LTを聞きながらこのライブラリを使えば前述の質問者の方が危惧されていた変な使い方はCIで検出できると思ったので、早速使ってみました。
使い方はとても簡単です。
composer require –devで追加して、使い方を限定したいクラス(今回の場合は Reiwa
)に @canOnlyUsedBy
をdocコメントとして書き込み、CIでdependency-analyzerのチェックコマンドを追加実行するよう設定するだけです。
設定済サンプルが下記にあります。
https://github.com/77web/DependencyAnalyzerUsage
dependency-analyzerで想定外の使われ方を防ぐ
CIにdependency-analyserを設定しておくと、Resolverで使うことを想定されたクラスが変な使い方をされてしまった場合には下記のようにCIが落ちます。
phpdoc in Quartetcom\TryDependencyAnalyzer\Wareki\Min
+------------------------------------------------+-----------+----+---------------------------------------------+-----------+
| depender | component | | dependee | component |
+------------------------------------------------+-----------+----+---------------------------------------------+-----------+
| Quartetcom\TryDependencyAnalyzer\WrongUsageApp | other | -> | Quartetcom\TryDependencyAnalyzer\Wareki\Min | phpdoc |
+------------------------------------------------+-----------+----+---------------------------------------------+-----------+
The command "./vendor/bin/analyze-deps verify ./src" exited with 1.
https://travis-ci.org/77web/DependencyAnalyzerUsage/jobs/522931490
CIが落ちたらマージしない、という簡単な判定で想定外の使われ方を確実に防ぐことができるようになりました。
まとめ
カルテットは世界一効率的な広告代理店を目指しており、開発部のみならず社内全体で人の注意力より道具・仕組みの力でミスを防ごうとする雰囲気があります。
新しい道具も積極的に使って、よりメンテしやすいコードを書いていきたいですね。