Refactoring of model for testing purpose

Question

Hi All,

I want to refactor my model so I can properly write a unit test for it. But I have some dependencies. Can anybody point me in the right direction how I can remove these dependencies?

class Customer
{
    public function save(array $data, $id = NULL)
    {
        // create or update
        if (empty($id)) {
            $customer = new \Entities\Customer();
        } else {
            $customer = $this->findById((int) $id);
        }

        $birthday = new DateTime();
        list($day, $month, $year) = explode("/", $data['birthday']);
        $birthday->setDate($year, $month, $day);

        $customer->setFirstName($data['firstName']);
        $customer->setLastName($data['lastName']);
        $customer->setCompany($data['company']);
        $languageModel = new Application_Model_Language();
        $customer->setLanguage($languageModel->findById($data['language']));
        $resellerShopModel = new Application_Model_ResellerShop();
        $customer->setResellerShop($resellerShopModel->findById($data['resellerShop']));
        $customer->setEmail($data['email']);
        $customer->setPhone($data['phone']);
        $customer->setGender($data['gender']);
        $customer->setBirthday($birthday);
        $customer->setType($data['type']);
        $customerSectorModel = new Application_Model_CustomerSector();
        $customer->setSector($customerSectorModel->findById($data['sector']));
        $customerReferenceModel = new Application_Model_CustomerReference();
        $customer->setReference($customerReferenceModel->findById($data['reference']));
        if (isset($data['password'])) {
            $customer->setPassword($this->_hashPassword($data['password']));
        }

        return $customer;
    }
}

Answer 1

I guess your dependencies are the constructor calls in the function body. Out of my mind there are 3 ways to replace them in the unit test:

1. Create a mock library where all the different classes are implemented, and to run the test include the mock library instead of the real modules.
2. Add a set of default parameters for the external classes to your function declaration. In the end your new function declaration would look like public function save(array $data, $id = NULL, $newCustomer=\Entities\Customer(), $newLangModel = Application_Model_Language, ...). Also in the function body you use the variables to create the actual objects, like $customer = new $newCustomer(). In the test code you can override every depended class by a mock class.

3. You do not add a parameter for every class, but create two factories: one which creates the current objects, and one which creates mock objects. Inside of the function you request new objects only from the factory.

   This approach is useful if there are many different places where the construction should be intercepted. If there is only one function which should be changed, a factory is over-engineering.

Answer 2

After looking back over your code, it appears this class is acting as both a Factory and Repository to the \Entities\Customer class (this was not obvious, so you could consider renaming to be more explicit your intentions). I also disagree with that function being named save since it is only returning an object that then needs to be persisted, but that is more semantics.

With the code as you have it now, I see two different tests that are needed.

1) Test your \Entities\Customer class.

Verify that each of your getters and setters are working correctly, or at least any one that has business logic behind it. For instance make sure that if you set the ResellerShop, then if you get a correct/valid ResellerShow object. More or less, you need to test your business logic. Get/Set of basic fields (ex. Name) don't really necessitate their own tests (unless 100% code coverage is a goal, I don't think it is).

2) Test the (Factory\Repository) class.

Ensure the class you showed correctly creates (or fails) depending on the data passed into the array. For instance it should fail if required fields are not passed in, and it should not return a customer entity object.

The idea is for separation of concerns. Your Entity class should contain your business logic and should be tested separately from your object instantiation code.