php type checking for method parameters - is it worth it?

Question

I'm wondering what you think the best practice is here-- does it buy you very much to type-check parameters in PHP? I.e have you actually seen noticeably fewer bugs on projects where you've implemented parameter type-checking vs. those that don't? I'm thinking about stuff like this:

public function __construct($screenName, $createdAt) {  
        if (!is_string($screenName) || !is_string($createdAt) {
            return FALSE;
        }
}

Answer 1

Checking function arguments is a very good practice. I suspect people often don't do that because their functions grow bigger and the code becomes uglier and less readable. Now with PHP 7 you can type-hint scalar types but there is still no solution for cases when you want your parameter to be one of two types: array or instance of \Traversable (which both can be traversed with foreach).

In this case, I recommend having a look at the args module from NSPL. The __constructor from your example will have the following look:

public function __construct($screenName, $createdAt)
{
    expectsAll(string, [$screenName, $createdAt]);
}

// or require a non-empty array, string or instance of \ArrayAccess
function first($sequence)
{
    expects([nonEmpty, arrayAccess, string], $sequence);
    return $sequence[0];
}

More examples here.

Answer 2

Normally within a PHP application that makes use of the skalar variable "types" is bound to actually string input (HTTP request). PHP made this easier so to convert string input to numbers so you can use it for calculation and such.

However checking scalar values for is_string as proposed in your example does not make much sense. Because nearly any type of variable in the scalar family is a string or at least can be used as a string. So as for your class example, the question would be, does it actually make sense to check the variable type or not?

For the code you proposed it does not make any sense because you exit the constructor with a return false;. This will end the constructor to run and return a not-properly-initialized object.

Instead you should throw an exception, e.g. an InvalidArgumentException if a constructors argument does not provide the expected / needed type of value.

Leaving this aside and taking for granted that your object constructor needs to differ between a string and an integer or bool or any other of the scalar types, then you should do the checks.

If you don't rely on the exact scalar types, you can cast to string instead.

Just ensure that the data hidden inside the object is always perfectly all-right and it's not possible that wrong data can slip into private members.