phpmd codesize警告の解説

codesize

特に、ExcessiveMethodLength（メソッド長すぎ）が重要です。

メソッドを短くすれば、CyclomaticComplexity（循環的複雑度）、NPathComplexity（実行パス数）の値も小さくなります。

リーダブルコードにも「メソッドに分割して、メソッドを小さくしよう」とあります。

その他の「メソッドの個数、メンバー変数の個数が多すぎるよシリーズ」は参考程度に考えて、あまり神経質にならなくてもいいと思います。

CyclomaticComplexity

メッセージ例

The xx yy() has a Cyclomatic Complexity of zz. The configured cyclomatic complexity threshold is aa.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.CyclomaticComplexity)
 */Code language: PHP (php)

説明

Complexity is determined by the number of decision points in a method plus one for the method entry.
The decision points are 'if', 'while', 'for', and 'case labels'.
Generally, 1-4 is low complexity, 5-7 indicates moderate complexity, 8-10 is high complexity, and 11+ is very high complexity.

循環的複雑度は、if、while、for、caseラベルの数から計算します。
1-4は低、5-7は中、8-10は高、11+は非常に複雑です。

コード例

// Cyclomatic Complexity = 11
class Foo {
1   public function example() {
2       if ($a == $b) {
3           if ($a1 == $b1) {
                fiddle();
4           } elseif ($a2 == $b2) {
                fiddle();
            } else {
                fiddle();
            }
5       } elseif ($c == $d) {
6           while ($c == $d) {
                fiddle();
            }
7        } elseif ($e == $f) {
8           for ($n = 0; $n < $h; $n++) {
                fiddle();
            }
        } else {
            switch ($z) {
9               case 1:
                    fiddle();
                    break;
10              case 2:
                    fiddle();
                    break;
11              case 3:
                    fiddle();
                    break;
                default:
                    fiddle();
                    break;
            }
        }
    }
}Code language: plaintext (plaintext)

解説

プログラムの複雑度を計算する有名な方法です。

まず「ExcessiveMethodLength（メソッド長すぎ）」に対応しましょう。メソッドを短くすれば、循環的複雑度の値も小さくなります。

NPathComplexity

メッセージ例

The xx yy() has an NPath complexity of zz. The configured NPath complexity threshold is aa.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.NPathComplexity)
 */Code language: PHP (php)

説明

The NPath complexity of a method is the number of acyclic execution paths through that method.
A threshold of 200 is generally considered the point where measures should be taken to reduce complexity.

NPath複雑度は、実行パス数のこと。
200をこえていたら、かなり複雑。
（リファクタして）複雑さを軽減したほうがいいよ。

解説

これはフローチャートで表現したとき、矢印をたどる経路が何とおりあるか、ということですね。

if文が1つあれば、✕2。elseがなくても、✕2です。

if文が2個あると、4。if文が10個で、1024です。

if elseif elseの3分岐なら、✕3。これもelseがなくても、✕3です。

switch文は、caseやdefaultが4個あれば、✕4。breakがないフォールスルーのcaseでもカウントされます。

for文、while文は、✕2。

メソッド呼び出しは、✕1で、増えません。

1つのメソッドに大きなswitch文があると、すぐに200をこえてしまいます。

まず「ExcessiveMethodLength（メソッド長すぎ）」に対応しましょう。メソッドを短くすれば、NPath複雑度の値も小さくなります。

リファクタ例

(なし)Code language: plaintext (plaintext)

ExcessiveMethodLength

メッセージ例

The xx yy() has zz lines of code. Current threshold is set to aa. Avoid really long methods.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
 */Code language: PHP (php)

説明

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.

メソッド長すぎ！
メソッドの処理内容が多すぎるね。
ヘルパーメソッドに分割して、行数を小さくしよう。
（空行もカウントします）

コード例

public function foo()
{
  $vars = [
    'x' => 'xxxx',
    'y' => 'yyyy',
    つづく
  ];

  if ($x) {
    複数行の処理
  }

  for ($i = 0; $i < $n; $i++) {
    複数行の処理
  }

  switch ($x) {
    case 1:
      複数行の処理
      break;
    case 2:
      複数行の処理
      break;
  }

  // xxxをする
  xxxxxxxxxxxx;   

  yyyyyyyyyyyy;
  yyyyyyyyyyyy;
  yyyyyyyyyyyy;
}
Code language: plaintext (plaintext)

