Today, we will start by refactoring some actions from the product module.

As symfony is modeled after the MVC design pattern, Vince was pretty confident that he did the separation of concerns pretty well.

The Controller role is to get data from the Model and pass them to the View.

Pretty simple, no?

A lot of people knows about the theory but it is hard to follow it by the letter in practice ; it is very easy to let some business logic slip into the controller. And Vince was no different.

Thin controller, fat model

The homepage of the website is composed of a list of all available products and the current code that manages this page is contained in the index action:

// apps/frontend/modules/product/actions.class.php
public function executeIndex()
{
  // get all available products
  $criteria = new Criteria();
  $criteria->add(ProductPeer::IS_IN_STOCK, true);
  $criteria->addAscendingOrderByColumn(ProductPeer::PRICE);
  $criteria->setLimit(10);
 
  $this->products = ProductPeer::doSelectJoinCategory($criteria);
 
  $this->getResponse()->setTitle('All products');
  $this->getResponse()->addStylesheet('homepage.css');
 
  return sfView::SUCCESS;
}
 

But The Criteria definition and the call to doSelectJoinCategory do not belong to the Controller, it belongs to the Model.

So, Vince decided to move this code to the ProductPeer class by creating a getAvailableProducts() method:

// lib/model/ProductPeer.php
static public function getAvailableProducts()
{
  $criteria = new Criteria();
  $criteria->add(self::IS_IN_STOCK, true);
  $criteria->addAscendingOrderByColumn(self::PRICE);
  $criteria->setLimit(10);
 
  return self::doSelectJoinCategory($criteria);
}
 

And the controller can now use this new method directly:

// apps/frontend/modules/product/actions.class.php
public function executeIndex()
{
  $this->products = ProductPeer::getAvailableProducts();
 
  $this->getResponse()->setTitle('All products');
  $this->getResponse()->addStylesheet('homepage.css');
 
  return sfView::SUCCESS;
}
 

This refactoring has several benefits over the previous code:

  • The logic to get the available products is now in the Model, where it belongs
  • The code in the controller is much more readable
  • The code is so readable that we do not need to document it anymore
  • The getAvailableProducts() method is re-usable (in another action, or a in task, ...)
  • The model code is now unit testable
  • Two developers can work together:
    • one is responsible to code the Model and to create a public API out of it
    • the other one uses the Model public API to code the Controller. And she does not care about how the products are retrieved (database, XML, Web Services...)

But what if you need the list of all available products in another action, or if you only need to get the five first ones? You will want to reuse the same method but as it is now, it is not very flexible as the limit (10) is hardcoded.

It is fine to keep it that way for now, but as soon as Vince will need to get the same type of results but with slightly different conditions, he will need refactor the new method to add some arguments:

// lib/model/ProductPeer.php
static public function getAvailableProducts($max = 10)
{
  $criteria = new Criteria();
  $criteria->add(self::IS_IN_STOCK, true);
  $criteria->addAscendingOrderByColumn(self::PRICE);
  $criteria->setLimit($max);
 
  return self::doSelectJoinCategory($criteria);
}
 

You get the idea. The goal is to be able to reuse the same code in different contexts. The first step is to refactor the code in the right layer and then add some way to customize the results by adding some arguments.

This one was easy. Let's refactor another part of the code.

Thin controller, fat model, revisited

Time to have a look at the favorite management code.

The code to add and remove products from the favorites is to be found in the product module:

// apps/frontend/modules/product/actions.class.php
public function executeAddToFavorites()
{
  $product = ProductPeer::retrieveByPk($this->getRequestParameter('id'));
  $this->forward404Unless($product);
 
  $favorites = $this->getUser()->getAttribute('favorites');
  $favorites[$product->getId()] = true;
 
  $this->getUser()->setAttribute('favorites', $favorites);
 
  $this->redirect('product/index');
}
 
public function executeRemoveFromFavorites()
{
  $product = ProductPeer::retrieveByPk($this->getRequestParameter('id'));
  $this->forward404Unless($product);
 
  $favorites = $this->getUser()->getAttribute('favorites');
  unset($favorites[$product->getId()]);
 
  $this->getUser()->setAttribute('favorites', $favorites);
 
  $this->redirect('product/index');
}
 

As you can see for yourself, the favorites are stored in the user session. But again, too much logic is embedded in this code. The controller does not need to know where and how the favorites are stored.

So, Vince decided to move this code to the myUser class by creating methods to encapsulate the logic:

// apps/frontend/lib/myUser.class.php
public function addProductToFavorites(Product $product)
{
  $favorites = $this->getAttribute('favorites');
  $favorites[$product->getId()] = true;
 
  $this->setAttribute('favorites', $favorites);
}
 
public function removeProductFromFavorites(Product $product)
{
  $favorites = $this->getAttribute('favorites');
  unset($favorites[$product->getId()]);
 
  $this->setAttribute('favorites', $favorites);
}
 

And the refactored actions in the product module class now uses this new user API:

// apps/frontend/modules/product/actions.class.php
public function executeAddToFavorites()
{
  $product = ProductPeer::retrieveByPk($this->getRequestParameter('id'));
  $this->forward404Unless($product);
 
  $this->getUser()->addProductToFavorites($product);
 
  $this->redirect('product/index');
}
 
public function executeRemoveFromFavorites()
{
  $product = ProductPeer::retrieveByPk($this->getRequestParameter('id'));
  $this->forward404Unless($product);
 
  $this->getUser()->removeProductFromFavorites($product);
 
  $this->redirect('product/index');
}
 

This refactoring has the same benefits as the one we did previously, but with an added bonus: if we decide to change the attribute name, or if we want to store the favorites in the database, the controller code won't change.

And the same goes for the template. Here is the snippet of code that deals with the favorites:

// apps/frontend/modules/product/templates/indexSuccess.php
<?php if (in_array($product->getId(), array_keys($sf_user->getAttribute('favorites', array())))): ?>
  <?php echo link_to(image_tag('/images/favorite.png'), 'product/removeFromFavorites?id='.$product->getId()) ?>
<?php else: ?>
  <?php echo link_to('add to my favorites', 'product/addToFavorites?id='.$product->getId()) ?>
<?php endif; ?>
 

The first line tests if the product is in the current user favorites to display the "add" link or the "remove" link.

Here again, we need to move this code to the user class by creating another method:

// apps/frontend/lib/myUser.class.php
public function hasInFavorites(Product $product)
{
  return in_array($product->getId(), array_keys($this->getAttribute('favorites', array())));
}
 

And here is the cleaned up template snippet:

// apps/frontend/modules/product/templates/indexSuccess.php
<?php if ($sf_user->hasInFavorites($product)): ?>
  <?php echo link_to(image_tag('/images/favorite.png'), 'product/removeFromFavorites?id='.$product->getId()) ?>
<?php else: ?>
  <?php echo link_to('add to my favorites', 'product/addToFavorites?id='.$product->getId()) ?>
<?php endif; ?>
 

And we did it again. The code is more readable thanks to the refactoring and the method name we chose. It is very important to think about method names. They convey a lot of information and most of the time, they can replace the documentation. You don't need to explain what $sf_user->hasInFavorites($product) does.

After this refactoring, all the code that deals with the favorites is now in the same class. And that's yet another benefit. Instead of having code that manipulates the favorites in a lot of different places, everything is now centralized in one class. You can even refactor it further by creating a dedicated ProductFavorite class if the need arises.

That's all for today. Next time, Vince and I will refactor the action that allows to modify the product and the associated image. In the meantime, you can apply the rules we have learned today to your own symfony projects.

Published in #Call the expert