4日目: リファクタリング

復習

３日目までは、MVC構造の全てのレイヤーを紹介し、トップページに質問のリストを表示するように変更を加えました。アプリケーションとしては良くなってきましたが、まだまだ内容が足りません。

４日目の目標は、質問への答えのリストを表示すること、質問の詳細へのもっと良いURLの設定、カスタムクラスの追加、重複したコードの塊をあるべき場所へ移動することです。これらによってテンプレート、モデル、ルーティングポリシー、リファクタリングの概要を理解できます。数日しか経っていないコードを変更するにはちょっと早すぎるのでは？と思うかもしれませんが、今日の終わりにはその理由が分かるでしょう。

このチュートリアルを読むには、symfonyのMVCの実装方法を良く理解しておく必要があります。 アジャイル開発 についての理解もあるとよりよいかと思います。

質問に対しての回答の表示

まず始めに、2日目 のQuestionのCRUDで生成したテンプレートから続けましょう。

question/showアクションは、idを渡すことによって質問の詳細を表示します。テストするには、次のコードを呼び出すだけです(2をテーブルに実在する質問idに変更しなければなりません):

http://askeet/frontend_dev.php/question/show/id/2

前にちょっと触ったことがある人ならこのshowページを見たことがあるかもしれません。ここが質問に対する答えを追加する場所になります。

このアクションについて少し

それではaskeet/apps/frontend/modules/question/actions/actions.class.phpファイルにあるこのshowアクションを見てみましょう:

public function executeShow()
{
  $this->question = QuestionPeer::retrieveByPk($this->getRequestParameter('id'));
  $this->forward404Unless($this->question);
}

Propelを知っている人なら、ここではQuestionテーブルに対して簡単なリクエストが発行されていると分かるかもしれません。ここではリクエストでのid引数を主キーとする、ユニークなレコードを取得しようとしています。先程の例でのURLでは、id引数は1であり、QuestionPeerクラスの->retrieveByPk()メソッドは、主キーを1とするQuestionクラスを返すことになります。Propelにあまり詳しくない人は、Propelのサイトにあるドキュメント を読んでみてください。

このリクエストの結果は$question変数を通してshowSuccess.phpテンプレートに渡されます。

sfActionオブジェクトの->getRequestParameter('id')メソッドは、GET/POSTに関わらずidという引数を受け取ります。たとえば、下のように呼んだ場合:

http://askeet/frontend_dev.php/question/show/id/1/myparam/myvalue

showアクションでは、$this->getRequestParameter('myparam')と書くことでmyvalueを受け取ることができます。

showSuccess.phpテンプレートの変更

生成されたshowSuccess.phpテンプレートは、今自分たちが必要な形ではないので、完全に書き換えてしまいます。frontend/modules/question/templates/showSuccess.phpファイルを開いて、内容を以下のものに変えてください:

<?php use_helper('Date') ?>
 
<div class="interested_block">
  <div class="interested_mark" id="mark_<?php echo $question->getId() ?>">
    <?php echo count($question->getInterests()) ?>
  </div>
</div>
 
<h2><?php echo $question->getTitle() ?></h2>
 
<div class="question_body">
  <?php echo $question->getBody() ?>
</div>
 
<div id="answers">
<?php foreach ($question->getAnswers() as $answer): ?>
  <div class="answer">
    posted by <?php echo $answer->getUser()->getFirstName().' '.$answer->getUser()->getLastName() ?> 
    on <?php echo format_date($answer->getCreatedAt(), 'p') ?>
    <div>
      <?php echo $answer->getBody() ?>
    </div>
  </div>
<?php endforeach; ?>
</div>

ここで昨日listSuccess.phpテンプレートにあったdiv要素のinterested_blockに気づくかもしれません。これは特定の質問に対しての関心を示しているユーザーの数を表示するだけです。それ以外でHTML自体は、タイトルにlink_toが無いだけでlistのような内容です。これはただ質問に対しての必要な情報だけを表示する初期のコードを書き換えただけです。

