Refactoring Multiple If Statements to Objects

Arie Visser • November 10, 2021

php refactoring

In this article I would like to describe how you can refactor a method that contains multiple if-statements by creating objects with a single responsibility, each making their own assertions.

Precondition Checks

Most functions you write, need precondition checks. Precondition checks might for example validate the arguments that are passed to the function, prevent failure scenarios, or make assertions about the state of the data in your system. When one of these checks fail, the function will not be executed, or an exception is thrown.

One example I came across in a project I am working on, was a function that adds an attempt to a course. When a student finishes a course exam, the provided answers are stored as a course attempt, that may lead to a certificate for the course.

Before the course attempt with the answers could be stored, there were many precondition checks, that covered multiple scenarios.

The function ended up to something like this:

use App\Exceptions\CreateCourseAttemptException;
use App\Models\CourseAttempt;
use App\Repositories\CourseAttemptRepository;
use Carbon\Carbon;

class CreateCourseAttempt
{
    public function __construct(private CourseAttemptRepository $courseAttemptRepository) 
    {
    }

    public function handle(Course $course, array $answers): CourseAttempt
    {
        if (Carbon::parse($course->expiration_date)->lt(Carbon::now())) {
            throw new CreateCourseAttemptException('Course expired');
        }

        if (count($course->attempts) >= $course->max_attempts){
            throw new CreateCourseAttemptException('Too many attempts');
        }

        if ($course->hasCertificate()) {
            throw new CreateCourseAttemptException('Course already finished');
        }

        return $this->courseAttemptRepository->store($course->id, $answers);
    }
}

A feature request was to add a new possibility, where the admin could create a course attempt for the student. In that case, not all precondition checks were to be executed, it wouldn't matter how many previous attempts were made. Enabling this would lead to an additional parameter and if-statement.

Functions can get a very high complexity over time because of these precondition checks.

Why Refactor?

The first incentive to refactor this method, was that at this point PHPStorm started warning about violations of the single-responsibility principle. The method tries to cover too many scenarios.

Additionally, there is no organisation principle in the form of an abstraction, for the precondition checks when using if-statements.

In some cases, precondition checks can be very complex. In the example above, each if-statement contains a condition that is based on a single comparison or value, but in reality an if-statement may contain checks for many comparisons and values.

Last but not least, testing functions with many precondition checks can get really hard, because all the scenarios need to be covered in the data setup of the test. When you just want to test the action of the method (in this case storing a course attempt), it is impossible to skip the precondition checks by creating test doubles. It is also impossible to test a precondition check separately, without running the other checks.

How to Refactor Precondition Checks to Objects?

First, we will create an abstraction for the checks, in this case CourseAttemptRejectionContract:

use App\Models\Course;

interface CourseAttemptRejectionContract
{
    public function handle(Course $course): bool;

    public function message(): string;
}

It not only contains the handle method for the check itself, but can also return a message that will show why the check failed.

For each precondition check, we can can now create an implementation of the CourseAttemptRejectionContract:

use App\Assertions\Contracts\CourseAttemptRejectionContract;
use App\Models\Course;
use Carbon\Carbon;

class CourseExpired implements CourseAttemptRejectionContract
{
    public function handle(Course $course): bool
    {
        return Carbon::parse($course->expiration_date)->lt(Carbon::now())
    }

    public function message(): string
    {
        return 'Course expired';
    }
}
use App\Assertions\Contracts\CourseAttemptRejectionContract;
use App\Models\Course;

class TooManyAttemptsForCourse implements CourseAttemptRejectionContract
{
    public function handle(Course $course): bool
    {
        return count($course->attempts) >= $course->max_attempts;
    }

    public function message(): string
    {
        return 'Too many attempts';
    }
}
use App\Assertions\Contracts\CourseAttemptRejectionContract;
use App\Models\Course;

class CourseAlreadyFinished implements CourseAttemptRejectionContract
{
    public function handle(Course $course): bool
    {
        return $course->hasCertificate();
    }

    public function message(): string
    {
        return 'Course already finished';
    }
}

In the handle method of CreateCourseAttempt, we now only need an array of objects that implement CourseAttemptRejectionContract, in order to check if we can store the attempt:

use App\Assertions\Contracts\CourseAttemptRejectionContract;
use App\Exceptions\CreateCourseAttemptException;
use App\Models\Course;
use App\Models\CourseAttempt;
use App\Repositories\CourseAttemptRepository;
use Assert\Assertion;

