ghedipunk / PHP-Websockets

A Websockets server written in PHP.
BSD 3-Clause "New" or "Revised" License
913 stars 375 forks source link

php 7 strict mode #69

Open Xaraknid opened 8 years ago

Xaraknid commented 8 years ago

Here what I found so far.

strict type not activate afunction( int $a ) {echo $a }

1 -  $a=3     return 3
2 - $a='3'    return 3
3 - $a='3a'  return 3 and flag PHP Notice:  A non well formed numeric value encountered
4 - $a='a'   PHP Fatal error:  Uncaught TypeError: Argument 1 passed to afunction() must be of the type integer, string given, called in 
>> abort code or  try block if you use try...catch

strict type activate

1 -  $a=3     return 3
2 - $a='3'   PHP Fatal error:  Uncaught TypeError: Argument 1 passed to afunction() must be of the type integer, string given, called in 
>> abort code or  try block if you use try...catch
3 - $a='3a'  PHP Fatal error:  Uncaught TypeError: Argument 1 passed to afunction() must be of the type integer, string given, called in
>> abort code or  try block if you use try...catch
4 - $a='a'   PHP Fatal error:  Uncaught TypeError: Argument 1 passed to afunction() must be of the type integer, string given, called in 
>> abort fullcode or  try block if you use try...catch

It do what you expect that should doing. Also I can clearly see it's purpose in a SAPI or "CLI script running once". You want to abort full or portion of code base on invalid type passing by argument or returning value.

Maybe I'm wrong and just didn't know how to do it. For a server you expect it to stay online, if a user be able to send data that will break any type hinting you provide in library, server will go down or a portion of it will break.

I see 2 solution : 1 - rely on casting type like a freak that in end defeat in my sense the purpose of strict type only moving the "validation" outside of function. 2 - Having try...catch for nearly every function. This lead in a wonderfull nested try...catch library

ghedipunk commented 8 years ago

In my opinion, it makes little sense to enforce strict typing in the library itself.

We can not (and should not, according to the inflated egos wise decision makers of PHP Internals) force people to send strictly typed values to our methods. We can include the line declare(strict_type=1); in every file, but that won't stop the users from passing numeric strings where we expect integers. Since PHP 7 automatically casts scalar values to their expected scalar types, nothing in our own code will ever know that the user passes "0foo", as we'll only ever see 0.

Also, PHP 5.6 has extended support, and doesn't reach EOL until the last day of 2018, so we won't be able to add scalar type hinting until 2019, unless we decide to split the library to support only php 5.x and php 7.x in different versions. (In my opinion, splitting code bases to support different language versions is a Bad Thing... see also Python 2/3.)

I think that a good compromise that works in php 5.x is to use objects wherever possible, and provide interfaces that users can implement as our type checking mechanism, even for passing simple sets of scalar values.

For example, if we were getting values out of a spreadsheet, rather than

$value = $sheet->getValue('A', 1);
// getValue(string $column, int $row): mixed { ... }; 

We would have

$location = new SpreadsheetLocation('A', 1);
$value = $sheet->getValue($location);
// getValue(\MyNamespace\Interfaces\SpreadsheetLocation $location): mixed { ... }

More verbose, and not for the least experienced users, but it allows us to separate out the concerns for each class; in the example, instead of the sheet class having to worry about how users expect to represent the locations in a sheet, it can be enforced by an interface, and validated in classes that implement that interface. This allows us to add or change how locations are referenced; such as, if we want a full string, we could pass in 'A1' instead of 'A' and 1, and we could explicitly disallow relative locations by allowing '$A$1', or ranges by allowing 'A1:A2', etc., without modifying the sheet's getValue() method, or any of the other dozens of methods that will be using locations.

Of course, in this library, we need to balance keeping things simple for end users until they want to start getting their hands dirty. We can't enforce SoC, DI, unit testing, or the like in the user's code, but we can let them use it when they're ready to delve deeper into the library. With more SoC, we don't have to worry about validating scalar types, because users will be less able to send numeric strings instead of integers, and the few times when they can use scalar types, we'll be able to enforce things just as strictly as PHP 7's strict mode.

divinity76 commented 7 years ago

"should not, according to the inflated egos wise decision makers of PHP Internals) force people to send strictly typed values to our methods"

No, and by using strict_types, we don't. strict_types is per-file. call a function inside a file using strict types, from a file that dont use strict types, and it will be called using loose types.

call a function inside a file that is NOT using strict_types, from a file that IS using strict_types, it will still be called using strict types.

wether or not strict_types is enabled for the CALLER, is up to the CALLER. you don't have a say in that, no matter which mode your library use.