リファクタリングについて

 たまにコードレビューなどをしてもらうと、自分ではこれでいいや、これが綺麗な書き方だなんて思っていたものが
全然コーディング規約に反していることが分かったりして”え、そういえば俺はなんでこれでいいと思っていたんだろ”と疑問に思ったりできるものだ。


 例えば自分の場合、PHPの開始タグの下にはインデントが入るものだと思い込んでいた。

<?php
  $hoge = hogehoge();
  echo $hoge;
?>

 のような感じである。しかし、これは完全にダメである。開始タグの下にインデントはいらないし、PHPの終了タグも、
そのコードがPHPオンリーのソースであれば、省略するべきだとされているのだ。
 つまりこうである。

<?php
$hoge = hogehoge();
echo $hoge;


 終了タグについては、危険性があるという理由があるそうなのだが、インデントについては開始タグをスコープとして誤解してしまっていたという
勘違いが原因であった。つまりif()とかfor()につける{}のように勝手に誤解してしまっていたのであった。


 まあそれはそうと、コーディング規約と言えば、変数の命名規則も重要だとされている。自分の場合は、例えば

$hogeHogeHoge = hogeHogeHoge();

 のように、先頭文字を小文字にして単語毎に頭文字を大文字でつないでいく書き方をどこかでこういうもんなのかと覚えていたのだが、
これはローワーキャメルケースと呼ばれるらしい。命名規則に名前があるという事自体も自分はほとんど意識してこなかった。
知っている記法と言えば、ハンガリアン記法とかいうものがあるらしいということぐらいであった。
ハンガリアン記法というのは、こういう奴だ。

$strText = '';

 型名などの略語が頭につく。C言語の世界ではよく見られるらしい。


 PHPはというと、言語リファレンスを見ても、標準の関数がすべてスニークケースと呼ばれる命名規約にしたがって言語自体が設計されている。
例えば、

array_change_key_case();

のように、すべて小文字の単語をアンダースコアで連結した形である。


 が、一方ではPHPを利用したフレームワークであるCakePHP

$this->getDataSource();

 のようにローワーキャメルで設計されている(が、公式ドキュメントのサンプルにはスニークケースのサンプルコードも散見される)。


 いくつかの命名規約を、例えばビュー側、フレームワーク標準の変数や関数、自ら定義した変数や関数、などで使い分けて区別しやすくするという
使い分けなどがあるのだろうか。


 ロジックについても、綺麗に書くためには色々なテクニックがあるようだ。
例えば我々はつい、こういうコードを書いて悦に入ってしまう。

<?php
foreach($this->Model->find('all') as $row) {
  // なになに処理
}

 これならまだしもまだ理解できるが、実際にはもっと複雑なものを一行に詰めようとしてしまっているかもしれない。
後々訳が分からなくならないようにするためには、一旦内容を適切に表した変数にいれておいたほうがいい。

<?php
$logData = $this->Model->find('all');
foreach($logData as $row) {
  // なになに処理
}

 行が短くなり、名前がついていることで内容はずっと分かりやすくなる。色々な事を記憶しながら読むことを強いられるコードは
ちょっと読むのが大変なのでそうならないようにしたい。


 また、コードを読みにくくしてしまう原因の一つに、巨大で深いスコープがある。

<?php
foreach($result as $row) {
  if($row != 1) {
    if($row != 2) {
      if($row != 3) {
        If($row != 4) {
          // なになに
        } else {
          // hogehoge
        }
      }
    } else {
      if($row === 3) {
        // なんとか
      }
    }
  }
}

 このコードは一見意味がわからない。しかし整理すると下のように直すことができる。すると処理内容は同じだが意味が分かりやすくなる。

<?php
foreach ($result as $row) {
  if(! in_array($row, array(2, 3, 4)) {
    // なになに
  } elseif ($row === 4) {
    // hogehoge
  } else if($row === 3) {
    // なんとか
  }
}

 あるいはこういうロジックがあったとする。

<?php
foreach ($users as $id => $user) {
  if($id >= 50000) {
    if ($user['isLogin'] == true) {
      cnt++;
    } else {
      msg = 'test';
    }

    // なにかしら
    // とてもながい処理
  }
}

 これは条件を反転すると下のようになる。このほうがインデントが浅くなってわかりやすい。真になる条件を重ねないで、ループを抜けられるところは抜けてしまうのが良しとされる。

<?php
foreach ($users as $id => $user) {
  if($id < 50000) {
    continue;
  }
  if ($user['isLogin'] == true) {
    cnt++;
  } else {
    msg = 'test';
  }

  // なにかしら
  // とてもながい処理
}