レガシープロジェクトで phpcs Generic.Files.LineLength をリファクタするには

概要

Line exceeds 120 characters; contains 192 characters
Code language: plaintext (plaintext)

1行が長すぎる(横に長い)、という警告ですね。

PHPに限らず、かつては1行80文字まででしたが、ディスプレイが大きくなり、1行120文字まで増えました。

しかし、クラス名もメソッド名も変数名も長くなり、プロパティを参照すると $this-> で7文字使います。1行で表現できる内容は昔と変わっていないか、少なくなっているかもしれません。

引数並びが長い

メソッドや関数の引数並びが長い場合は、1行1引数に改行します。

関数やメソッドを定義している側

function foo($aaaaaaaaa1, $aaaaaaaaa2, $aaaaaaaaa3, $aaaaaaaaa4, $aaaaaaaaa5, $aaaaaaaaa6, $aaaaaaaaa7, $aaaaaaaaa8, $aaaaaaaaa9) { }
Code language: PHP (php)

1行1引数に改行します。

function foo( $aaaaaaaaa1, $aaaaaaaaa2, $aaaaaaaaa3, $aaaaaaaaa4, $aaaaaaaaa5, $aaaaaaaaa6, $aaaaaaaaa7, $aaaaaaaaa8, $aaaaaaaaa9 ) { }
Code language: PHP (php)

関数やメソッドを呼んでいる側

$str = sprintf("長いフォーマット文字列", $aaaaaaa1, $aaaaaaa2, $aaaaaaa3);
Code language: PHP (php)

1行1引数に改行します。

$str = sprintf( "長いフォーマット文字列", $aaaaaaa1, $aaaaaaa2, $aaaaaaa3 );
Code language: PHP (php)

関数やメソッドに分けてしまうのもいい方法です。

$str = myFormat($aaaaaaa1, $aaaaaaa2, $aaaaaaa3); function myFormat($a1, $a2, $a3); $fmt = "長いフォーマット文字列"; $str = sprintf($fmt, $a1, $a2, $a3); return $str; }
Code language: PHP (php)

計算式が長い

計算式に、メソッド呼び出しを含んでいて長い場合は、

$x = $y * anotherFunc1($aaaaaaa1, $aaaaaaa2, $aaaaaaa3) + anotherFunc2($bbbbbbb1, $bbbbbbb2, $bbbbbbb3);
Code language: PHP (php)

メソッドの結果を変数に代入してから、計算します。

$a1 = anotherFunc1($aaaaaaa1, $aaaaaaa2, $aaaaaaa3); $a2 = anotherFunc2($bbbbbbb1, $bbbbbbb2, $bbbbbbb3); $x = $y * $a1 + $a2;
Code language: PHP (php)

ifの条件式が長い

ifの条件式が長い場合は、

// リファクタ前 if ($a1->a1 * $b1->b1 < $c1->c1 * $d1->d1 + $e1->e1() || $a2->a2 * $b2->b2 < $c2->c2 * $e2->e2()) {
Code language: PHP (php)

|| や && を区切りに改行したり、

