Advice on Zend Framework 2 and Doctrine

Question

I've just started looking at Zend Framework 2 and Doctrine ORM, with a view to possibly using it in one of my future products. So far I have a very simple application which allows the user to select data from a 'configuration' table. This table has just one row and several columns with settings that the application will use at some point.

So far this is what I have come up with:

ConfigController.php (my Zend controller)

<?php
/**
 * A restful controller that retrieves and updates configuration information
 */
namespace Application\Controller;

use Zend\Mvc\Controller\AbstractRestfulController;
use Zend\View\Model\JsonModel;

class ConfigController extends AbstractRestfulController
{
    /**
     * Retrieves the configuration from the database 
     */
    public function getList(){
      //locate the doctrine entity manager
      $em = $this->getServiceLocator()
                 ->get('Doctrine\ORM\EntityManager');

      //there should only ever be one row in the configuration table, so I use findAll
      $config = $em->getRepository("\Application\Entity\Config")->findAll();

      //return a JsonModel to the user. I use my toArray function to convert the doctrine
      //entity into an array - the JsonModel can't handle a doctrine entity itself.
      return new JsonModel(array(
        'data' => $config[0]->toArray(),
      ));
    }

    /**
     * Updates the configuration
     */
    public function replaceList($data){
      //locate the doctrine entity manager
      $em = $this->getServiceLocator()
                 ->get('Doctrine\ORM\EntityManager');

      //there should only ever be one row in the configuration table, so I use findAll
      $config = $em->getRepository("\Application\Entity\Config")->findAll();

      //loop through each submitted field
      foreach($data as $column=>$value){
        //work out the name of the setter function for each field
        $func = "set".ucfirst($column);
        $config[0]->$func($value);
      }

      //save the entity to the database
      $em->persist($config[0]);
      $em->flush();

      //return a JsonModel to the user. I use my toArray function to convert the doctrine
      //entity into an array - the JsonModel can't handle a doctrine entity itself.
      return new JsonModel(array(
        'data' => $config[0]->toArray(),
      )); 
    }
}

Config.php (my Doctrine entity)

<?php

namespace Application\Entity;
use Doctrine\ORM\Mapping as ORM;
/** 
 * @ORM\Entity 
 * @ORM\HasLifecycleCallbacks
 */
class Config {
    /** 
     * @ORM\Id
     * @ORM\Column(type="integer") 
     */
    protected $minLengthUserId;

    /** 
     * @ORM\Id
     * @ORM\Column(type="integer") 
     */
    protected $minLengthUserName;

    /** 
     * @ORM\Id
     * @ORM\Column(type="integer") 
     */
    protected $minLengthUserPassword;

    /** 
     * @ORM\Id
     * @ORM\Column(type="integer") 
     */
    protected $daysPasswordReuse;

    /** 
     * @ORM\Id
     * @ORM\Column(type="boolean") 
     */
    protected $passwordLettersAndNumbers;

    /** 
     * @ORM\Id
     * @ORM\Column(type="boolean") 
     */
    protected $passwordUpperLower;

    /** 
     * @ORM\Id
     * @ORM\Column(type="integer") 
     */
    protected $maxFailedLogins;

    /** 
     * @ORM\Id
     * @ORM\Column(type="integer") 
     */
    protected $passwordValidity;

    /** 
     * @ORM\Id
     * @ORM\Column(type="integer") 
     */
    protected $passwordExpiryDays;

    /** 
     * @ORM\Id
     * @ORM\Column(type="integer") 
     */
    protected $timeout;

    // getters/setters

    /**
     * Get the minimum length of the user ID
     * @return int 
     */
    public function getMinLengthUserId(){
        return $this->minLengthUserId;
    }

    /**
     * Set the minmum length of the user ID
     * @param int $minLengthUserId
     * @return \Application\Entity\Config This object
     */
    public function setMinLengthUserId($minLengthUserId){
        $this->minLengthUserId = $minLengthUserId;
        return $this;
    }

