HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpMinor

Testable code with ORM factory

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
withormtestablefactorycode

Problem

I'm trying to improve the following code for testability.

public function can_apply_extension()
{

    $search_dates = [
        Carbon::createFromTimestamp(strtotime($this->billing_verified_to))->subMonth(1),
        $this->billing_verified_to
    ];

    $record = ORM::factory('a model')
                 ->where('created_at', 'BETWEEN', $search_dates)
                 ->find();

    if( !$record->loaded() ) 
    {
        return true;
    }

    return false;

}


Here's how I think I can improve it:

public function can_apply_extension($search_from = null, $search_to = null)
{

    $search_from = is_null($search_from) ? Carbon::createFromTimestamp(strtotime($this->billing_verified_to))->subMonth(1) : $search_from;
    $search_to = is_null($search_to) ? $this->billing_verified_to : $search_to

    $search_dates = [$search_from, $search_to];

    $record = ORM::factory('a model')
                 ->where('created_at', 'BETWEEN', $search_dates)
                 ->find();

    if( !$record->loaded() ) 
    {
        return true;
    }

    return false;

}


I still have to mock ORM part. I think the only option is that I pass an object as a parameter or make a method that injects the ORM object.

Are there any other better ways? I'm aware of IoC container, but I can't use it in our current environment.

Update: Beside passing ORM as a parameter so that I can just swap it out, I found another workaround.

Since ORM::factory() actually returns an object, I was able to add a method and modify the factory method.

public function testing($bool)
{ 
    $this->_testing = $bool;
}

public function instead($when_this_passed, ORM $use_this_instead)
{
    $this->_instead_map[$when_this_passed] = $use_this_instead;
}

public function factory($name)
{
    if($this->_testing && array_key_exists($name, $this->_instead_map))
    {
        return $this->_instead_map[$name];
    } 
    else
    {
        parent::factory($name);
    }
}

Solution

(I've not coded in PHP recently, my examples are Java or Java-like pseudocode.)

In his Working Effectively with Legacy Code book Michael C. Feathers describes a lot of techniques which help making any code testable. Another good resource is the following article: Killing the Helper class, part two

The usual way to breaking static dependencies is the following: Create a non-static interface which has a similar method than the original static class:

public interface OrmFactory {
    OrmSomething create($name);
}


Then create an implementation which calls the static method/class:

public class OrmFactoryImpl implements OrmFactory {
    public OrmSomething create($name) {
        return ORM::factory($name);
    }
}


Finally, use the interface in the can_apply_extension function:

public class MyClass {

    private OrmFactory ormFactory;

    public MyClass(OrmFactory ormFactory) {
        this.ormFactory = ormFactory;
    }

    public void can_apply_extension() {
        ...
        $record = ormFactory.create('a model')
                     ->where('created_at', 'BETWEEN', $search_dates)
                     ->find();
        ...
    }
}


In tests you can pass a mocked OrmFactory to the class while in production code you can use the original one through the OrmFactoryImpl wrapper/delegate. (I guess PHP also has some mocking framework implementations which makes testing easier but you can implement a TestOrmFactory which emulates some behaviour of the original one without the need of any database.)

The second snippet in the question has test logic in production smell. I would avoid that and use the much cleaner dependency breaking above. The linked page contains some disadvantages of the smell. One of them is the following:

Code that was not designed to work in production
and which has not been verified to work properly
in the production environment could accidently be
run in production and cause serious problems.

And another one is:

Software that is added to the SUT For Tests Only
makes the SUT more complex. It can confuse potential
clients of the software's interface by introducing
additional methods that are not intended to be
used by any code other than the tests. This
code may have been tested only in very specific
circumstances and might not work in the typical
usage patterns used by real clients.

(SUT means System Under Test, in this case the class with can_apply_extension().)

Code Snippets

public interface OrmFactory {
    OrmSomething create($name);
}
public class OrmFactoryImpl implements OrmFactory {
    public OrmSomething create($name) {
        return ORM::factory($name);
    }
}
public class MyClass {

    private OrmFactory ormFactory;

    public MyClass(OrmFactory ormFactory) {
        this.ormFactory = ormFactory;
    }

    public void can_apply_extension() {
        ...
        $record = ormFactory.create('a model')
                     ->where('created_at', 'BETWEEN', $search_dates)
                     ->find();
        ...
    }
}

Context

StackExchange Code Review Q#37776, answer score: 3

Revisions (0)

No revisions yet.