On Fri, Jun 28, 2013 at 9:41 PM, Yasuo Ohgaki <yohg...@ohgaki.net> wrote:
> Hi Sherif,
>
> The problem is that current code 'returns false for invalid hex' while it
> 'returns false and E_WARNING for invalid length'.
>
> We may choose behavior for invalid inputs
> - return false always
> - return false and raise E_WARNING always
>
> Mixing them up is not good, especially in the same function. IMHO. PHP has
> enough inconsistency. We should try to remove inconsistency much as
> possible. Besides the change will not break code as long as it is used as
> it should be.
>
>
If we tried to fix all of PHP's functions to either return false always or
return false and raise errors always we'd have to fix every single PHP
function there is since there's absolutely no consistency there as it is.
Different functions handle failure differently, and not all functions
report every possible error they may encounter. Take functions like
file_get_contents, for example, where it wraps other API functions that
could RETURN_FAILURE and/or trigger the error handler, and
file_get_contents isn't even documented to raise those errors in some cases.
I see these as two separate problems. The first problem is clearly a design
problem (but we can't really do much about that in the short term). The
second is a documentation problem. Beyond that there are users that don't
clearly understand the behavior of some functions due to the previous two
reasons.
> Considering the use case of hex2bin(), it will not handle external inputs
> in most cases. Therefore, it's better to raise errors invalid inputs. Do
> you prefer not to raise errors?
>
Considering the fact that we have a discussion in the bug report mentioned
earlier (and it was an extensive discussion because not only were our users
confused but even our core developers were confused if you read the
responses in that bug report), I would prefer that we leave the function
the way it is.
The error was added for a very specific reason. People thought it was doing
what pack('h*', $hex) does, and clearly it wasn't. So the documentation was
also updated to make note of this and explain it to the user, along with
the helpful error message to indicate that hex2bin('f') would not work. I
don't think there was ever any expectation for the user to do something
like hex2bin('Hello World') and expect any kind of sensible result. The
error really does not inform them of anything useful.
</my $0.000002>
>
> Regards,
>
> --
> Yasuo Ohgaki
> yohg...@ohgaki.net
>
>
>