解説

筆者は年をとり、目も悪くなってきたせいか、100行以上の長いメソッドを理解しようする気力が衰えてきたのかもしれません。

筆者が決めていいなら、メソッドは50行までにするかな。

「ThoughtWorksアンソロジー」の第5章「オブジェクト指向エクササイズ」の9つのルールは、もっと厳しくて、クラスが50行までです。メソッドではなく、クラスです。

メソッドが長いことで、いいことは何もありません。誰もが理解していることだと思うけど、プログラミング言語や文法チェッカーが強制しなければ、守れないのかもしれないですね。

それでも時間が許すかぎり、メソッドを短く保ちましょう。

• コメントをつけるよりメソッドに分ける。
• 連続行を空白行で区切りたくなったらメソッドに分ける。（そのかたまりは意味のあるかたまりかもしれない）
• メソッド内の配列リテラルは、それを返すメソッドに分ける。
• 長い配列リテラルをJSONファイルなどの外部ファイルから読み込む。
• for文まるごとメソッドに分ける。
• for文内の処理をメソッドに分ける。
• if文まることメソッドに分ける。
• if文内の処理をメソッドに分ける。
• switch文まるごとメソッドに分ける。
• case文の処理をメソッドに分ける。

リファクタ例

public function foo()
{
  $vars = $this->getVars();

  $this->a();
  $this->b();
  $this->c();

  $this->xxx();
  $this->yyy();
  $this->zzz();
}

public function getVars()
{
  return [
    'x' => 'xxxx',
    'y' => 'yyyy',
    つづく
  ];
}

public function a()
{
  if ($x) {
    $this->a_foo();
  }
}

public function a_foo()
{
}

public function b()
{
  for ($i = 0; $i < $n; $i++) {
    $this->b_foo();
  }
}

public function b_foo()
{
}

public function c()
{
  switch ($x) {
    case 1:
      $this->c_case1();
      break;
    case 2:
      $this->c_case2();
      break;
  }
}

public function c_case1()
{
}

public function c_case2()
{
}

public function xxx()
{
  xxxxxxxxxxxx;
}

public function yyy()
{
  yyyyyyyyyyyy;
  yyyyyyyyyyyy;
}
Code language: PHP (php)

テストケースから呼ぶため、publicにしています。

正しいお作法は、protectedで宣言しておき、テストケース用の派生クラスでpulbicでオーバーライドするんですね。どうも面倒なので、最初からpublicにしてしまうことが多いです。

モンスターメソッド（phpmdのExcessiveMethodLengthをリファクタするには）

いつメソッドを短くするの？今でしょExcessiveMethodLengthとはメソッドの行数が多すぎる（縦に長い）という警告です。最初にリファクタしてほしい警告であり、常に気をつけてほしい警告です。phpmdの既定値は100行ですが、まだ...

www.ninton.co.jp

2021.01.07

ExcessiveClassLength

メッセージ例

The class xx has yy lines of code. Current threshold is zz. Avoid really long classes.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.ExcessiveClassLength)
 */Code language: PHP (php)

説明

Long Class files are indications that the class may be trying to do too much. Try to break it down, and reduce the size to something manageable.

長いクラスは、多くのことをしすぎているかもね。クラスを分割して、扱いやすいサイズに小さくしよう。
（1000行以下。空白行をカウントします）

解説

クラスの行数が長いということは、メソッドの行数が長い、または、メソッド数が多いということです。長いメソッドはリファクタしたほうがいいです。メソッド数の多さをリファクタするには、クラスに分割するしかなく、他への影響が大きいのでリファクタ優先度は低くてもいいと思います。

長いクラスをリファクタするのなら、

• インスタンス変数にアクセスしないメソッドはstaticメソッドにして、新しくヘルパークラスを作って移動する。
• 長いメソッドをそれだけの新しいクラスに移動する。

をするぐらいで、無理にリファクタすることはないと思います。

長いクラスの、既存メソッドに機能追加したり、新しいメソッドを追加する場合は、

