Juozas devBlog

It’s always good to use great tools, but you need to make sure that you use them correctly, not just trying to code “just for it to work”. For this reason I decided to write down my usual list of things I mention when taking over some legacy project or just consulting someone how to start.

Most of the outlined problems and solutions are focused on testability, maintainability and other good code practices. If you are not familiar with them, I recommend read about them ASAP as there is big chance that you are doing those things described in this post and don’t even realize how wrong they are. Believe me, you will soon find yourself a way better developer.

Separate logic

This one is the most obvious one, but trust me, I have found cases of it in every single project I have worked before (more than 10 with Zend Framework in a past half year and counting). If it’s controller, don’t do business logic, if it’s model don’t base behavior on POST parameters etc. Same applies to forms, bootstrap, views, various helpers and many more other components – logic should be separate and have its place.

Move all logic from controller to a model or service. Use forms only to handle validation and filtering, not to actually process data and persist it in any fashion. Hide session and authentication handling in one place and provide API for other algorithms. I can go on, but I hope it’s starting to get pretty clear, or at least it will eventually when you start testing your code: when you need to setup frontController, request, cookie and mail server just to test a form you realize that something is really wrong.

Globals

If you haven’t seen this video, please do it immediately – you won’t regret it. Global state makes testing problematic and is completely opposite to what OOP proposes. This applies for use of $_SERVER, $_SESSION etc. values and all of them are accessible via request object (look at Zend_Controller_Request_Http) methods or separate classes, like Zend_Session.

Yet again I’m going to mention testability, because that’s something you always need to keep in mind. Test should not modify global variables, but rather inject mocked request/other objects which just return expected values (like client IP), because Zend Framework does pretty good job abstracting access to all global variables.

Use form values, not request

This one is both security and ease of coding issue, let’s look at this code sample:

$form = new Form();
 
if ($this->_request->isPost()) {
 
    if ($form->isValid($this->_request->getPost())
    {
        $model = new Model($this->_request->getPost());
        $model->save();
    }
    else
    {
        $form->populate($this->_request->getPost());
    }
}

After form is validated (and hence values are filtered), raw values from request are still used from $this->_request->getPost(). Not only do you lose Zend_Form functions like ignored elements (for submit buttons for example) but also none of the filters are applied (which you use, yes?). Also I can pass pretty much anything I want and model needs to do way more validation. Hence $form->getValues() should always be used as it returns form data with respect to all defined rules and filters.

Form populate() method is also very overused. This method is designed to set default values for a form, from for example GB entry (when editing something) apart from that, you simply don’t need to use it as method isValid() sets values for all elements, so there is no need to apply default values too.

Do not rely or use exit()/die()

One of the first things I do is remove all these exit() calls and handle such cases with exceptions or return statements. There is only very very limited amount of situations where you actually need to use one of those functions. Just for example, imagine this controller action code:

if (!$this->userHasPermissions())
{
    $this->_redirect('/');
}
 
$form = new Form_Add();
 
if (//submit form)
{
    // save with $form->getValues();
    $this->_redirect('/index');
    exit();
}
 
// do something else

First problem – thinking that $this->_redirect() will call exit() and hence nothing else will be executed. Even though this is true by default, this should be avoided in all cases. Not only it makes all post* events to not be fired, but also it makes testing impossible or incorrect. Zend_Test disables use of exit() in controller helpers, so in this case while testing you cannot test permissions checking as in all cases it will still execute later code. To fix this just add return in front of redirect (return $this->_redirect(‘/’)) and you are safe.

Furthermore, second exit() is completely useless and makes code even more untestable. Again you can just use return to cancel later code (view will not be rendered as viewRenderer helper checks for redirection header and does nothing if detects such). From my experience, after saving entry, there is only some assignments to view in left code so you won’t suffer a lot if you just leave redirect, of course without exit().

Use a framework, not PHP

This can sound wrong at first, but if you use a framework (Zend Framework in this case) don’t start throwing in hacks from 5 year old PHP apps. Like this (controller action):

$object = new Some_Object();
 
$image = $object->generateImage();
 
header ('Content-type: image/jpeg');
echo $image;

I don’t even know where to start… All this logic is incorporated in response object, so you can do things like:

$this->getResponse()->setHeader("Content-type", 'image/jpeg');
$this->getResponse()->setBody($image);

It might seem as same thing, but it’s not. Yet again you can actually test it, you are not working with global state (header() is global state function) and request dispatch process is left working as it should. Controllers do not output any data (hence no echo should be used) they just get request object and return response object, that’s it. In a similar fashion as exit() breaks dispatch process, outputting from controller does that too, so don’t forget to make sure that you preserve this flow.

Some small ones

Application.ini has a property includePaths, which is used to add additional paths to include path. Even though it works great, I still recommend not to use it because it will add those paths every time you create new application instance (true for 1.9, can change in future). If you try to do some controllers testing, you are probably going to instantiate it before every single test and after some hundreds of tests you will notice that somehow it’s getting slower and slower. That took me a few hours to find, though fix was easy, but still keep that in mind.

If you are using jQuery or any other javascript view helpers, take full power of them. By that I mean use functions like addJavascriptFile(), addJavascript(), addStylesheet(), addOnload() etc. to add some additional resources and code from views. If you add javascript straight to view everything will still work, but by using view helper container you will have all your code nicely placed in one place and not scattered all other the place.

Conclusion

This is just a small list of problems and issue I have seen in my work – there are way more to look at (you can share some tips too). I hope those will give you some idea how to work with Zend Framework in a clean fashion and you will soon find that your code is starting to look nicer and coding time is decreasing as everything is nicely separated and transparent to other code.