Every two weeks I send out a newsletter containing lots of interesting stuff for the modern PHP developer. You can expect quick tips, links to interesting tutorials, opinions and packages. Want to learn the cool stuff? Then sign up now!

Further refactoring code for readability

A few days ago Dylan Bridgman published a post on writing highly readable code. He cleaned up a truly horrible piece of code. The code was further improved the very same day by Ryan Winchester.

I believe the code can be improved still. Read the mentioned blog posts to see which code we are going to improve. Warning: I’m going to nitpick.

The readability of the code can be improved:

  • by adding docblocks to the variables and methods of the class. I makes it very clear what type of parameters are passed an returned.
  • The age variable isn’t used. Reading dead code is useless, so let’s remove it.
  • Also if the variables or protected instead of private, they can be used when inheriting from the class.

So in my mind this is better:


class User
{
/**
* @var string
*/
protected $name;

/**
* @var DateTime
*/
protected $birthday;

/**
* @param string $name
* @param DateTime $birthday
*/
public function __construct($name, DateTime $birthday)
{
$this->name = $name;
$this->birthday = $birthday;
}

/**
* @return string
*/
public function calculateAge()
{
return $this->birthday->diff(new DateTime)->format("%y");
}
}

The last bit of code in Ryan’s post is a foreach loop. By using array_map instead the variable inside the loop can be typehinted. This further increases readability.


array_map(function(User $user) {
echo $user->calculateAge();
}, $users);

In this case you could argue that using array_map is overkill. But there’s a good chance that in production code there isn’t an array initialized with the objects just above the loop.

When working with large chunks of code these kind of details matter. Code gets read a lot more times than it is written, so it should be highly optimized for reading.

Freek Van der Herten is a partner and developer at Spatie, an Antwerp based company that specializes in creating web apps with Laravel. After hours he writes about modern PHP and Laravel on this blog. When not coding he’s probably rehearsing with his kraut rock band. He loves waffles and butterflies.
  • It’s interesting to see code evolve as different people refactor it. I’m glad you took the time to give this additional insight. I’ve added a link to your article from my original one so that anyone starting there can also follow the progression.

  • Corez

    If you convert users into a collection rather than just an array you would end up with

    $users->map(function(User $user) {
    echo $user->calculateAge();
    });

    Which I think is even more readable because the unfortunate thing with the array_map function is that the array parameter is the second argument, which was fine before anonymous functions, but it may now be several lines below the opening of the function and isn’t always obvious.

    • Shalvah Adebayo

      Yeah, this is better. In my opinion, `array_map` is less readable than `foreach`.

  • Shalvah Adebayo

    To be honest, Freek, the whole introducing classes thing might be overkill without any knowledge of the context of the code, such as if it were a simple form submit.