• スプラウトクラス（レガシーコード改善ガイド 6.2）

を使います。

まず、新しく追加する機能を、新しいクラスとして実装します。既存クラスに新しいメソッドを作り、そこで新しいクラスを呼び出します。既存メソッドの該当箇所では、新しいメソッドを呼び出します。

既存クラスの膨張を最小限に抑えることができます。

リファクタ例

class Foo
{
  public function bar()
  {
      // 既存処理

      $this->sprout();  // ★既存メソッドに機能追加

      // 既存処理
  }

  public function sprout()  // ★既存クラスに新規メソッド
  {
    $context = [
      // 既存クラスのプロパティから、Barで必要なものを渡す
    ];
    $bar = new Bar($context);
    $bar->main();
  }
}

class Bar
{
    public function __constrcut($context)
    {
    }

    public function main()
    {
    }
}Code language: plaintext (plaintext)

ExcessiveParameterList

メッセージ例

The xx yy has zz parameters. Consider reducing the number of parameters to less than aa.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.ExcessiveParameterList)
 */Code language: PHP (php)

説明

Long parameter lists can indicate that a new object should be created to wrap the numerous parameters. Basically, try to group the parameters together.

メソッドの引数リストが長いよ。引数をまとめた新しいオブジェクトを作ったほうがいいよ。（すべての引数をまとめるのではなく）引数のグループ化を考えよう
（引数は10個まで）

コード例

class Foo
{
    public function addData(
        $p0, $p1, $p2, $p3, $p4, $p5,
        $p5, $p6, $p7, $p8, $p9, $p10) {
    }
}Code language: PHP (php)

解説

すでに複数の箇所でこのメソッドを使っていると、引数の個数を減らすのは難しいです。テストケースがあるなら、テストケースも修正しなくてはいけません。

この警告も無理にリファクタすることはありません。

なので、新しいメソッドを作るときに、引数の個数に注意しましょう。

引数の個数が多くなると、呼ぶ側も、呼ばれるメソッド本体の側も、読みにくくなります。

最近は、ローカル変数名も長いことが多いので、3〜4個並べただけで、1行120桁をこえることがあります。

メソッドは縦にも横にも短いほうがいいですね。

リファクタ例

class FooParams
{
   var $p0;
   ...
   var $p10;
}

class Foo
{
    public function addData($params)
    {
    }
}Code language: PHP (php)

ExcessivePublicCount

メッセージ例

The xx yy has zz public methods and attributes. Consider reducing the number of public items to less than aa.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.ExcessivePublicCount)
 */Code language: PHP (php)

説明

A large number of public methods and attributes declared in a class can indicate the class may need to be broken up as increased effort will be required to thoroughly test it.

クラスのpublicメソッドとpublic変数の個数が多すぎるよ。クラス全体のテストを網羅するのが大変だから、クラスを分割できないか考えよう。
（45個まで、getter/setter含む）

コード例

public class Foo
{
    public $value;
    public $something;
    public $var;
    // [... more more public attributes ...]

    public function doWork() {}
    public function doMoreWork() {}
    public function doWorkAgain() {}
    // [... more more public methods ...]
}Code language: PHP (php)

解説

メソッドの個数、メンバー変数の個数が多すぎるよシリーズの第1弾です。

(getter、setterを含むpublicメソッドの総数) + (publicメンバー変数の総数) <= 45

この警告と同時に、クラスが1000行をこえているよ警告もでていて、誰の手にも負えなくなっている可能性があります。

新しくクラスを作るときは、この警告に注意したほうがいいです。

「メソッドの個数、メンバー変数の個数が多すぎるよシリーズ」は、このクラスは普通より複雑だ、と知ることが大事です。この警告のためだけに、既存のクラスを急いでリファクタすることはありません。

TooManyFields

メッセージ例

The xx yy has zz fields. Consider redesigning yy to keep the number of fields under aa.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.TooManyFields)
 */Code language: PHP (php)

説明

Classes that have too many fields could be redesigned to have fewer fields, possibly through some nested object grouping of some of the information. For example, a class with city/state/zip fields could instead have one Address field.