新しい部分はdiv要素のanswersです。ここは質問に対しての全ての回答を表示し(Propelの$question->getAnswers()メソッドを使用)、その個々で、信頼性の合計、回答した人の名前、日付、内容が表示されます(訳注：「信頼性の合計」はコード中にないと思う SVNを見るとある)。

format_date()は初期宣言が必要なテンプレートヘルパーのもう1つの使用例です。ヘルパーの構文やその他のヘルパーについては、symfony bookの国際化ヘルパーの章(これらのヘルパーは面倒な日付のフォーマットを楽にしてくれます)を参照してください。

テストデータの追加

それではdata/fixtures/test_data.ymlの最後の行からanswerとrelevancyテーブルのデータを追加してみましょう(その他独自のデータを入れても構いませんよ):

Answer:
  a1_q1:
    question_id: q1
    user_id:     francois
    body:        |
      You can try to read her poetry. Chicks love that kind of things.

  a2_q1:
    question_id: q1
    user_id:     fabien
    body:        |
      Don't bring her to a donuts shop. Ever. Girls don't like to be
      seen eating with their fingers - although it's nice. 

  a3_q2:
    question_id: q2
    user_id:     fabien
    body:        |
      The answer is in the question: buy her a step, so she can 
      get some exercise and be grateful for the weight she will
      lose.

  a4_q3:
    question_id: q3
    user_id:     fabien
    body:        |
      Build it with symfony - and people will love it.

下のコマンドでデータをリロードします:

$ php batch/load_data.php

変更がうまくいったか質問を表示するアクションにアクセスしましょう:

http://askeet/frontend_dev.php/question/show/id/XX

質問に加え回答も表示され、なかなか良い感じになりましたね。

モデルの変更 パート１

このアプリケーションとして、書き込みをした人のフルネームをまたどこかで使うことはほぼ確実です。また、フルネームと言うだけにUserオブジェクトの属性の一部になると考えられます。この事からするとアクション内でフルネームを組み立てるのではなく、Userモデルにフルネームを取得することができるメソッドがあるべき姿のようです。それでは書いてみましょう。askeet/lib/model/User.phpを開いて下のメソッドを追加してください:

public function __toString()
{
  return $this->getFirstName().' '.$this->getLastName();
}

なぜこのメソッドはgetFullName()みたいなものではなくて__toString()なのでしょう？PHP 5において、__toString()メソッドはオブジェクトを文字として表現するために使われるデフォルトのメソッドだからです。ということは同じ結果を表示するにも

posted by <?php echo $answer->getUser()->getFirstName().' '.$answer->getUser()->getLastName() ?> 

askeet/apps/frontend/modules/question/templates/showSuccess.phpテンプレートのこの行は

posted by <?php echo $answer->getUser() ?> 

のように置き換えることができます。綺麗にまとまりましたよね？

同じことは繰り返さない

アジャイルな開発で重複するコードは避けるという良い教訓があります。これは "Don't Repeat Yourself" (D.R.Y.)(訳注：同じことは繰り返さない)と言われています。重複するコードはカプセル化されたコードを使うより、レビューするにも、変更するにも、テストするにも、確認するにも2倍かかってしまいますし、アプリケーションのメンテナンス性も下げます。また、今日のチュートリアルの最後の部分を見てみると、昨日書かれたlistSuccess.phpテンプレートとshowSuccess.phpには、重複部分があることに気づくと思います:

<div class="interested_block">
  <div class="interested_mark" id="mark_<?php echo $question->getId() ?>">
    <?php echo count($question->getInterests()) ?>
  </div>
</div>

よって最初の リファクタリング は、この２つのテンプレートから重複する部分を抜き出し、フラグメント(コードの断片) 又は再利用可能なコードにしていきます。_interested_user.phpというファイルをaskeet/apps/frontend/modules/question/template/ディレクトリに作り、下のコードを書いてください:

<div class="interested_mark" id="mark_<?php echo $question->getId() ?>">
  <?php echo count($question->getInterests()) ?>
</div>

