Call the expert: A refactoring story (Part 4/5)

In the last part, we have seen how to move code from the Controller to the Model. The principle is quite simple, you want thin controllers and fat models.

Today, we will see how the new form framework can help us keep this clean separation of concerns.

When the user is logged in as an administrator, he can update the information of each product and upload a new picture.

This page is managed by the edit action of the product module:

public function executeEdit()
{
  $this->product = ProductPeer::retrieveByPk($this->getRequestParameter('id'));
  if (is_null($this->product))
  {
    $this->forward404();
  }
 
  $this->form = new ProductForm($this->product);
 
  if (sfRequest::POST == $this->getRequest()->getMethod())
  {
    $this->form->bind($this->getRequestParameter('product'), $this->getRequest()->getFiles('product'));
    if ($this->form->isValid())
    {
      // get the current product
      $product = $this->form->getObject();
      $image = $this->form->getValue('image');
      $filename = $product->getImage();
 
      if ($image)
      {
        // remove old image
        if (file_exists($file = sfConfig::get('sf_web_dir').'/images/products/'.$currentImage))
        {
          unlink($file);
        }
      }
 
      // update the product
      $product = $this->form->updateObject();
 
      if ($image)
      {
        // save new image
        $filename = sha1($image->getOriginalName()).$image->getExtension($image->getOriginalExtension());
        $image->save(sfConfig::get('sf_web_dir').'/images/products/'.$filename);
      }
 
      $product->setImage($filename);
 
      // save the product
      $product->save();
 
      $this->getUser()->setFlash('notice', sprintf('The product "%s" has been updated successfully.', $product->getTitle()));
      $this->redirect('product/index');
    }
  }
}
 

The code is self-explanatory:

• The product is retrieved from the Model (Product)
• A form is created for the given product (ProductForm)
• And if the form is posted (POST request method)
  • The form is validated
  • The product is updated

Most of the code manages the image upload process:

• If an image is uploaded, remove the old one
• Save the new image with a unique name
• Change the image path for the Product object

But the code that manages the image upload process does not really belongs to this Controller, does it?

It is pretty simple to know if the code is in the right place. Ask yourself: How can I reuse the ProductForm form elsewhere in my application? In this specific case, you will have to duplicate a lot of code. And you know there is a better way to do this. So, it is time to refactor.

If you have read the "symfony forms in action" book, you know that the form framework allows a clean separation between the Controller (the form itself), the Model (the validators), and the View (the widgets).

So, let's move some code from the action Controller to the form Controller. I want the action Controller to look like this after the refactoring:

public function executeEdit()
{
  $this->product = ProductPeer::retrieveByPk($this->getRequestParameter('id'));
  if (is_null($this->product))
  {
    $this->forward404();
  }
 
  $this->form = new ProductForm($this->product);
 
  if (sfRequest::POST == $this->getRequest()->getMethod())
  {
    $this->form->bind($this->getRequestParameter('product'), $this->getRequest()->getFiles('product'));
    if ($this->form->isValid())
    {
      $product = $this->form->save();
 
      $this->getUser()->setFlash('notice', sprintf('The product "%s" has been updated successfully.', $product->getTitle()));
      $this->redirect('product/index');
    }
  }
}
 

The code we have removed must go somewhere. The first idea is to move it to the form save() method. But there is a better place. The method we want to override is updateObject(). The updateObject() method is where symfony update the object with the cleaned up values. This method is called by save() within a database transaction.

So, here is the new ProductForm code:

class ProductForm extends BaseProductForm
{
  public function configure()
  {
    unset($this['created_at']);
 
    $this->widgetSchema['image'] = new sfWidgetFormInputFile();
    $this->validatorSchema['image'] = new sfValidatorFile(array('required' => false));
  }
 
  public function updateObject()
  {
    $image = $this->getValue('image');
    $filename = $this->object->getImage();
 
    if ($image)
    {
      // remove old image
      if (file_exists($file = sfConfig::get('sf_web_dir').'/images/products/'.$filename))
      {
        unlink($file);
      }
    }
 
    // update the product
    parent::updateObject();
 
    if ($image)
    {
      // save new image
      $filename = sha1($image->getOriginalName()).$image->getExtension($image->getOriginalExtension());
      $image->save(sfConfig::get('sf_web_dir').'/images/products/'.$filename);
    }
 
    $this->object->setImage($filename);
  }
}
 

As an added bonus, we only have to save the Product object once instead of twice as previously. The code is better encapsulated as we have a fairly standard executeEdit() action code that is self-explanatory and the ProductForm can be reused elsewhere with very little effort.

But now that the logic is in the form, we can do even better. As we have now direct access to the values variable, we can refactor the code like this:

public function updateObject()
{
  if ($image = $this->getValue('image'))
  {
    // remove old image
    if (file_exists($file = sfConfig::get('sf_web_dir').'/images/products/'.$this->object->getImage()))
    {
      unlink($file);
    }
 
    // save new image
    $filename = sha1($image->getOriginalName()).$image->getExtension($image->getOriginalExtension());
    $image->save(sfConfig::get('sf_web_dir').'/images/products/'.$filename);
  }
 
  unset($this->values['image']);
 
  // update the product
  return parent::updateObject();
}
 

The trick is here is to remove the image value from the cleaned up values after we have processed it. That way, the default updateObject() method of symfony won't update the image column. You know that Vince want his code to be perfect, so I even make it better by creating a dedicated method that update the image:

public function updateObject()
{
  $this->updateImageColumn();
 
  return parent::updateObject();
}
 
protected function updateImageColumn()
{
  if ($image = $this->getValue('image'))
  {
    // remove old image
    if (file_exists($file = sfConfig::get('sf_web_dir').'/images/products/'.$this->object->getImage()))
    {
      unlink($file);
    }
 
    // save new image
    $filename = sha1($image->getOriginalName()).$image->getExtension($image->getOriginalExtension());
    $image->save(sfConfig::get('sf_web_dir').'/images/products/'.$filename);
  }
 
  unset($this->values['image']);
}
 

As of symfony 1.2, you can even remove the form code (updateObject and updateImageColumn) altogether as Propel forms now does all this boiler-plate code automatically for you.

To sum up the work we have done during the first three parts: we have made our Controller thiner by moving code to the Model (ProductPeer), to the user (myUser), and to a form (ProductForm).

In the next part, we will see that some code must still be moved away from the Controller. We will also learn some nice shortcuts made available as of symfony 1.1.