    /**
     * Get the minimum length of the user name
     * @return int 
     */
    public function getminLengthUserName(){
      return $this->getminLengthUserName;
    }

    /**
     * Set the minimum length of the user name
     * @param int $minLengthUserName
     * @return \Application\Entity\Config 
     */
    public function setMinLengthUserName($minLengthUserName){
      $this->minLengthUserName = $minLengthUserName;
      return $this;
    }

    /**
     * Get the minimum length of the user password
     * @return int 
     */
    public function getMinLengthUserPassword(){
      return $this->minLengthUserPassword;
    }

    /**
     * Set the minimum length of the user password
     * @param int $minLengthUserPassword
     * @return \Application\Entity\Config 
     */
    public function setMinLengthUserPassword($minLengthUserPassword){
      $this->minLengthUserPassword = $minLengthUserPassword;
      return $this;
    }

    /**
     * Get the number of days before passwords can be reused
     * @return int 
     */
    public function getDaysPasswordReuse(){
      return $this->daysPasswordReuse;
    }

    /**
     * Set the number of days before passwords can be reused
     * @param int $daysPasswordReuse
     * @return \Application\Entity\Config 
     */
    public function setDaysPasswordReuse($daysPasswordReuse){
      $this->daysPasswordReuse = $daysPasswordReuse;
      return $this;
    }

    /**
     * Get whether the passwords must contain letters and numbers
     * @return boolean 
     */
    public function getPasswordLettersAndNumbers(){
      return $this->passwordLettersAndNumbers;
    }

    /**
     * Set whether passwords must contain letters and numbers
     * @param int $passwordLettersAndNumbers
     * @return \Application\Entity\Config 
     */
    public function setPasswordLettersAndNumbers($passwordLettersAndNumbers){
      $this->passwordLettersAndNumbers = $passwordLettersAndNumbers;
      return $this;
    }

    /**
     * Get whether password must contain upper and lower case characters
     * @return type 
     */
    public function getPasswordUpperLower(){
      return $this->passwordUpperLower;
    }

    /**
     * Set whether password must contain upper and lower case characters
     * @param type $passwordUpperLower
     * @return \Application\Entity\Config 
     */
    public function setPasswordUpperLower($passwordUpperLower){
      $this->passwordUpperLower = $passwordUpperLower;
      return $this;
    }

    /**
     * Get the number of failed logins before user is locked out
     * @return int 
     */
    public function getMaxFailedLogins(){
      return $this->maxFailedLogins;
    }

    /**
     * Set the number of failed logins before user is locked out
     * @param int $maxFailedLogins
     * @return \Application\Entity\Config 
     */
    public function setMaxFailedLogins($maxFailedLogins){
      $this->maxFailedLogins = $maxFailedLogins;
      return $this;
    }

    /**
     * Get the password validity period in days
     * @return int 
     */
    public function getPasswordValidity(){
      return $this->passwordValidity;
    }

    /**
     * Set the password validity in days
     * @param int $passwordValidity
     * @return \Application\Entity\Config 
     */
    public function setPasswordValidity($passwordValidity){
      $this->passwordValidity = $passwordValidity;
      return $this;
    }

    /**
     * Get the number of days prior to expiry that the user starts getting
     * warning messages
     * @return int 
     */
    public function getPasswordExpiryDays(){
      return $this->passwordExpiryDays;
    }

    /**
     * Get the number of days prior to expiry that the user starts getting
     * warning messages
     * @param int $passwordExpiryDays
     * @return \Application\Entity\Config 
     */
    public function setPasswordExpiryDays($passwordExpiryDays){
      $this->passwordExpiryDays = $passwordExpiryDays;
      return $this;
    }

    /**
     * Get the timeout period of the application
     * @return int 
     */
    public function getTimeout(){
      return $this->timeout;
    }

    /**
     * Get the timeout period of the application
     * @param int $timeout
     * @return \Application\Entity\Config 
     */
    public function setTimeout($timeout){
      $this->timeout = $timeout;
      return $this;
    }

