いつメソッドを短くするの？
今でしょ
ExcessiveMethodLengthとは
メソッドの行数が多すぎる（縦に長い）という警告です。
最初にリファクタしてほしい警告であり、常に気をつけてほしい警告です。
phpmdの既定値は100行ですが、まだまだ長すぎです。
「ThoughtWorksアンソロジー」の第5章で、Jeff Bay氏が提唱した「オブジェクト指向エクササイズ」のルール1は、もっと厳しくて、（メソッドではなく）クラスが50行までです。
例えばね、ケント・ベックが長いメソッドを書いているの、想像できる？
たまには書くかもしれないよ
じゃあね、ケント・ベックが長いメソッドのまま、デザインパターンを使おうとしているの、想像できる？
モンスターメソッドだらけのまま、ちょちょいとデザインパターンを使ってしまうかもね
まじめに答えてよー
メッセージ例
The xx yy() has zz lines of code. Current threshold is set to aa. Avoid really long methods.抑止方法
/**
*
* @SuppressWarnings(PHPMD.ExcessiveMethodLength)
*/説明
Violations of this rule usually indicate that the method is doing too much. Try to reduce the method size by creating helper methods and removing any copy/pasted code.
メソッド長すぎ！
メソッドの処理内容が多すぎるね。
ヘルパーメソッドに分割して、行数を小さくしよう。
（空行もカウントします）
モンスターメソッド
この世界には、メソッド人とエンジニア人が暮らしていました。どちらの民族も友好的でした。
メソッド人のなかには巨人化してモンスターメソッドに変化する者がいます。モンスターメソッドは、エンジニア人が対抗することができない恐ろしい怪物なのです。
メソッド人の大きさは「行」で数えます。何行以上をモンスターメソッドとするという明確な定義はありませんが、一般的には、200〜300行級を小型、500行級を中型、1000行級を大型と分類することが多いようです。
200〜300行級の小型はあちこちに大勢います。それどころか、エンジニア人の多くは、200〜300行級をモンスターメソッドと認識していないかもしれません。1000行級の大型も珍しくなく、うわさでは3000行級の超大型もいると聞きます。
メソッド人は数値の計算が得意なおだやかな民族なので、なぜ凶暴なモンスターメソッドに変化してしまうのか、謎につつまれていました。
エンジニア人の調査によると、モンスターメソッドは病気であり、メソッド人は誰でもモンスターメソッドになってしまうかもしれないことがわかりました。
メソッド人の主食はコード芋です。メソッド人の身体には排泄器官がないため、コード芋を食べると食べただけ、内蔵脂肪や皮下脂肪の一種であるコード脂肪がたまり、体が大きくなります。
そのため、メソッド人は健康オタクが多く、テストケースサプリを飲んだり、フィットネスジムでリファクタ運動することがブームとなっているほどです。
ところが、テストケースサプリを飲み忘れると、メソッド人は体を動かすのが面倒になります。リファクタ運動をさぼるようになり、少しづつ巨人化がはじまります。そして、満腹中枢が麻痺し、コード芋をむさぼるように食べ、巨人化へのループが止まらなくなるのです。
いくつかの治療報告では、巨人化の初期に、テストケースサプリを飲んだり、リファクタ運動することで、元のメソッド人に戻すことができたそうです。
別の治療報告では、巨人化がはじまったメソッド人は、性格が凶暴化し、テストケースサプリを飲むこともリファクタ運動をすることも拒否するため、治療は難しいともあります。
病気は進行し、やがて完全なモンスターメソッドとなり、エンジニア人に対して大暴れするのです。
この惨状を見かねたエンジニア人政府は、巨人化がはじまったメソッド人にコード芋を与えてはいけいないという法律（コード芋禁止法）をつくりました。
コード芋禁止法は、いつから巨人化がはじまったとするのか明確ではない、守らなかったときの罰則がないなど、その効果が疑問視されています。
おおかたの予想どおり、エンジニア人はコード芋禁止法を守ることはなく、今このときもモンスターメソッドは増え続けています。
メソッドの成長速度を遅くする
fooメソッドに機能追加するとき、
2行以上を追加する場合は、fooメソッドにその2行を実装するのではなく、
class MyFoo
{
public function foo()
{
...
$x += ($y + $z); // 新規コード
$a += ($b + $c); // 新規コード
...
}Code language: PHP (php)新しくbarメソッドを作り、barメソッドに実装します。fooメソッドには、barメソッドを呼ぶ1行だけを追加するようにします。
class MyFoo
{
public function foo()
{
...
list($x, $a) = $this->bar($a, $b, $c, $x, $y, $z);
...
}
public function bar($a, $b, $c, $x, $y, $z)
{
$x += ($y + $z); // 新規コード
$a += ($b + $c); // 新規コード
return [$x, $a];
}Code language: PHP (php)MyFooクラスは新しいbarメソッドの分だけ大きくなってしまいました。MyFooクラスを大きくしたくない場合は、新しくMyBarクラスを作って、barメソッドを移動します。
class MyFoo
{
public function foo()
{
...
list($x, $a) = $this->bar($a, $b, $c, $x, $y, $z);
...
}
private function bar($a, $b, $c, $x, $y, $z)
{
$myBar = new MyBar();
return $myBar->bar($a, $b, $c, $x, $y, $z);
}Code language: PHP (php)class MyBar
{
public function bar($a, $b, $c, $x, $y, $z)
{
$x += ($y + $z); // 新規コード
$a += ($b + $c); // 新規コード
return [$x, $a];
}
}Code language: PHP (php)メソッドに分ける範囲
我々は１行２行でもヘルパーメソッドに分ける、そのドメインの言葉でメソッド名をつけることで、一種のDSL（ドメイン固有言語）を作っているのだ
実践テスト駆動開発
100行に及ぶメソッドを再利用できる機会はない
ThoughtWorksアンソロジー 第5章 Jeff Bay氏
さまざまな文脈で利用できるのは、3行のメソッド
ThoughtWorksアンソロジー 第5章 Jeff Bay氏
ふだんから、どんどんメソッドに分けましょう。
●メソッド前半にガード節と早期リターンが何個もある場合、ガード節群とメインメソッドをそれぞれメソッドに分けます。
●コメントをつけたくなったら、メソッドに分けます。
つけたいコメントを参考にしてメソッド名にします。
●連続行を空白行で区切りたくなったらメソッドに分けます。
そのかたまりには意味があり、他のかたまりとは意味が違うかもしれません。
●メソッド内の配列リテラルは、それを返すメソッドに分けます。
例えば、getFooLiteralというメソッドにします。
●for文まるごとメソッドに分けます。
例えば、最後に Allをつけて、addFooAllというメソッドにします。
●for文内の処理をメソッドに分けます。
例えば、最後に SingleやOneをつけて、addFooOneというメソッドにします。
●if文まるごとメソッドに分けます。
例えば、最後に Ifxxxをつけて、addFooIfAgeIsLessThanEquals にします。
●if文内の処理をメソッドに分けます。
●switch文まるごとメソッドに分けます。
●case文の処理をメソッドに分けます。
●preg_matchやpreg_replaceをメソッドに分けて、ユニットテストを書きます。
正規表現の間違いが見つかるかもれしません。
これまで動いていたのは、たまたまってことね
どんどんメソッドに分けようー！
メソッドを短くしようー！
デモ行進みたい
メソッドを新しいメソッドに分ける
メソッドに分けるときは、IDEのリファクタ機能を使うと便利です。PhpStormでは、分けたい行や範囲を選択して、メニューのRefactor＞Refactor this＞7. Extract Method...です。
（１）〜（３）は、メインロジックに手を付けないので、テストケースがない場合でも、比較的安心してリファクタできます。
（１）まず、巨大な文字列は、それだけを返すメソッドに分けます。
public function foo()
{
$str = "長い文字列、複数行かもね:" . $x;Code language: PHP (php)PhpStormでリファクタする場合は、$str = の右辺、セミコロン含まない、"長い文字列、複数行かもね:" . $x
の範囲を選択してから、リファクタします。
public function foo()
{
$str = $this->getStr($a);
}
/**
* @param $a
* @return string
*/
public function getStr($a): string
{
return "長い文字列、複数行かもね:" . $a;
}
Code language: PHP (php)（２）配列リテラルを定義している場合も、それだけを返すメソッドに分けます。
public function foo()
{
$arr = [
['xxx' => 0, 'yyy' => 1],
['xxx' => 1, 'yyy' => 2],
];
Code language: PHP (php)PhpStormでリファクタする場合は、$arr =の右辺、セミコロン含まない
の範囲を選択してから、リファクタします。
public function foo()
{
$arr = $this->getArr();
}
/**
* @param $a
* @return array
*/
public function getArr(): array
{
return [
['xxx' => 0, 'yyy' => 1],
['xxx' => 1, 'yyy' => 2],
];
}
Code language: PHP (php)（３）ガード節と早期リターンが何個もある場合です。
public function foo($x, $y)
{
if ($x === '') {
return;
}
if ($y === '') {
return;
}
// メインロジック
}Code language: PHP (php)（３−１）phpstormのリファクタ機能を使って、メインロジックをメソッドに分けます。
public function foo($x, $y)
{
if ($x === '') {
return;
}
if ($y === '') {
return;
}
$this->fooMainLogick($x, $y);
}
private function fooMainLogic($x, $y)
{
// メインロジック
}Code language: PHP (php)（３−２）手作業で、ガード節をメソッドに分けます。
public function foo($x, $y)
{
if (!$this->fooIsValidate($x, $y)) {
return;
}
$this->fooMainLogic($x, $y);
}
private function fooIsValidate($x, $y)
{
if ($x === '') {
return false;
}
if ($y === '') {
return false;
}
return true;
}
private function fooMainLogic($x, $y)
{
// メインロジック
}Code language: PHP (php)（３−３）fooメソッドごと、別クラスにしてもいいと思います。
public function foo($x, $y)
{
$fooLogic = new FooLogic();
$fooLogic->foo($x, $y);
}
Code language: PHP (php)class FooLogic
{
public function foo($x, $y)
{
if (!$this->isValidate($x, $y)) {
return;
}
$this->mainLogic($x, $y);
}
private function isValidate($x, $y)
{
if ($x === '') {
return false;
}
if ($y === '') {
return false;
}
return true;
}
private function mainLogic($x, $y)
{
// メインロジック
}
}Code language: PHP (php)（４）for文まるごとメソッドに分けます。
public function foo($x, $y)
{
// 何か処理
foreach ($arr as $i => $rcd) {
$rcd['xxx'] = $rcd['yyy'];
$arr[$i] = $rcd;
}
// 何か処理
}Code language: PHP (php)foreach内で$arrを変更しているので、return で返しました。
public function foo($x, $y)
{
// 何か処理
$arr = $this->doDomethingAll($arr);
// 何か処理
}Code language: PHP (php) public function doDomethingAll($arr)
{
foreach ($arr as $i => $rcd) {
$rcd['xxx'] = $rcd['yyy'];
$arr[$i] = $rcd;
}
}
Code language: PHP (php)長いメソッドをリファクタするときは、同じように、if文、switch文をメソッドに分けていきます。
for文やif文ではなくて、意味のある範囲を選んで、メソッドに分けたほうがいいと思うんだけど？
それができないから、モンスターメソッドになったんでしょ
短くしているだけの気がするけど、これってリファクタと言えるのかな？
分けたメソッドなら、ユニットテストを書こうという気になるでしょ。一部分だけでもユニットテストができれば、すごい進歩よ！
まじで面倒だよ、モンスターメソッドが放置されてしまうのもわかるよ。最初からユニットテストを書いたり、短いメソッドを書いたほうが楽だよ
１日２日でモンスターメソッドになったわけではないから、リファクタするのにも時間も手間もかかるのよ。ダイエットと同じね。
メソッドを新しいクラスにする
PhpStormのリファクタ機能で、barメソッドから新しいクラスFooBarを作りました。
class Foo
{
private $x;
private $arr;
public function bar($a)
{
$this->arr = [
['xxx' => 0, 'yyy' => 1],
['xxx' => 1, 'yyy' => 2],
];
$this->x = $this->getStr($a);
$this->arr = $this->arrUpdate($this->arr);
}Code language: PHP (php)元のクラスには、privateプロパティのsetterが自動生成されました。元のクラスの$thisを、新しいクラスのコンストラクタで渡しています。
class Foo
{
private $x;
private $arr;
/** @var FooBar */
private $fooBar;
public function __construct()
{
$this->fooBar = new FooBar($this);
}
public function bar($a)
{
$this->fooBar->bar($a);
}
/**
* @param mixed $arr
*/
public function setArr($arr): void
{
$this->arr = $arr;
}
/**
* @param mixed $x
*/
public function setX($x): void
{
$this->x = $x;
}
Code language: PHP (php)class FooBar
{
private $foo;
public function __construct(Foo $foo)
{
$this->foo = $foo;
}
public function bar($a)
{
$this->foo->setArr([
['xxx' => 0, 'yyy' => 1],
['xxx' => 1, 'yyy' => 2],
]);
$this->foo->setX($this->foo->getStr($a));
$this->foo->setArr($this->foo->arrUpdate($this->foo->getArr()));
var_dump($this->foo->getArr());
}
}Code language: PHP (php)元のクラスはすっきりしました。
しかし、新しいクラスのbarメソッドは、元のbarメソッドより、読みにくくなったようです。$this->foo経由でset/getしているためですね。
PhpStormが自動生成しているので、リファクタ後も同じように動作する可能性が高く、この安心感は捨てがたいです。
これをベースに読みやすいようにリファクタします。
メソッドの最初で、$this->foo->getXX() をローカル変数やprivateプロパティに代入し、
メソッド内では、ローカル変数やprivateプロパティで処理し、
メソッド最後に、$this->foo->setXX()で、結果を設定します。
筆者が考えていたリファクタは、次のようなコードです。
元のBarクラスのbarメソッドを、新FooBarクラスへコピペ。
barメソッドで使っているFooクラスのプロパティを、新FooBarクラスのコンストラクタで参照渡し。
class Foo
{
private $x;
private $arr;
public function bar($a)
{
$fooBar = new FooBar($x, $arr);
$fooBar->$bar($a);
}
Code language: PHP (php)class FooBar
{
private $x;
private $arr;
// barメソッドで使っているプロパティを参照渡し
public function __construct(&$x, &$arr)
{
$this->x = &$x;
$this->arr = &$arr;
}
// 元のFooのbarメソッドからコピペ
public function bar($a)
{
$this->arr = [
['xxx' => 0, 'yyy' => 1],
['xxx' => 1, 'yyy' => 2],
];
$this->x = $this->getStr($a);
$this->arr = $this->arrUpdate($this->arr);
}
Code language: PHP (php)こちらのほうが元のbarメソッドのままで、リファクタしやすいとも思いましたが、こちらの方法は手動です。
barメソッドで使っているprivateフィールドの個数が多いと、コンストラクタを呼ぶ側、コンストラクタ側のコードを正しく書くのが面倒そうです。
ましてやユニットテストなしでは、手動で正しくリファクタできたかどうか不安です。
結局、
（１）元のクラスでリファクタを続ける
（２）PhpStormのリファクタ機能を使って新クラスに分ける
（３）手動で新クラスに分ける
のどれを使うかはケースバイケースですね。
