PHP 7.2: the pitfalls to avoid for a better and cleaner code

What are the pitfalls to avoid with this new version of PHP and its new functionalities?

As my previous article on PHP 7 scalar and return types pitfalls, instead of describing the new super cool features of this perfect PHP 7.2, I will describe what can be dangerous for the good health of your application.

As Ward Cunningham stated it: "You know you are working on clean code when each routine you read turns out to be pretty much what you expected." As you will see, this new PHP version can make your code pretty confusing if you don't know when to use those new functionalities.

Update: after reading some reactions about this article on reddit, on another article and after reading some comments on the PR, I modified this article to be more nuanced.

Misuse of the new object type

Without further ado, here an example of this new type:

<?php declare(strict_types=1);

class LemonPie  
{
    public $ingredient = 'lemon';
}

class Car  
{
    public $wheel = 4;
}

function wheelCounts(object $vehicle): string  
{
    return sprintf("This vehicle has %d wheels", $vehicle->wheel);
}

echo wheelCounts(new LemonPie);  

The output will be a beautiful Notice: Undefined property: LemonPie::$wheel.

This new scalar type object allows instances of any class as argument to your wheelCount function. At the end you try to display the number of wheel of a... lemon pie.

I can hear you in my head: "but yes but why he didn't use a vehicle in there?".

Well, I think we have the right to use a LemonPie object since the function wheelCounts states that any instance can be used.

This example is trivial but imagine on a large scale application. You need to use a method in a totally different part of your shiny app. You don't care about the implementation of it, you just want to use it.

However this method has an object as argument type:

  • What object to pass to this method? Maybe an instance of stdClass. Maybe not. Who knows?
  • You will have to go through the implementation and try to understand what type this method really expect as argument. It's annoying and shouldn't be necessary.
  • If an error is thrown because you didn't pass the real type used in the method itself, the error message won't really describe the root cause of the error. Look at the example above: the problem is not that the lemon pie has no wheel, we simply shouldn't pass the object LemonPie as argument!

In short: this object type can bring confusion and doubts.

Let's come back to our trivial example. What would be a better solution?

<?php declare(strict_types=1);

interface Dish {}

interface Vehicle {}

class LemonPie implements Dish  
{
    public $ingredient = 'lemon';
}

class Car implements Vehicle  
{
    public $wheel = 4;
}

function wheelCounts(Vehicle $vehicle): string  
{
    return sprintf("This vehicle has %d wheels", $vehicle->wheel);
}

echo wheelCounts(new LemonPie);

The result? PHP Fatal error: Uncaught TypeError: Argument 1 passed to wheelCounts() must implement interface Vehicle, instance of LemonPie given.

Thanks to the dependency inversion principle, it's now clear what argument the method wheelCounts expect: a Vehicle. Nobody needs to go to the implementation, everything is clearly stated. Even the error message explains the root problem.

But wait! It's not over. object is not only a scalar type. You can use this object type as return type as well!

<?php declare(strict_types=1);

class LemonPie  
{
    public $ingredient = 'lemon';
}

class Camel {}

function getVehicle(Vehicle $vehicle): object  
{
    return new LemonPie;
}

This return is confusing, hypothetical on the real nature of this object. A developer shouldn't have to guess.

I disagree with some of the RFC examples as well: you shouldn't use any instance of stdClass (like the return of the json_encode function) in your code.

stdClass doesn't have any meaning. An object should have a behavior and be properly named for everybody to understand its role in the application. You need to decode some json? Use an array or wrap the stdClass instance with a fully qualified class.

Is the object type useless?

At that point you can think this new type shouldn't have been implemented in PHP 7.2.

Actually it can be very useful.

Let say one of your method can accept indeed any type of object in order to do some general operation on them (freezing / saving in memory / caching / serializing...). For those cases having a type object is better than not having any typehint at all.

Another case I can think of: refactoring a legacy application. If you have a codebase without any typehint, using object return types for some obscure method returning a bunch of different objects can be the first step to sanity.

However on the long term I would create interfaces and split those "methods which can return everything", throwing away the scalar / return type object. As a rule of thumb, if your methods can accept (or return) multiple types of object, you should first try to split it.

This is the general rule I would follow: you need to specify the object type as narrowly as allowed by the language. You need a Vehicle instance in your method? Specify it clearly by using the Vehicle type.
If in some rare cases you can't narrow your object types using interfaces, object can be a better solution than no types at all.

Parameter Type Widening

PHP 7.2 allows abstract function override as following:

<?php declare(strict_types=1);

abstract class Canidae {  
    abstract public function getAnimalSound(string $tone): AnimalSound;
}

class AnimalSound  
{
}

class Dog extends Canidae  
{
    public function getAnimalSound($tone): AnimalSound
    {
    }
}

With PHP 7.1 a Fatal error would be thrown: Declaration of Dog::getAnimalSound($tone): AnimalSound must be compatible with Canidae::getAnimalSound(string $tone): AnimalSound.

With PHP 7.2, everything is fine. It means that $tone can be of any type. Confusing?

Reading the RFC you could think you can use parameter type widening only on abstract classe's children.
Therefore you could think that declaring an interface to enforce the string type would work.

Well...

<?php declare(strict_types=1);

interface Animal {  
    public function getAnimalSound(Tone $tone): AnimalSound;
}

class Tone {}


class Dog implements Animal  
{
    public function getAnimalSound($tone): AnimalSound
    {
    }
}

This code doesn't throw any error.
We know now it's not the case.

I know some will disagree but to me the contract becomes less clear, open to hypothesis and doubts. Now $tone can be anything and everything... depending on the implementation.

If you expect every implementation of your interface to always have a Tone instance as argument, well don't expect it too much anymore. A developer can simply forgot it, omit the type to hack around.

That's why I would advice to be extra careful with these new rules. First you need to be aware of them and second you shouldn't use them except if you really need this extra widening.

Do we really need those functionalities?

Let's be clear: PHP 7.2 bring a lot of improvements (argon2 as password hash, disallowing null as parameter for get_class()...). I don't deny that.

I came to the conclusion that we need those functionalities but again, we need to know what it implies when we use them. Ask yourself if you can make your types more narrow before using them.