This article is part of my #100DaysOfCode and #100DaysOfBlogging challenge. R1D22


A mentor commented on my yesterday’s solution to the Grade School exercise.

Good solution

Awesome! using strict_types

It’s approved for me but try to use return types as much as possible, I’m missing initGradeIfItDoesNotExist

This post will reflect some of my personal thoughts. Since the mentor’s suggestion could be just some personal preference, I will not name the mentor.

I am very grateful for each one of them taking time and providing valuable feedback. This has been an awesome experience. And since I am doing these exercises to learn, the caused reflection is what I am looking for. Thank you!

Reflection

The suggestion to “use return types as much as possible” made me review my code.

private function initGradeIfItDoesNotExist(int $grade) : void
{
    if (! isset($this->students[$grade])) {
        $this->students[$grade] = [];
    }
}

So first off, initGradeIfItDoesNotExist() has a declared return type. Okay, it is void. And secondly, how could an actual return value improve the method and what should be returned?

A value should only be returned, if it is required. A returned value adds complexity. It is a dependency, to the code invoking the method. And it probably needs some validation, if the returned variable is what is expected.

Since initGradeIfItDoesNotExist() can mutate $this->students, it could return either that or $this->students[$grade]. Both variables are an array. The return type could be changed to array. And the caller could possibly check if the returned value is an array of at least 0 elements. Additionally the if condition could check, if $this->students[$grade] is set that is is an array. Ending up with:

private function initGradeIfItDoesNotExist(int $grade) : array
{
    if (! isset($this->students[$grade])
        || ! is_array($this->students[$grade])
    ) {
        $this->students[$grade] = [];
    }

    return $this->students[$grade];
}

Maybe I am missing something here, but that does not make any sense to me.

You have thoughts on that? Happy to hear about it on Twitter.