    /**
     * Returns the properties of this object as an array for ease of use
     * @return array 
     */
    public function toArray(){
      return get_object_vars($this);
    }

    /**
     * Before persisting this entity, check that the minimum
     * length of the user ID is not less than 1
     * @ORM\PrePersist 
     * @ORM\PreUpdate
     */
    public function assertMinLengthUserIdNotLessThan1(){
        if($this->minLengthUserId < 1){
            throw new \Exception('Minimum length of user ID cannot be less than 1.');
        }
    }
}

One of my goals is to make the application 'testable'. The applications we have written so far do not include any automated testing whatsoever, which has proved to be very time consuming.

I have several questions regarding what I've done so far:

• Is this easily testable at the moment? How could I improve it in this regard?
• Do I really need all the getters and setters or could I use a generic 'magic methods' __get and __set to do this job?
• Should I be validating input in the setter functions or is it fine to validate before persisting (like in my assertMinLengthUserIdNotLessThan1 function)?
• Is there any better way of getting the Doctrine entity into an array? I have a simple toArray() function so far but I there may be better ways of doing it for more complicated objects.

Any general comments/advice would be much appreciated also!

Thanks in advance!

Answer 1

Quick answers:

• Is it easy to test? AFAICT, with the code provided it's quite easy to test. If you follow the docs (and it looks like you did follow them rather closely) then it should be easy to test.
• Could I use magic-methods: In theory, you can, but I wouldn't. More on this later (there's a lot to be said on this matter)
• Should I be validating input in the setter: YES definitly. When some snippet of code passes the wrong value to your entity, that could mean there's a bug. If your setter just blindly accepts whatever value it gets, the line that actually sets the value won't throw up errors. If you later check if everything is as it should be, then you'll have to track back to see where the wrong value was passed... have fun debugging. Adding additional validation methods is fine, but the setter should at least check the type of the value.
• entity 2 array: The easiest (And dirtiest) approach would be (array) $entity. All in all, I feel as if this is a bad idea. If you miss your arrays that much use getArrayResult on the query object

---

On the setters and getters. First off, all magic-methods are slow, they cause way to much overhead. I've linked to an answer I gave on SO that goes into this a bit more, but basically, using the magic methods boils down to this:
Asumme you have an instance, with some private/protected properties, and a magic getter:

class Foo
{
    private $bar = null;
    public function __get($name)
    {
        if (property_exists($this, $name))
        {
            return $this->{name};
        }
        return null;
    }
    public function __set($name, $val)
    {
        return $this->{$name} = $val;
    }
}

You may think that this is very handy (and it can be) because now you can simply write $instance->bar = 123; and echo $instance->bar;. What happens internally in PHP is a different matter, though. First, the object's properties_info HashTable is searched, to get the offset of a given public property in the properties_table HashTable. If this fails (which it will), PHP performs another lookup for the __get or __set method. In this case, that will work. The function is called, and the corresponding parameters are passed as arguments. Inside the method's body, the property-lookup is repeated.
So: more work is done == more time taken == slow.
Using getter methods is faster, because the a getter method lookup is an O(1) operation. The method will access a property, that should be predefined, so that the property-lookup is O(1), too. Adding properties on-the-fly (and then getting too their respective values), on the other hand is O(n) (the more you do this, the slower it gets).

Furthermore, magic methods, essentially expose private or protected properties. Why bother using access modifiers, if you're going to allow the users of your code to mess with every property your object has to offer anyway? Simple example:

class Wrapper
{
    protected $conn = null;
    private $params = array(/*default settings here*/);
    public function __construct($wsdl)
    {
        $this->conn = SoapClient($wsdl, $this->params);
    }
    public function getData(array $params)
    {//get something, no worries
        return $this->conn($params);
    }
    public function deleteAll()
    {
        throw new RuntimeExcpetion('Delte is disabled, are you insane');
    }
    public function __get($name)
    {
        return $this->{$name};
    }
}

What is preventing me from doing this:

