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

In the last part, we have moved some code to ProductForm. Today, we will enhance the actions even more by moving code to the View and by using some nice shortcuts provided by symfony for common situations.

In part 3, we have refactored the index action which now looks like this:

In this action, we set the page title and we add a specific stylesheet. Even if this code works as expected, it is not the right way to do this. This code really belongs to the view. So let's move this code to the indexSuccess.php template, so that our action looks like this:

As of symfony 1.1, symfony passes the request object as the first argument to the execute* methods. So, instead of using the getRequestParameter() proxy method, we can just use the getParameter() method from the request object. We can also use the new isMethod() method to simplify the POST check.

Here is the final executeEdit() method after Vince made all the changes:

The executeEdit() code is now so common, that you can use a nice shortcut for the form processing. The bindAndSave() method is a shortcut that calls the bind() method and then save() the object if the form is valid:

The new productActions class has now 31 lines of code instead of 96. We have cut the code by more than two-thirds!

Here is a LOC table for our project:

Layer

Before

After

Model

Product

0

9

User

0

19

View

Index

35

37

Controller

Actions

96

31

Forms

7

24

Total

138

120

So, after the refactoring, we have a bit less of code and especially a better separation of concerns. We also have created re-usable objects and methods:

ProductForm

ProductPeer::getAvailableProducts()

myUser::addProductToFavorites()

myUser::removeProductFromFavorites()

myUser::hasInFavorites()

And reusable objects also means that they are self-contained. So, you can even package your model classes and forms as a plugin now.

As an added bonus, writing unit tests for our code is now also much easier.

The main goals of every refactoring process are always the same:

make the code more maintainable

allow the code to evolve easily

reduce the amount of code needed to write new features by reusing existing code

Even if Vince was quite tired after this refactoring session, he was pretty happy and started to refactor other parts of his website right away. I hope you have also learned new tricks that you will be able to use for your next symfony project.

Great tutorial, learning is definitively easier when you work an a concrete case. I think that everybody would enjoy to have other articles like this serie :)

I still have a question : why didn't you use a view.yml at the module level to set the title and the css file ?
For readability, and erros saving, isn't configuration file editing better than code writing ?

Nice work Fabien.
You could have conclude that even Vince was tired, he ran his functional test (only one command line). All the tests passed so he got confident to haven't broken the application.
Which is a quite important goal in our job.

Thanks Fabien. Great learning.
I wanted to do some refactoring in my project (still symfony 1.0). I have "products" that spread across several tables (optional configurable addons to products). I need to get the logic out of the controller and I'm thinking where to put the logic into. Model? Which model table, as I'm using several. In the model of the main product table? Or in a separate library class?

I'm not yet fluent with best practice of this separation. Thanks for a hint.