そして、元の (listSuccess.phpとshowSuccess.php) で、抜き出した場所を下のように変更してください:

<div class="interested_block">
  <?php include_partial('interested_user', array('question' => $question)) ?>
</div>

フラグメントはオブジェクトに対するアクセスを持っていません。このフラグメントでは$question変数を使っているので、include_partialの呼び出しで定義されなくてはいけません。また、フラグメントのファイル名にある_は、template/ディレクトリにおいてフラグメントとそうでないものとを区別するために使われています。フラグメントについて詳しくは、symfonyブックの ビューの章 を読んでください。

モデルの変更 パート２

新しく作ったこのフラグメントで$question->getInterests()はデータベースにリクエストを発行し、Interestクラスのオブジェクト配列を返します。これでは関心のある人の数を持ってくるだけにしてはかなり重い処理で、データベースへの処理もかなりのものになってしまいます。listSuccess.phpテンプレートでも同じ事をして、しかも質問のリスト内でのループです。ちょっと最適化が必要ですね。

良い方法としては、Questionテーブルにinterested_usersという項目を持たせ、この項目を質問に対する関心データが作られる度に更新すればいいかもしれません。

Userオブジェクトモデルへの項目の追加

それではどーんとaskeet/config/schema.xmlデータモデルのask_questionテーブルに次の行を追加して変更してみましょう:

<column name="interested_users" type="integer" default="0" />

で、モデルの再設定:

$ symfony propel-build-model

そうです。現状の変更点に構わず、いきなり再設定します。Userクラスに書き込んだ変更はaskeet/lib/model/User.phpにあり、これはPropelで生成したaskeet/lib/model/om/BaseUser.phpを継承しています。だからaskeet/lib/model/om/ディレクトリに変更を加えてはいけないんですね: このディレクトリのクラスはbuild-modelが実行されるたびに上書きされてしまうからです。Symfonyでは、Webプロジェクトの初期に通常起こるこのような変更に柔軟に対応するよう作られています。

これに加えて、実際のデータベースにも変更が必要です。SQLをわざわざ書かないためにも、SQLスキーマを再設定し、テストデータをロードしましょう:

$ symfony propel-build-sql
$ mysql -u youruser -p askeet < data/sql/lib.model.schema.sql
$ php batch/load_data.php

$ mysql -u youruser -p askeet -e "alter table ask_question add interested_users int default '0'"

Interestオブジェクトのsave()メソッドの変更

この新しい項目の値は、ユーザーが質問に対して関心を表明するたびに更新されなければなりません。例えるならば、Interestテーブルにレコードが追加されるごとにです。MySQLのトリガーでも同じ事はできますが、これでもデータベース依存になってしまいますし、データベースを変更する場合には足かせになってしまいます。

一番良い方法としては、Interestオブジェクトが新たに作られるたびに呼び出されるsave()メソッドをオーバーライドし変更を加えることです。 それではaskeet/lib/model/Interest.phpファイルを開いて下のコードを加えてください:

public function save($con = null)
{  
    $ret = parent::save($con);
 
    // questionテーブルのinterested_usersを更新する
    $question = $this->getQuestion();
    $interested_users = $question->getInterestedUsers();
    $question->setInterestedUsers($interested_users + 1);
    $question->save($con);
 
    return $ret;
}