class CreateCourseAttempt
{
    public function __construct(
        private CourseAttemptRepository $courseAttemptRepository,
        private ?array $courseAttemptRejections 
    ) 
    {
    }

    public function handle(Course $course, array $answers): CourseAttempt
    {
        foreach ($this->courseAttemptRejections as $rejectionClass) {
            $rejection = new $rejectionClass();

            Assert::that($rejection)->isInstanceOf(CourseAttemptRejectionContract::class);

            if ($rejection->handle($this->course)) {
                throw new CreateCourseAttemptException($rejection->message());
            }
        }

        return $this->courseAttemptRepository->store($course->id, $answers);
    }
}

When all precondition checks are to be made, the service can be called like this:

use App\Actions\CreateCourseAttempt;
use App\Assertions\CourseExpired;
use App\Assertions\TooManyAttemptsForCourse; 
use App\Assertions\CourseAlreadyFinished;

$createCourseAttemptAction = new CreateCourseAttempt(
    $courseAttemptRepository,
    [
        CourseExpired::class,
        TooManyAttemptsForCourse::class,
        CourseAlreadyFinished::class,
    ]   
);
$createCourseAttemptAction->handle($course, $answers);

However, when the admin creates an attempt, it is possible to skip the TooManyAttemptsForCourse precondition check, like this:

use App\Actions\CreateCourseAttempt;
use App\Assertions\CourseExpired;
use App\Assertions\CourseAlreadyFinished;

$createCourseAttemptAction = new CreateCourseAttempt(
    $courseAttemptRepository,
    [
        CourseExpired::class,
        CourseAlreadyFinished::class,
    ]   
);
$createCourseAttemptAction->handle($course, $answers);

You can now write unit tests for the create course attempt action with the help of test doubles. It is possible to create a stub object for the precondition checks, for example one that always returns true:

use App\Assertions\Contracts\CourseAttemptRejectionContract;
use App\Models\Course;

class CourseAttemptRejectionStub implements CourseAttemptRejectionContract
{
    public function handle(Course $course): bool
    {
        return true;
    }

    public function message(): string
    {
        return 'Failure';
    }
}

use PHPUnit\Framework\TestCase;
use App\Actions\CreateCourseAttempt;
use App\Exceptions\CreateCourseAttemptException;
use App\Models\Course;
use App\Repositories\CourseAttemptRepository;
use Tests\Stubs\CourseAttemptRejectionStub;

class CreateCourseAttemptTest extends TestCase
{
    public function testActionPreconditionFailed(): void
    {
        $course = new Course();
        $courseAttemptRepository = new CourseAttemptRepository();

        $createCourseAttemptAction = new CreateCourseAttempt(
            $courseAttemptRepository,
            [
                CourseAttemptRejectionStub::class,
            ]   
        );

        $this->expectException(CreateCourseAttemptException::class);

        $createCourseAttemptAction->handle($course, ['answer']);
    }

    public function testActionSuccessfulWithoutChecks(): void
    {
        $course = new Course();
        $courseAttemptRepository = new CourseAttemptRepository();

        $createCourseAttemptAction = new CreateCourseAttempt(
            $courseAttemptRepository,
            [
            ]   
        );

        $this->assertTrue($createCourseAttemptAction->handle($course, ['answer']));
    }
}

In this example we create the stub ourselves, but you can also create mocks with PHPUnit.

A unit test for a single precondition check could look like this:

use PHPUnit\Framework\TestCase;
use App\Assertions\TooManyAttemptsForCourse;

class CreateCourseAttemptTest extends TestCase
{
    public function testTooManyAttemptsPreconditionFailed(): void
    {
        $course = new Course([
            'attempts' => 3,
            'max_attempts' => 3,
        ]);

        $condition = new TooManyAttemptsForCourse();

        $this->assertEquals(true, $rejection->handle($course));
    }

    public function testTooManyAttemptsPreconditionPassed(): void
    {
        $course = new Course([
            'attempts' => 3,
            'max_attempts' => 3,
        ]);

        $condition = new TooManyAttemptsForCourse();

        $this->assertEquals(true, $rejection->handle($course));
    }
}

This way of adding precondition checks to your methods, will give freedom to unit test whichever part of the method you like.