$soap = new Wrapper();
try
{
    $soap->deleteAll();
}
catch(RuntimeExcpetion $e)
{//I can't delete via wrapper, ok: use client directly, then...
    $soap = $soap->conn;//magic getter returns client!
    $soap->deleteAll();
}

Of course, you might think that adding some simple checks in the magic getter will prevent me from doing this (and it will), but the more logic you add to __get, the slower it becomes.
Then think of inheritance:

class ExposeWrapper extends Wrapper
{
    public function __get($n)
    {
        return $this->{$n};
    }
}

All I had to do was extend your class, and overwrite the getter, to regain access to the protected properties. Defining the magic methods as final might seem like an option, but it'll severly limit your ability to extend from your own classes (and thus your abilities to reuse your code later on). If all objects have one or 2 properties they want to prevent from being exposed, they all have a final getter defined, and can't extend from each other. Does that sound like proper OO code to you? Of course not

As if that weren't bad enough, consider what a mess this would leave you with, when you consider typo's (very, if not _the _most, common cause of bugs):

protected $id, $foo, $bar;//three fields in entity
public function __set($name, $val)
{
    $this->{$name} = $val;
    return $this;
}
//on the instance:
$entity->baz = 'typo';//!!

I've added a fourth property. That doesn't make sense. I might then go on to assume I've set the bar property, and then spent ages tracking down a silly typo. Still, I, too, often implement __get and __set, with a difference:

public function setBar($val)
{
    if ($val != (string) $val)
    {
        throw new InvalidArgumentException(__FUNCTION__ .' expects string (or string-like argument not '. typeof $val);
    }
    $this->bar = (string) $val;
    return $this;
}
public function __set($name, $val)
{
    $name = 'set'.ucfirst(strtolower(trim($name)));
    if (!method_exists($this, $name))
    {
        throw new RuntimeException(get_class($this).' cannot be overloaded no method: '.$name);
    }
    return $this->{$name}($val);
}

And the same for the getter. This allows you to do something like:

$entity->id = 123;

But avoids things like:

$entity->idd = 123;//typo
$entity->idd = new stdClass;//check int||numeric in setter of course

Another reason to implement individual getters/setters is to program by interface. Suppose you have a dozen of data models (which isn't a lot). A couple of these models share some data (like Client and CurrentUser or something). Let's, for example, assume they both have a property that contains the email address. Since these models are linked to a particular table, it's not unlikely that the properties have different names (eg email and mailAddress).
When using type-hints, which I hope you do, you can't just hint to the abstract model type (some models don't have the required property/method). Enter Interface:

interface HasEmail
{
    public function getEmail();
    public function setEmail($email);
}

class Client extends AbsctractModel implements HasEmail
{
    private $email = null;
    public function getEmail()
    {
        return $this->email;
    }
    public function setEmail($email)
    {
        $this->email = $email;
        return $this;
    }
}

class CurrentUser extends AbsctractModel implements HasEmail
{
    private $mailAddress = null;//no email property
    public function getMailAddress()
    {
        return $this->mailAddress;
    }
    public function setMailAddress($mail)
    {
        if (!filter_var($mail, FILTER_VALIDATE_EMAIL))
        {
            throw new InvalidArgumentExcetion('invalid email address: '.$mail)
        }
        $this->mailAddress = $mail;
        return $this;
    }
    public function getEmail()
    {
        return $this->mailAddress;
    }
    public function setEmail($email)
    {
        return $this->setMailAddress($email);
    }
}

With this code, you can use a type-hint that assures you the argument has specific methods, that will return you the expected data:

public function updateEmail(HasEmail $instance)
{
    $newAddress = $instance->getEmail();
}

A since a class can implement multiple Interfaces, and interfaces can extend eachother, they offer a very flexible, and solid way of assuring the correct objects are being passed to the right methods, at the right time.

Lastly, I've said that magic methods are slower... properties that are added using __set (ie properties that are not part of the class definition) are slower, too. That's another reason to disable PHP's object overloading. See here for more details