このsave()メソッドでは関連する質問を取得し、interested_users項目をカウントアップします。その後は通常通りにsave()を行います。しかし$this->save();としてしまうと無限ループになってしまうので、クラスメソッドであるparent::save()を代わりに使用しています(訳注：$ret = parent::save($con);の位置は言っている位置と逆）。

トランザクションで更新を安全に

QuestionとInterestオブジェクトの更新の間にデータベースで何か起こったらどうなるでしょう？ 整合性のないデータになってしまうかもしれません。これは、銀行での”最初にある口座からある額が引き落とされ、もう一つの口座の額を増やす”送金処理で起こりえる問題と同じです。

もし２つのリクエストが互いに依存しているのであれば、トランザクションを使って安全に実行すべきです。トランザクションは片方だけのリクエストが成功するということがないことを保証してくれます。トランザクション中のリクエストで何か起こった場合、それ以前に成功していたようなリクエストがあったとしてもこれをキャンセルし、データベースをトランザクションが開始される前の状態に戻すことができます。

save()メソッドは、symfonyにおいてのこのトランザクションを説明する良い材料なので、先程のコードを下のように変更してみましょう:

public function save($con = null)
{
  $con = Propel::getConnection();
  try
  {
    $con->begin();
 
    $ret = parent::save($con);
 
    // questionテーブルのinterested_usersを更新する
    $question = $this->getQuestion();
    $interested_users = $question->getInterestedUsers();
    $question->setInterestedUsers($interested_users + 1);
    $question->save($con);
 
    $con->commit();
 
    return $ret;
  }
  catch (Exception $e)
  {
    $con->rollback();
    throw $e;
  }
}

まず始めにCreole経由でのデータベース接続を直にオープンします。->begin()と->commit()宣言の間は、トランザクションが全ての処理の成功を保証します。何か失敗するようなことがあれば、例外が投げられ以前の状態に戻すためにロールバック処理がデータベースで実行されます。

テンプレートの変更

Questionオブジェクトの->getInterestedUsers()メソッドには問題はありませんが、_interested_user.phpフラグメントを変える準備はできているので、次の部分を変更しましょう:

<?php echo count($question->getInterests()) ?>

を下のように

<?php echo $question->getInterestedUsers() ?>

リクエスト、実行回数の数を考えてもこの方が良いですね。Webデバッグツールバーにあるデータベースアクセス回数を見ても一目瞭然です。また、データベースアイコンをクリックすると現在のページでの実行されたSQLの詳細が確認できます:

変更点の正当性テスト

showアクションを呼んで何も変わってないか確認してみましょう。でもちょっとその前に、昨日書いたバッチを走らせてデータを再設定してみましょう:

$ cd /home/sfprojects/askeet/batch
$ php load_data.php

Interestテーブルにレコードを作るとき、sfPropelDataオブジェクトはオーバーライドされたsave()メソッドを使い、関連を持つUserレコードをちゃんと更新するはずです。Interestオブジェクト用のCRUDインターフェースはまだありませんが、これは今回の変更をテストするために使えます。

トップページと質問を呼んでみて確認してみましょう:

http://askeet/frontend_dev.php/
http://askeet/frontend_dev.php/question/show/id/XX

関心のあるユーザーの数は変わってませんね？変更はうまくいきました。

回答にも同じ事を

count($question->getInterests())にしたことは、count($answer->getRelevancys())にも適用できます。違いといえば、質問には”関心”という一つの票があるだけですが、回答にはプラスとマイナスな票があるということです。すでにどうやってモデルに変更を加えるか分かっているので、サクッと説明します。一応下がその変更点です。askeet SVN リポジトリ を使ってコピーすれば、わざわざ手で書く必要もありません。

*schema.xmlのanswerテーブルに下の項目を追加する

    [xml]
    <column name="relevancy_up" type="integer" default="0" />
    <column name="relevancy_down" type="integer" default="0" />

• モデルを再設定して、データベースを更新する

  $ symfony propel-build-model
  $ symfony propel-build-sql
  $ mysql -u youruser -p askeet < data/sql/lib.model.schema.sql
  

*lib/model/Relevancy.phpにあるRelevancyクラスの->save()メソッドをオーバーライドする

    [php]
    public function save($con = null)
    {
      $con = Propel::getConnection();
      try
      {
        $con->begin();

        $ret = parent::save();

        // answerテーブルのrelevancyを更新する
        $answer = $this->getAnswer();
        if ($this->getScore() == 1)
        {
          $answer->setRelevancyUp($answer->getRelevancyUp() + 1);
        }
        else
        {
          $answer->setRelevancyDown($answer->getRelevancyDown() + 1);
        }
        $answer->save($con);

        $con->commit();

        return $ret;
      }
      catch (Exception $e)
      {
        $con->rollback();
        throw $e;
      }
    }

*Answerクラスに次の2つのメソッドを追加する:

    [php]
    public function getRelevancyUpPercent()
    {
      $total = $this->getRelevancyUp() + $this->getRelevancyDown();

      return $total ? sprintf('%.0f', $this->getRelevancyUp() * 100 / $total) : 0;
    }

    public function getRelevancyDownPercent()
    {
      $total = $this->getRelevancyUp() + $this->getRelevancyDown();

      return $total ? sprintf('%.0f', $this->getRelevancyDown() * 100 / $total) : 0;
    }

*question/templates/showSuccess.phpの回答に関する部分を以下のように変更する:

    [php]
    <div id="answers">
    <?php foreach ($question->getAnswers() as $answer): ?>
      <div class="answer">
        <?php echo $answer->getRelevancyUpPercent() ?>% UP <?php echo $answer->getRelevancyDownPercent() ?> % DOWN
        posted by <?php echo $answer->getUser()->getFirstName().' '.$answer->getUser()->getLastName() ?> 
        on <?php echo format_date($answer->getCreatedAt(), 'p') ?>
        <div>
          <?php echo $answer->getBody() ?>
        </div>
      </div>
    <?php endforeach; ?>
    </div>

• フィクスチャにテストデータを追加する

  Relevancy:
    rel1:
      answer_id: a1_q1
      user_id:   fabien
      score:     1
  
    rel2:
      answer_id: a1_q1
      user_id:   francois
      score:     -1
  

• バッチを走らせる

*question/showページを確認する

ルーティング

チュートリアルが始まってから、ずっと下のようなURLを使ってきました

http://askeet/frontend_dev.php/question/show/id/XX

symfonyではデフォルトのルーティングルールとしてこのリクエストは、下のように呼ばれたものと解釈されています

http://askeet/frontend_dev.php?module=question&action=show&id=XX

でもルーティングシステムによってもっと別な可能性が見えてきます。同じページを表示するにも質問のタイトルをURLの一部にも使えます。

http://askeet/frontend_dev.php/question/what-shall-i-do-tonight-with-my-girlfriend

これによってSEO対策ができたり、URLとしてはより意味のあるものとして表示できます。

タイトルの別バージョンを作る

始めに、URLとして使えるようにタイトルの変換バージョン(ストリップバージョン - カスタムクラスのセクションを確認)が必要です。There's more than one way to do it 方法は１つだけじゃない また、この変換されたタイトルはQuestionテーブルの項目に保存するようにします。schema.xmlのQuestionテーブルに下の行を追加してください:

<column name="stripped_title" type="varchar" size="255" />
<unique name="unique_stripped_title">
  <unique-column name="stripped_title" />
</unique>

そして、いつものようにモデルを再設定および、データベースを更新します:

$ symfony propel-build-model
$ symfony propel-build-sql
$ mysql -u youruser -p askeet < data/sql/lib.model.schema.sql

ストリップされたタイトルが同時に設定されるようにQuestionオブジェクトのsetTitle()メソッドをオーバーライドしていきます。

カスタムクラス

と、その前にタイトルをストリップバージョンに変換するカスタムクラスを作ります。カスタムクラスにする理由は、Questionオブジェクト自体には関係のない機能だからです(どのみちAnswerオブジェクトにも使うでしょうし)。

askeet/lib/ディレクトリにmyTools.class.phpファイルを作ってください:

<?php
 
class myTools
{
  public static function stripText($text)
  {
    $text = strtolower($text);
 
    // すべての非単語を剥ぎ取る
    $text = preg_replace('/\W/', ' ', $text);
 
    // すべての空白文字のセクションをダッシュに置き換える
    $text = preg_replace('/\ +/', '-', $text);
 
    // ダッシュをトリムする
    $text = preg_replace('/\-$/', '', $text);
    $text = preg_replace('/^\-/', '', $text);
 
    return $text;
  }
}

askeet/lib/model/Question.phpクラスを開いて下のコードを追加してください:

public function setTitle($v)
{
  parent::setTitle($v);
 
  $this->setStrippedTitle(myTools::stripText($v));
}

myToolsカスタムクラスの宣言が無いのに気づきましたか？: symfonyではlib/ディレクトリにあるものは、必要な時に自動的に読み込んでくれるからです。

それではテストデータのリロードをしてみましょう:

$ symfony cc
$ php batch/load_data.php

カスタムクラスやカスタムヘルパーについての詳しい情報は、symfony bookの拡張の章を読んでください。

showアクションへのリンクを変更する

listSuccess.phpテンプレートで下の行を

<h2><?php echo link_to($question->getTitle(), 'question/show?id='.$question->getId()) ?></h2>

次のように変更してください

<h2><?php echo link_to($question->getTitle(), 'question/show?stripped_title='.$question->getStrippedTitle()) ?></h2>

questionモジュールのactions.class.phpを開いて、showアクションを次のように変更してください:

public function executeShow()
{
  $c = new Criteria();
  $c->add(QuestionPeer::STRIPPED_TITLE, $this->getRequestParameter('stripped_title'));
  $this->question = QuestionPeer::doSelectOne($c);
 
  $this->forward404Unless($this->question);
}

トップページを更新し、質問のタイトルをクリックしてそれぞれにアクセスできるか確認してください:

http://askeet/frontend_dev.php/

URLは質問のタイトルで、ストリップバージョンが表示されているはずです:

http://askeet/frontend_dev.php/question/show/stripped-title/what-shall-i-do-tonight-with-my-girlfriend

ルーティングルールの変更

でもこれが表示したいURLの形ではありません。それではルーティングルールを編集してみましょう。routing.yml設定ファイルを開いて(askeet/apps/frontend/config/ディレクトリにあります)、ファイルの一番上に次のルールを追加してください:

question:
  url:   /question/:stripped_title
  param: { module: question, action: show }

urlの行でquestionという文字は最終的なURLに表示されるカスタムテキストです。stripped_titleは引数となります (:で始まっています)。link_to()ヘルパーをテンプレートに使っていることによって、symfonyルーティングシステムがこの同じ パターン をquestion/showアクションでのリンクに適用しています。

それでは最終テストです: トップページを表示し、質問のタイトルをクリックしてください。質問が表示されるだけでなく (何もおかしなところはなく)、ブラウザのアドレスバーには下のように表示されていると思います:

http://askeet/frontend_dev.php/question/what-shall-i-do-tonight-with-my-girlfriend

ルーティング機能についての詳しい情報は、symfony bookのルーティングポリシーの章 を読んでください。

それではまた明日

今日はWebサイト自体への新しい機能はあまりありませんでした。でもテンプレートのコーディングの仕方、モデルの変更の仕方を勉強し、いろんな場所をリファクタリングしていきました。

symfonyプロジェクトでは、このようなことは常に起こります: 再利用可能なコードはフラグメントに落とし込んだり、カスタムクラスにしたり、アクションやテンプレートに書かれたようなコードで実際はモデルにあるべきものはモデルに移動されます。コード自体は至る所に散らばってしまいますが、メンテナンスや変更はより簡単になります。付け加えれば、symfonyプロジェクトのファイル構造においては、コードフラグメントがどこにあるか簡単に見つけられるようになっています(ヘルパー、モデル、テンプレート、カスタムクラス、その他)。

今日やったリファクタリングで今後の開発がよりスピーディーになります。今のやり方(後に発生するサイトとしての機能にはあまり心配せず、今要る機能を盛り込んでいくやり方)においては、定期的にこのようなリファクタリングは繰り返していきます。

明日は？フォームに話を移して、フォームからの変数のやりとりを見ていきます。また、トップページの質問のリストを複数のページに分けていきます。それまでは今日のコードをSVN リポジトリ(release_day_4 タグ)からダウロードしてみてください:

http://svn.askeet.com/tags/release_day_4/

質問も大歓迎です askeet メーリングリストやフォーラムまで。