Refactoring argument checking

4-minute read

Where I test exceptions because bad arguments happen.

Testing for the input

Sadly not all the input to a function or method can be type-hinted and implicitly checked for conformity.
Getting back to the WPSchedule_Schedule_Schedule class, the star of the wp-schedule show, I want to test that the method schedule will throw an Exception when the hook name is not a string or is an empty string; right now the method is not complicated at all

public function schedule( WPSchedule_Time_TimeInterface $activation_time, WPSchedule_Interval_IntervalInterface $recurrence_interval, $hook, array $args = null ) {

    wp_schedule_event( $activation_time->getTime(), $recurrence_interval->getInterval(), $hook, $args );
}

but, beside for time and interval type hinting, the $hook argument could be anything.
I’ve set up a test method and a corresponding test provider for the purpose

public function badHookArgs() {
    return [
        [ 23 ],
        [ '' ],
        [ array() ],
        [ new stdClass() ],
        [ null ]
    ];
}

/**
 * @test
 * it should fail if calling with wrong or missing args
 * @dataProvider badHookArgs
 */
public function it_should_fail_if_calling_with_wrong_or_missing_args( $badHookArg ) {
    $now = time();
    $time = FunctionMocker::replace( '\WPSchedule_Time_TimeInterface::getTime', $now );
    $interval = FunctionMocker::replace( '\WPSchedule_Interval_IntervalInterface::getInterval', 500 );

    $this->setExpectedException( '\Exception' );

    $this->sut->schedule( $time, $interval, $badHookArg );
}

Where the test provider method will allow me to add tests at pleasure later should the need arise.
Without any check in place the tests will fail hook argument checking tests failing

Evolving the argument check

First line of defense is adding a check for the $hook arg in the schedule method

    public function schedule( WPSchedule_Time_TimeInterface $activation_time, WPSchedule_Interval_IntervalInterface $recurrence_interval, $hook, array $args = null ) {
        if ( ! is_string( $hook ) || empty( $hook ) ) {
            throw new Exception( 'Hook name must be a non empty string' );
        }

        wp_schedule_event( $activation_time->getTime(), $recurrence_interval->getInterval(), $hook, $args );
    }

and that will do it hook argument checking tests passing

Refactoring

The following phase of TDD is refactoring and that I will do to keep code DRY; I will need to check for the $hook argument in any of the class methods accepting it as an argument and it makes sense to create a private method to check for the argument that current and future methods of the class will be able to use; simply moving code I add this method to the class

private function checkHook( $hook ){
    if ( ! is_string( $hook ) || empty( $hook ) ) {
        throw new Exception( 'Hook name must be a non empty string' );
    }
}

and will call it refactoring the schedule method

public function schedule( WPSchedule_Time_TimeInterface $activation_time, WPSchedule_Interval_IntervalInterface $recurrence_interval, $hook, array $args = null ) {
    $this->checkHook($hook);

    wp_schedule_event( $activation_time->getTime(), $recurrence_interval->getInterval(), $hook, $args );
}

Good laziness

A good thing about TDD is that it makes me lazy: to write code I have to test it so the more code I write the more I have to write tests.
I don’t want to write more tests that’s needed.
Refactoring the argument checking into a method of its own allows me to test the whole class code, “cover it”, without having to test that any method accepting an $hook argument throws if an invalid $hook argument is passed to the method: I just need to test it in one place.
Do I really need to test such trivial code? Probably not right now but might need to do it in the future as the class evolves and the $hook parameter becomes more complex and contextual: in that occurrence I will just need to modify/update the code in one place.

Even more laziness

The version of the project that’s on GitHub testifies to my laziness as a TDDer even more: I’ve developed an argument checking utility class that’s PHP 5.2 compatible and that I’m using to make the checks.
The checkHook method evolves to

private function checkHook( $hook ) {
    Arg::_( $hook, "Hook name" )->is_string()
       ->assert( ! empty( $hook ), 'Hook name must be a non empty string' );
}

Which will give the caller a more precise exception.
Why that? Because that class is tested already and I know it will work as intended. Same concept as before, just worse.

Next

Time to wrap up the wp-schedule package.