symfony advent calendar day four: refactoring
=============================================

Previously on symfony
---------------------

During [day three](3.txt), all the layers of a MVC architecture were shown and modified to have the list of questions properly displayed on the homepage. The application is getting nicer but still lacks content.

The objectives for the fourth day are to show the list of answers to a question, to give a nice URL to the question detail page, to add a custom class, and to move some chunk of codes to a better place. This should help you understand the concepts of template, model, routing policy, and refactoring. You may think that it is too early to rewrite code that is only a few days old, but we'll see how you feel about it at the end of this tutorial.

To read this tutorial, you should be familiar with the concepts of the [MVC implementation in symfony](http://www.symfony-project.com/book/1_0/02-Exploring-Symfony-s-Code). It would also help if you had an idea about what [agile development][1] is.

Show the answers to a question
------------------------------

First, let's continue the adaptation of the templates generated by the `Question` CRUD during [day two](2.txt)

The `question/show` action is dedicated to display the details of a question, provided that you pass it an `id`. To test it, just call (you will have to change `2` to the real `id` the question has in your table):

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

[question detail](/images/askeet/question_day4.gif)

You probably already saw the `show` page if you played with the application before. This is where we are going to add the answers to a question. 

### A quick look at the action

First, let's have a look at the `show` action, located in the `askeet/apps/frontend/modules/question/actions/actions.class.php` file:

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

If you are familiar with Propel, you recognize here a simple request to the `Question` table. It is aimed to get the unique record having the value of the `id` parameter of the request as a primary key. In the example given in the URL above, the `id` parameter has a value of `1`, so the `->retrieveByPk()` method of the `QuestionPeer` class will return the object of class `Question` with `1` as a primary key. If you are not familiar with Propel, come back after you've read [some documentation][2] on their website.

The result of this request is passed to the `showSuccess.php` template through the `$question` variable. 

The `->getRequestParameter('id')` method of the `sfAction` object gets... the request parameter called `id`, whether it is passed in a GET or in a POST mode. For instance, if you require:

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

...then the `show` action will be able to retrieve `myvalue` by requesting `$this->getRequestParameter('myparam')`. 

>**Note**: The `forward404Unless()` method sends to the browser a 404 page if the question does not exist in the database. It's always a good pratice to deal with edge cases and errors that can occur during execution and symfony gives you some simple methods to help you do the right thing easily.

### Modify the `showSuccess.php` template

The generated `showSuccess.php` template is not exactly what we need, so we will completely rewrite it. Open the `frontend/modules/question/templates/showSuccess.php` file and replace its content by:

    [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>

You recognize here the `interested_block` div that was already added to the `listSuccess.php` template yesterday. It just displays the number of interested users for a given question. After that, the markup also looks very much like the one of the `list`, except that there is no `link_to` on the title. It is just a rewriting of the initial code to display only the necessary information about a question. 

The new part is the `answers` div. It displays all the answers to the question (using the simple `$question->getAnswers()` Propel method), and for each of them, shows the total relevancy, the name of the author, and the creation date in addition to the body.

The `format_date()` is another example of template helpers for which an initial declaration is required. You can find more about this helper's syntax and other helpers in the [internationalization helpers chapter](http://www.symfony-project.com/book/1_0/13-I18n-and-L10n) of the symfony book (these helpers speed up the tedious task of displaying dates in a good looking format).

>**Note**: Propel creates method names for linked tables by adding an 's' automatically at the end of the table name. Please forgive the ugly `->getRelevancys()` method since it saves you several lines of SQL code. 

### Add some new test data

It is time to add some data for the `answer` and `relevancy` tables at the end of the `data/fixtures/test_data.yml` (feel free to add your own):

    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.

Reload your data with:

    $ php batch/load_data.php

Navigate to the action showing the first question to check if the modifications were successful:

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

>**Note**: Replace XX with the current `id` of your first question.

![question answers](/images/askeet/answers_day4.gif)

The question is now displayed in a fancier way, followed by the answers to it. That's better, isn't it?

Modify the model, part I
------------------------

It is almost certain that the full name of an author will be needed somewhere else in the application. You can also consider that the full name is an attribute of the `User` object. This means that there should be a method in the `User` model allowing to retrieve the full name, instead of reconstructing it in an action. Let's write it. Open the `askeet/lib/model/User.php` and add in the following method:

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

Why is this method named `__toString()` instead of `getFullName()` or something similar? Because the `__toString()` method is the default method used by PHP5 for object representation as string. This means that you can replace the  

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

line of the `askeet/apps/frontend/modules/question/templates/showSuccess.php` template by a simpler

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

to achieve the same result. Neat, isn't it ?

Don't repeat yourself
---------------------

One of the good principles of agile development is to avoid duplicating code. It says "Don't Repeat Yourself" (D.R.Y.). This is because duplicated code is twice as long to review, modify, test and validate than a unique encapsulated chunk of code. It also makes application maintenance much more complex. And if you paid attention to the last part of today's tutorial, you probably noticed some duplicated code between the `listSuccess.php` template written yesterday and the `showSuccess.php` template:

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

So our first session of [refactoring](http://en.wikipedia.org/wiki/Refactoring) will remove this chunk of code from the two templates and put it in a **fragment**, or reusable chunk of code. Create an `_interested_user.php` file in the `askeet/apps/frontend/modules/question/templates/` directory with the following code:

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

Then, replace the original code in both templates (`listSuccess.php` and `showSuccess.php`) with:

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

A fragment doesn't have native access to any of the current objects. The fragment uses a `$question` variable, so it has to be defined in the `include_partial` call. The additional `_` in front of the fragment file name helps to easily distinguish fragments from actual templates in the `templates/` directories. If you want to learn more about fragments, read the [view chapter](http://www.symfony-project.com/book/1_0/07-Inside-the-View-Layer) of the symfony book. 

Modify the model, part II
-------------------------

The `$question->getInterests()` call of the new fragment does a request to the database and returns an array of objects of class `Interest`. This is a heavy request for just a number of interested persons, and it might load the database too much. Remember that this call is also done in the `listSuccess.php` template, but this time in a loop, for each question of the list. It would be a good idea to optimize it.

One good solution is to add a column to the `Question` table called `interested_users`, and to update this column each time an interest about the question is created.

>**Caution**: We are about to modify the model without any apparent way to test it, since there is currently no way to add `Interest` records through askeet. You should never modify something without any way to test it. 
>
>Luckily, we do have a way to test this modification, and you will discover it later in this part.

### Add a field to the `User` object model

Go without fear and modify the `askeet/config/schema.xml` data model by adding to the `ask_question` table:

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

Then rebuild the model:

    $ symfony propel-build-model

That's right, we are already rebuilding the model without worrying about existing extensions to it! This is because the extension to the `User` class was made in the `askeet/lib/model/User.php`, which inherits from the Propel generated `askeet/lib/model/om/BaseUser.php` class. That's why you should never edit the code of the `askeet/lib/model/om/` directory: it is overridden each time a `build-model` is called. Symfony helps to ease the normal life cycle of model changes in the early stages of any web project.

You also need to update the actual database. To avoid writing some SQL statement, you should rebuild your SQL schema and reload your test data:

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

>**Note**: TIMTOWTDI: There is more than one way to do it. Instead of rebuilding the database, you can add the new column to the MySQL table by hand:
>
>     $ mysql -u youruser -p askeet -e "alter table ask_question add interested_users int default '0'"
>

### Modify the `save()` method of the `Interest` object

Updating the value of this new field has to be done each time a user declares its interest for a question, i.e. each time a record is added to the `Interest` table. You could implement that with a trigger in MySQL, but that would be a database dependent solution, and you wouldn't be able to switch to another database as easily. 

The best solution is to modify the model by overriding the `save()` method of the `Interest` class. This method is called each time an object of class `Interest` is created. So open the `askeet/lib/model/Interest.php` file and write in the following method:

    [php]
    public function save($con = null)
    {  
        $ret = parent::save($con);
        
        // update interested_users in question table
        $question = $this->getQuestion();
        $interested_users = $question->getInterestedUsers();
        $question->setInterestedUsers($interested_users + 1);
        $question->save($con);

        return $ret;
    }

The new `save()` method gets the question related to the current interest, and increments its `interested_users` field. Then, it does the usual `save()`, but because a `$this->save();` would end up in an infinite loop, it uses the class method `parent::save()` instead.

### Secure the updating request with a transaction

What would happen if the database failed between the update of the `Question` object and the one of the `Interest` object? You would end up with corrupted data. This is the same problem met in a bank when a money transfer means a first request to decrease the amount of an account, and a second request to increase another account. 

If two request are highly dependent, you should secure their execution with a **transaction**. A transaction is the insurance that both requests will succeed, or none of them. If something wrong happens to one of the requests of a transaction, all the previously succeeded requests are cancelled, and the database returns to the state where it was before the transaction. 

Our `save()` method is a good opportunity to illustrate the implementation of transactions in symfony. Replace the code by:

    [php]
    public function save($con = null)
    {
      $con = Propel::getConnection();
      try
      {
        $con->begin();
  
        $ret = parent::save($con);
  
        // update interested_users in question table
        $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;
      }
    }

First, the method opens a direct connection to the database through Creole. Between the `->begin()` and the `->commit()` declarations, the transaction ensures that all will be done or nothing. If something fails, an exception will be raised, and the database will execute a rollback to the previous state.

### Change the template

Now that the `->getInterestedUsers()` method of the `Question` object works properly, it is time to simplify the `_interested_user.php` fragment by replacing:

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

by

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

>**Note**: Thanks to our briliant idea to use a fragment instead of leaving duplicated code in the templates, this modification only needed to be made once. If not, we would have to modify the `listSuccess.php` AND `showSuccess.php` templates, and for lazy folks like us, that would have been overwhelming.

In terms of number of requests and execution time, this should be better. You can verify it with the number of database requests indicated in the web debug toolbar, after the database icon. Notice that you can also get the detail of the SQL queries for the current page by clicking on the database icon itself:

![database queries before refactoring](/images/askeet/debug_before_day4.gif)
![database queries after refactoring](/images/askeet/debug_after_day4.gif)

### Test the validity of the modification

We'll check that nothing is broken by requesting the `show` action again, but before that, run again the data import batch that we wrote yesterday:

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

When creating the records of the `Interest` table, the `sfPropelData` object will use the overridden `save()` method and should properly update the related `User` records. So this is a good way to test the modification of the model, even if there is no CRUD interface with the `Interest` object built yet.

Check it by requesting the home page and the detail of the first question:

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

The number of interested users didn't change. That's a successful move!

Same for the answers
--------------------

What was just done for the `count($question->getInterests())` could as well be done for the `count($answer->getRelevancys())`. The only difference will be that an answer can have positive and negative votes by users, while a question can only be voted as 'interesting'. Now that you understand how to modify the model, we can go fast. Here are the changes, just as a reminder. You don't have to copy them by hand for tomorrow's tutorial if you use the [askeet SVN repository](http://svn.askeet.com/tags/release_day_4/).

* Add the following columns to the `answer` table in the `schema.xml`

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

* Rebuild the model and update the database accordingly

        $ symfony propel-build-model
        $ symfony propel-build-sql
        $ mysql -u youruser -p askeet < data/sql/lib.model.schema.sql
        
* Override the `->save()` method of the `Relevancy` class in the `lib/model/Relevancy.php`

        [php]
        public function save($con = null)
        {
          $con = Propel::getConnection();
          try
          {
            $con->begin();
      
            $ret = parent::save();
      
            // update relevancy in answer table
            $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;
          }
        }

* Add the two following methods to the `Answer` class in the model:

        [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;
        }

* Change the part concerning the answers in `question/templates/showSuccess.php` by:

        [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>

* Add some test data in the fixtures

        Relevancy:
          rel1:
            answer_id: a1_q1
            user_id:   fabien
            score:     1
    
          rel2:
            answer_id: a1_q1
            user_id:   francois
            score:     -1
    
* Launch the population batch

* Check the `question/show` page

![relevancies on answers](/images/askeet/home_day4.gif)

Routing
-------

Since the beginning of this tutorial, we called the URL

    http://askeet/frontend_dev.php/question/show/id/XX
    
The default routing rules of symfony understand this request as if you had actually requested

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

But having a routing system opens up a lot of other possibilities. We could use the title of the questions as the URL, to be able to require the same page with:

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

This would optimize the way the search engines index the pages of the website, and to make the URLs more readable.

### Create an alternate version of the title

First, we need a converted version of the title - a stripped title - to be used as an URL. [There's more than one way to do it](http://en.wikipedia.org/wiki/Perl), and we will choose to store this alternate title as a new column of the `Question` table. In the `schema.xml`, add the following line to the `Question` table:

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

...and rebuild the model and update the database:

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

We will soon override the `setTitle()` method of the `Question` object so that it sets the stripped title at the same time. 

### Custom class

But before that, we will create a custom class to actually transform a title into a stripped title, since this function doesn't really concern specifically the `Question` object (we will probably also use it for the `Answer` object).

Create a new `myTools.class.php` file under the `askeet/lib/` directory:

    [php]
    <?php
    
    class myTools
    {
      public static function stripText($text)
      {
        $text = strtolower($text);
    
        // strip all non word chars
        $text = preg_replace('/\W/', ' ', $text);
    
        // replace all white space sections with a dash
        $text = preg_replace('/\ +/', '-', $text);
    
        // trim dashes
        $text = preg_replace('/\-$/', '', $text);
        $text = preg_replace('/^\-/', '', $text);
    
        return $text;
      }
    }

Now open the `askeet/lib/model/Question.php` class file and add:

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

Notice that the `myTools` custom class doesn't need to be declared: symfony autoloads it when needed, provided that it is located in the `lib/` directory.

We can now reload our data:

    $ symfony cc
    $ php batch/load_data.php

If you want to learn more about custom class and custom helpers, read the [extension chapter](http://www.symfony-project.com/book/1_0/07-Inside-the-View-Layer) of the symfony book.

### Change the links to the `show` action

In the `listSuccess.php` template, change the line

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

by

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

Now open the `actions.class.php` of the `question` module, and change the `show` action to:

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

Try to display again the list of questions and to access each of them by clicking on their title:

    http://askeet/frontend_dev.php/
    
The URLs correctly display the stripped title of the questions:

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

### Changing the routing rules

But this is not exactly how we wanted them to be displayed. It is now time to edit the routing rules. Open the `routing.yml` configuration file (located in the `askeet/apps/frontend/config/` directory) and add the following rule on top of the file:

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

In the `url` line, the word `question` is a custom text that will appear in the final URL, while the `stripped_title` is a parameter (it is preceded by `:`). They form a **pattern** that the symfony routing system will apply to the links to the `question/show` action calls - because all the links in our templates use the `link_to()` helper.

It is time for the final test: Display again the homepage, click on the first question title. Not only does the first question show (proving that nothing is broken), but the address bar of your browser now displays:

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

If you want to learn more about the routing feature, read the [routing policy chapter](http://www.symfony-project.com/book/1_0/09-Links-and-the-Routing-System) of the symfony book.

See you Tomorrow
----------------

Today, the website itself didn't get many new features. However, you saw more template coding, you know how to modify the model, and the overall code has been refactored in a lot of places. 

This happens all the time in the life of a symfony project: the code that can be reused is refactored to a fragment or a custom class, the code that appears in an action or a template and that actually belongs to the model is moved to the model. Even if this spreads the code in lots of small files disseminated in lots of directories, the maintenance and evolution is made easier. In addition, the file structure of a symfony project makes it easy to find where a piece of code actually lies according to its nature (helper, model, template, action, custom class, etc.). 

The refactoring job done today will speed up the development of the upcoming days. And we will periodically do some more refactoring in the life of this project, since the way we develop - make a feature work without worrying about the upcoming functionalities - requires a good structure of code if we don't want to end up with a total mess.

What's for tomorrow? We will start writing a form and see how to get information from it. We will also split the list of questions of the home page into pages. In the meantime, feel free to download today's code from the SVN repository (tagged release_day_4) at: 

    http://svn.askeet.com/tags/release_day_4/
    
and to send us any questions using the [askeet mailing-list](mailto:askeet-subscribe@symfony-project.com) or the [dedicated forum](http://www.symfony-project.com/forum/index.php/f/8/).

[1]: http://en.wikipedia.org/wiki/Agile_software_development  "Agile Software development definition at Wikipedia"
[2]: http://propel.phpdb.org/docs/user_guide/                 "Propel documentation"