> On 21 Nov 2014, at 00:45, Adam Harvey <ahar...@php.net> wrote:
>
> My -1 is pretty much the same as Levi's:
>
> On 19 November 2014 13:57, Levi Morrison <le...@php.net> wrote:
>> - The RFC does not address how this is different from
>> FILTER_VALIDATE_* from ext/filter. I know there was a mention of this
>> on the mailing list, but the RFC should say why a tool that already
>> exists to solve this kind of problem is insufficient, especially when
>> it is enabled by default.
While that was true when Levi said it, the RFC *does* address this now.
>
> I'm also somewhat concerned that these functions are conflating two
> concerns (validation and conversion).
Ideally, usage of these functions should be accompanied by some sort of
validation logic if necessary for the application. They aren’t validation
functions in themselves, they just prevent certain conversions that don’t make
sense. Say someone fails to do proper validation and does this:
$user = User::get((int)$_GET[‘id’]);
With this code, absolutely anything could be passed for “id” in the URL
parameter and it’d be converted to a valid integer, transforming garbage into
apparently valid values.
On the other hand, say they did this:
$user = User::get(to_int($_GET[‘id’]));
If I pass something that is not a number (like “foobar”), it would throw an
exception.
That might not be the best example, I suppose, but it does show where it would
hopefully lead to less broken software. A better example might be conversion
from an integer to a float within an application. If (int) is used, the data
can be horribly mangled beyond recognition (overflow, INF/NAN handling) without
so much as a warning. If to_int() is used, then it will throw an exception in
this circumstance.
Basically, these functions aren’t meant to be substitutes for validation.
Instead, they prevent dangerous conversions. The hope is that people, when
needing to convert a value between types, will use these functions rather than
the more dangerous explicit casts, and their applications can failsafe if
something goes wrong.
>
>> - PHP suffers a lot from function bloat and this RFC provides
>> multiple functions that do the same thing but differ only in how they
>> handle errors. A simple validation of "can this be safely cast to an
>> integer without dataloss?" avoid the issue entirely and would be fewer
>> functions.
>
> +1: the idea of adding two sets of identical functions except for how
> they signal errors isn't one I like.
The problem is that it’s the least bad of the available options.
> Derick raised a good point elsethread: this is really tied into how we
> want to signal errors in PHP 7. If we switch to exceptions, then let's
> figure out a plan of attack and switch to exceptions everywhere, not
> just in the odd function here and there. If we don't, then the to_*
> functions shouldn't be added.
This isn’t a switch, though, these are new additions. We already have
exceptions in a few places in core, and there’s nothing wrong with new
additions using exceptions if and when appropriate. The question is over how to
deal with legacy code.
In this specific case, I suppose the other option would have been to throw an
E_RECOVERABLE_ERROR. But exceptions can be handled easier, and so I don’t see a
particular reason why it shouldn’t throw an exception.
--
Andrea Faulds
http://ajf.me/
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