フィールド（メンバー変数）が多すぎるよ。フィールドをグループ化しよう。例えば、city、state、zipの3つのフィールドは、Addressクラスを作って、addressフィールドにできるよ。

（15個まで）

コード例

class Foo
{
    private $city;
    private $state;
    private $zip;
    ... many more fields ...
}Code language: plaintext (plaintext)

解説

メソッドの個数、メンバー変数の個数が多すぎるよシリーズの第2弾です。

publicやprotected、privateなど、全てのメンバー変数の総数は、15個まで。

「メソッドの個数、メンバー変数の個数が多すぎるよシリーズ」は、このクラスは普通より複雑だ、と知ることが大事です。この警告のためだけに、既存のクラスを急いでリファクタすることはありません。

リファクタ例

class Foo
{
    private $address;
    ... many more fields ...
}

class Address
{
    public $city;
    public $state;
    public $zip;
}Code language: PHP (php)

TooManyMethods

メッセージ例

The xx yy has zz non-getter- and setter-methods. Consider refactoring yy to keep number of methods under 25.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.TooManyMethods)
 */Code language: PHP (php)

説明

A class with too many methods is probably a good suspect for refactoring, in order to reduce its complexity and find a way to have more fine grained objects.

By default it ignores methods starting with 'get' or 'set'.

The default was changed from 10 to 25 in PHPMD 2.3.

（25個まで、setter/getterを除く）

解説

メソッドの個数、メンバー変数の個数が多すぎるよシリーズの第3弾です。

publicやprotected、privateなど、メソッドの総数は、25個まで。getterやsetterを除く。

「メソッドの個数、メンバー変数の個数が多すぎるよシリーズ」は、このクラスは普通より複雑だ、と知ることが大事です。この警告のためだけに、既存のクラスを急いでリファクタすることはありません。

TooManyPublicMethods

メッセージ例

The xx yy has zz public methods. Consider refactoring yy to keep number of public methods under 10.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.TooManyPublicMethods)
 */Code language: PHP (php)

説明

A class with too many public methods is probably a good suspect for refactoring, in order to reduce its complexity and find a way to have more fine grained objects.

By default it ignores methods starting with 'get' or 'set'.

（最大10個まで、setter/getter除く）

解説

メソッドの個数、メンバー変数の個数が多すぎるよシリーズの第4弾です。

publicメソッドの総数は、10個まで。getterやsetterを除く。

「メソッドの個数、メンバー変数の個数が多すぎるよシリーズ」は、このクラスは普通より複雑だ、と知ることが大事です。この警告のためだけに、既存のクラスを急いでリファクタすることはありません。

ExcessiveClassComplexity

メッセージ例

The class xx has an overall complexity of yy which is very high. The configured complexity threshold is zz.Code language: plaintext (plaintext)

抑止方法

/**
 *
 * @SuppressWarnings(PHPMD.ExcessiveClassComplexity)
 */Code language: PHP (php)

説明

The Weighted Method Count (WMC) of a class is a good indicator of how much time and effort is required to modify and maintain this class.
The WMC metric is defined as the sum of complexities of all methods declared in a class.
A large number of methods also means that this class has a greater potential impact on derived classes.

加重メソッドカウント（WMC）は、このクラスを変更および維持するために必要な時間と労力の良い指標です。
WMCメトリックは、クラス内の各メソッドの（循環的）複雑度の合計です。
WMCメトリックの値が大きいということは、（このクラスが複雑というだけでなく）派生クラスにも潜在的な影響があるかもしれません。
（最大50）

解説

各メソッドのCyclomatic Complexity（循環的複雑度）を合計して、クラス全体の複雑度を表したものです。

循環的複雑度=5のメソッドが10個あると、50です。

循環的複雑度=10のメソッドが5個でも、50です。

簡単なメソッドは多くてもいいけど、複雑なメソッドは少しだけ、ということですね。

PHP Mess Detector (phpmd)の警告の一覧

phpmdとはPHPコードの静的解析ツールです。バッドプラクティスな箇所を指摘してくれます。どこからリファクタリングすればいいかの目安にもなります。ソースを書いた人も、レビューする人にも便利なツールです。古くは、2012年に1.4.0で。p...

www.ninton.co.jp

2020.11.13