if ($a1->a1 * $b1->b1 < $c1->c1 * $d1->d1 + $e1->e1() || $a2->a2 * $b2->b2 < $c2->c2 * $e2->e2()) {
Code language: PHP (php)

条件部の計算や比較をメソッドに分けます。

if ($this->isSomething1($a1->a1, $b1->b1, $c1->c1, $d1->d1, $e1->e1()) || $this->isSomething2($a2->a2, $b2->b2, $c2->c2, $e2->e2())) { } function isSomething1($a, $b, $c, $d, $e) { return $a * $b < $c * $d + $e; } function isSomething2($a, $b, $c, $e) { return $a * $b < $c * $e; }
Code language: PHP (php)

さらに、次のように書くとデバッガでトレースしやすいですが、さきほどの例とは意味が違うので、注意が必要です。

$f1 = $this->isSomething1($a1->a1, $b1->b1, $c1->c1, $d1->d1, $e1->e1()); $f2 = $this->isSomething2($a2->a2, $b2->b2, $c2->c2, $e2->e2()); if ($f1 || $f2) { }
Code language: PHP (php)

何が違うのかな?

$e2->e2()が呼ばれるたびに、$e2内部のカウンタをインクリメントしているとするわね

1つめの例で、isSomething1()がtrueとします。isSomething2()は呼ばれません。$e2内部のカウンタはいくつかしら?

初期値0とするね。呼ばれないんだから、0のままだね

ピンポーン

2つめの例も、isSomething1()がtrueとします。常にisSomething2()は呼ばれます。$e2内部のカウンタはいくつかしら?

1回呼ばれるから、1だね。さっきと違うね!

ピンポーン。&&は左側がfalseなら、右側の判定はしないの。

じゃあメソッドを呼ばず、プロパティだけを使っていれば、1つめの例も2つめの例も同じかな?

通常は同じね。でもgetterのマジックメソッドを実装しているかもしれないから、断言はできないわ。

HTMLタグが長い

HTMLを表示するphpの場合です。

タグを入れ子で改行して、1行1タグにしておくだけでいいと思います。

<input type="text" id="xxxx" name="yyyy" value="<?php echo $xxxx; ?>" />
Code language: PHP (php)

長いタグを途中で改行したり、

<input type="text" id="xxxx" name="yyyy" value="<?php echo $xxxx; ?>" />
Code language: PHP (php)

属性ごとに改行したり。ここは好みにおまかせします。

<input type="text" id="xxxx" name="yyyy" value="<?php echo $xxxx; ?>" />
Code language: PHP (php)

AタグのURLクエリーが長い

出力する直前でurlエンコード、htmlエンコードしたほうがいいのですが、それを守ると、クエリー変数2つ並べると、120文字をオーバーしてしまいます。

<a href="xxxx.php?category=<?php echo htmlspecialchars(urlencode($category));?>&amp;from_date=<?php echo htmlspecialchars(urlencode($from_date));?>">
Code language: PHP (php)

方法1:urlエンコード、htmlエンコード済みの変数を用意します。

Aタグは少し短くなります。HTMLタグに混じってPHPコードがあるので、ゴチャゴチャした感じになります。Aタグごとに似たようなエンコード処理を書くことになります。

<?php $hu_category = htmlspecialchars(urlencode($category)); $hu_from_date = htmlspecialchars(urlencode($from_date)); ?> <a href="xxxx.php?category=<?php echo $hu_category;?>&amp;from_date=<?php echo $hu_from_date;?>">
Code language: PHP (php)

方法2:短い名前のヘルパー関数をphp側のグローバルに用意しておきます。

<?php function ehu($str) { echo htmlspecialchars(urlencode($str)); } ?>
Code language: PHP (php)

短くなりました。名前が短すぎて何をしているか連想しやすいかというと...

<a href="xxxx.php?category=<?php ehu($category);?>&amp;from_date=<?php ehu($from_date);?>">
Code language: PHP (php)

方法3:build_http_query関数を使って、URLクエリーを作成します。

<?php $vars = [ 'category' => $category, 'from_date' => $from_date, 'to_date' => $to_date, ]; $query = build_http_query($vars); ?> <a href="xxxx.php?<?php echo htmlspecialchars($query);?>">
Code language: PHP (php)

外部ファイルに関数定義しておき、

<?php function xxxx_build_query($category, $from_date, $to_date) { $vars = [ 'category' => $category, 'from_date' => $from_date, 'to_date' => $to_date, ]; $query = build_http_query($vars); return htmlspecialchars($query); }
Code language: PHP (php)

ファイル先頭でrequire_onceします。

<?php require_once __DIR__ . '/xxxx_helper.php'; ?> <a href="xxxx.php?<?php echo xxxx_build_query($category, $from_date, $to_date);?>">
Code language: PHP (php)

SQL文が長い

生のSQLで、SELECTするカラムを "*" ではなく、一つ一つ指定すると、すぐに1行が長くなり、phpcsのLineLengthに引っかかります。

public function selectXxx() { .... $sql = "SELECT id, name, category, aaa, bbb, ccc, ddd FROM ... WHERE ... ORDER BY ...;"; .... }
Code language: PHP (php)

まず、元々の箇所からコピペして、SQL文だけを返すメソッドに分けて、phpcs:ignoreしておきます。

public function selectXxx() { .... $sql = $this->sqlToSelectXxx(); .... } public function sqlToSelectXxx() { // phpcs:ignore Generic.Files.LineLength $sql = "SELECT id, name, category, aaa, bbb, ccc, ddd FROM ... WHERE ... ORDER BY ...;"; return $sql; }
Code language: PHP (php)

元々の場所で、SQL文字列内で変数を参照している場合は、

public function selectXxx() { .... $sql = "SELECT id, name, category FROM ... WHERE date >= '$date_from' ORDER BY ...;"; .... }
Code language: PHP (php)

参照している変数をメソッドの引数で渡します。

public function selectXxx() { .... $sql = $this->sqlToSelectXxx($date_from); .... } public function sqlToSelectXxx($date_from) { // phpcs:ignore Generic.Files.LineLength $sql = "SELECT id, name, category FROM ... WHERE date >= '$date_from' ORDER BY ...;"; return $sql; }
Code language: PHP (php)

phpMyAdminのSQLフォーマット風に改行してあると行数が長くなり、phpmdの1メソッド100行にひっかかります。

元のメソッドを @SupressWarnings(PHPMD.ExssesiveMethodLength)で抑制するよりは、

/** * * @SupressWarnings(PHPMD.ExssesiveMethodLength) */ public function selectXxx() { .... $sql = " SELECT id, name, category FROM ... WHERE date >= '$date_from' ORDER BY ...; "; .... }
Code language: PHP (php)

SQL文だけをメソッドに分けて、分けたメソッドを@SupressWarningsで抑制したほうがいいでしょう。元々のメソッドの本来の行数の長さでphpmdをかけることができます。

public function selectXxx() { .... $sql = $this->sqlToSelectXxx($date_from); .... } /** * * @SupressWarnings(PHPMD.ExssesiveMethodLength) */ public function sqlToSelectXxx($date_from) { $sql = " SELECT id, name, category FROM ... WHERE date >= '$date_from' ORDER BY ...; "; return $sql; }
Code language: PHP (php)

コピペしたSQL文は、リファクタせずに、そのままにしておきます。この手のSQL文を編集する機会はとても少ないので、そのままでも問題ないと思います。

SQL文をリファクタする場合は、テスト用にインポートするデータを用意して、selectXxxのユニットテストを書いてからにします。

SQL文のここは間違いかも?と思っても、修正も慎重になりましょう。

ユニットテストがない状態で、修正やリファクタする場合は、SQL文を表示して、それをコピペして、phpMyAdminやmysqlコマンドで動作確認します。

何回も動作確認するのが面倒になって、ユニットテストを書くようになるのよ

文字列が長い

巨大文字列は、別ファイルにしましょう。

public function foo() { .... $text = " とにかく長い $name とにかく長い $category "; .... }
Code language: PHP (php)

まず、文字列だけを返すメソッドに切り分けます。

public function foo() { .... $text = $this->getTextFoo($name, $category); .... } /** * * @SupressWarnings(PHPMD.ExssesiveMethodLength) */ public function getTextFoo($name, $category) { $text = " とにかく長い $name とにかく長い とにかく長い $category とにかく長い "; return $text; }
Code language: PHP (php)

文字列を別のphpファイル、foo_text.phpにします。

$nameや、$categoryなどの変数を参照している箇所は、<?php echo $name; ?><?php echo $category; ?>にします。

とにかく長いかく長い<?php echo $name; ?> とにかく長いかく長い<?php echo $category; ?> とにかく長いかく長い
Code language: PHP (php)

つまり、HTMLを表示するphpファイルと同じ使い方です。

バックスラッシュエスケープは必要ありません。ダブルクォート、シングルクォートも書きたいように書くことができます。

気をつけることは、行末に <?php echo $name; ?>を書くと、その行は改行されません。そこで、改行するための空行を一つ入れます。

とにかく長い <?php echo $name; ?>

とにかく長い <?php echo $category; ?>

元のphpファイルに戻ります。

一時的に新メソッドgetTextFoo_v2を作ります。foo_text.phpを(require_onceではなく)requireします。このままではfoo_text.phpの内容が表示されてしまうので、ob_start()とob_get_clean()を使って変数にキャプチャします。

public function foo() { ... $text = $this->getTextFoo($name, $category); ... } public function getTextFoo($name, $category) ... } public function getTextFoo_v2($name, $category) { ob_start(); require __DIR__ . '/foo_text.php'; $text = ob_get_clean(); return $text; }
Code language: PHP (php)

簡易的な動作確認として、foo()メソッド内で比較するコードを書いておきます。デバッガのステップ実行で$fの値がtrueかどうか確認したり、$fの値をvar_dumpします。

(最後にユニットテストの例があります)

public function foo() { .... $text = $this->getTextFoo($name, $category); $text2 = $this->getTextFoo_v2($name, $category); $f = $text === $text2; var_dump($f); .... } public function getTextFoo($name, $category) } public function getTextFoo_v2($name, $category) { ob_start(); require __DIR__ . '/foo_text.php'; $text = ob_get_clean(); return $text; }
Code language: PHP (php)

古いメソッドを削除し、getTextFoo_v2メソッドをgetTextFooにリネームして、リファクタ完了です。

public function foo() { .... $text = $this->getTextFoo($name, $category); .... } public function getTextFoo($name, $category) { ob_start(); require __DIR__ . '/foo_text.php'; $text = ob_get_clean(); return $text; }
Code language: PHP (php)

この例はデータベースを使わないので、ユニットテストを書いて、テスト実行しましょう。

元の巨大文字列をユニットテスト側にコピペしておきます。そのメソッドの結果を$expected、新しいメソッドの結果を$actualとして、$this->assertEqualsで比較します。

/** * @test */ public function getTextFoo() { $name = "太郎"; $category = "カテA"; $target = new Bar(); $actual = $target->getTextFoo($name, $category); $expected = $this->getTextFoo_old($name, $category); $this->assertSame($expected, $actual); } private function getTextFoo_old($name, $category) { $text = " とにかく長い $name とにかく長い とにかく長い $category とにかく長い "; return $text; }
Code language: PHP (php)

タイトルとURLをコピーしました