Hi!
One of the many things that is chaotic in PHP is what internal function
returns when invalid parameters are given (i.e. params parsing fails).
Most of those functions do one of:
1a. just return - this happens with most standard ext code, which was
converted from old params parsing to a new one.
1b. RETURN_NULL() - this is effectively the same as 1a, but the code is
different.
- RETURN_FALSE - some random set of functions does that, e.g. some of
PDO functions (PDO::prepare, PDO::setAttribute, etc.). NB: I'm not
singling out PDO here, it happens all over the code, it's just first
thing that came to my mind.
So, both of the values are kind of OK and both have the same flaw -
function could legitimately return both NULL
and false. I don't have
good argument for either of these as opposed to another one. However, I
think we should have ONE standard return value in this case - even
better, some macro like RETURN_ARGS_FAIL() (if you have a better name, ok).
So, what do you think of that?
Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com
Hi!
One of the many things that is chaotic in PHP is what internal function
returns when invalid parameters are given (i.e. params parsing fails). Most
of those functions do one of:1a. just return - this happens with most standard ext code, which was
converted from old params parsing to a new one.
1b. RETURN_NULL() - this is effectively the same as 1a, but the code is
different.
- RETURN_FALSE - some random set of functions does that, e.g. some of PDO
functions (PDO::prepare, PDO::setAttribute, etc.). NB: I'm not singling out
PDO here, it happens all over the code, it's just first thing that came to
my mind.So, both of the values are kind of OK and both have the same flaw - function
could legitimately return bothNULL
and false. I don't have good argument
for either of these as opposed to another one. However, I think we should
have ONE standard return value in this case - even better, some macro like
RETURN_ARGS_FAIL() (if you have a better name, ok).So, what do you think of that?
standard macro is a good idea. I also think, that NULL
makes more
sense in this case.
NULL
kinda means "nothing was returned", which should be the case
exactly (as function didn't start to work, actually)
--
Alexey Zakhlestin
http://www.milkfarmsoft.com/
Sent from Prague, Czech Republic
2009/12/31 Alexey Zakhlestin indeyets@gmail.com:
Hi!
One of the many things that is chaotic in PHP is what internal function
returns when invalid parameters are given (i.e. params parsing fails). Most
of those functions do one of:1a. just return - this happens with most standard ext code, which was
converted from old params parsing to a new one.
1b. RETURN_NULL() - this is effectively the same as 1a, but the code is
different.
- RETURN_FALSE - some random set of functions does that, e.g. some of PDO
functions (PDO::prepare, PDO::setAttribute, etc.). NB: I'm not singling out
PDO here, it happens all over the code, it's just first thing that came to
my mind.So, both of the values are kind of OK and both have the same flaw - function
could legitimately return bothNULL
and false. I don't have good argument
for either of these as opposed to another one. However, I think we should
have ONE standard return value in this case - even better, some macro like
RETURN_ARGS_FAIL() (if you have a better name, ok).So, what do you think of that?
standard macro is a good idea. I also think, that
NULL
makes more
sense in this case.
NULL
kinda means "nothing was returned", which should be the case
exactly (as function didn't start to work, actually)--
Alexey Zakhlestin
http://www.milkfarmsoft.com/
Sent from Prague, Czech Republic--
Would changing the returning value to Null to indicate invalid
arguments (or any other error condition) would create a significant BC
surely? Just how many functions are documented with the entity
&return.falseforfailure;? (198 files using this so far vs Return
&null; which is used very rarely).
Differentiating between False/Null (something was wrong) and
False/Null (the answer to the question you asked is False or Null) is
pretty impossible without constantly checking the manual to see what
the return values for failure is.
The use of error_get_last()
would be useful here. I call a function.
Something is wrong with my call in some way. I expect Null as a return
and have error_get_last()
available to explain to me what went wrong.
Obviously, the userland code needs to be more defensive in handling
these errors. Having a consistent response to ANY call to indicate an
error would be extremely useful. The use of set_error_handler()
is
useful also here, but an inline mechanism may be easier to read.
Of course, the question I would next ask (and, oh look, am doing so)
is could an exception be thrown? Invalid arguments is a pretty
exceptional circumstance. I'd almost say that in the normal course of
things, you wouldn't rely on the return value of a function when
invalid arguments have been supplied and you wouldn't expect the code
to continue. This feels like a true exception. (Hey whadda ya say we
talk about an ini option to allow exceptions to be thrown from core
for specific errors? ... Nah? ... <grin>).
Regards,
Richard.
--
Richard Quadling
"Standing on the shoulders of some very clever giants!"
EE : http://www.experts-exchange.com/M_248814.html
Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731
ZOPA : http://uk.zopa.com/member/RQuadling
Hi!
Would changing the returning value to Null to indicate invalid
arguments (or any other error condition) would create a significant BC
surely? Just how many functions are documented with the entity
&return.falseforfailure;? (198 files using this so far vs Return
&null; which is used very rarely).
There are different kinds of failure. There's failure for the function
(i.e. function tried to do something and failed, like couldn't open the
file) and failure where function wheren't even executed (like you called
fopen without giving it a filename). Some functions return the same in
both cases, some do not. I'm sure half of the functions documented as
&return.falseforfailure; return NULL
on incorrect args.
As for BC - it may break some scripts that don't check properly but
having each function do different thing is worse, since you couldn't
even write a proper check - since you have no way of knowing what to
check for.
Differentiating between False/Null (something was wrong) and
False/Null (the answer to the question you asked is False or Null) is
pretty impossible without constantly checking the manual to see what
the return values for failure is.
It's impossible with or without the change, it's entirely different
question.
Of course, the question I would next ask (and, oh look, am doing so)
is could an exception be thrown? Invalid arguments is a pretty
No, exception is not a good thing (that's why I didn't mention it) since
a lot of code isn't organized in a way that exceptions can be properly
handled, and using exceptions in this case would pretty much lead to
using exceptions for any error handling which I don't think PHP
community agrees to, and also entirely different topic.
Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com
2009/12/31 Stanislav Malyshev stas@zend.com:
Hi!
Would changing the returning value to Null to indicate invalid
arguments (or any other error condition) would create a significant BC
surely? Just how many functions are documented with the entity
&return.falseforfailure;? (198 files using this so far vs Return
&null; which is used very rarely).There are different kinds of failure. There's failure for the function (i.e.
function tried to do something and failed, like couldn't open the file) and
failure where function wheren't even executed (like you called fopen without
giving it a filename). Some functions return the same in both cases, some do
not. I'm sure half of the functions documented as &return.falseforfailure;
returnNULL
on incorrect args.As for BC - it may break some scripts that don't check properly but having
each function do different thing is worse, since you couldn't even write a
proper check - since you have no way of knowing what to check for.
Aha. I see, so, a big +1 for consistency for argument checking.
--
Richard Quadling
"Standing on the shoulders of some very clever giants!"
EE : http://www.experts-exchange.com/M_248814.html
Zend Certified Engineer : http://zend.com/zce.php?c=ZEND002498&r=213474731
ZOPA : http://uk.zopa.com/member/RQuadling
Would changing the returning value to Null to indicate invalid
arguments (or any other error condition) would create a significant BC
surely? Just how many functions are documented with the entity
&return.falseforfailure;? (198 files using this so far vs Return
&null; which is used very rarely).There are different kinds of failure. There's failure for the function (i.e.
function tried to do something and failed, like couldn't open the file) and
failure where function wheren't even executed (like you called fopen without
giving it a filename). Some functions return the same in both cases, some do
not. I'm sure half of the functions documented as &return.falseforfailure;
returnNULL
on incorrect args.As for BC - it may break some scripts that don't check properly but having
each function do different thing is worse, since you couldn't even write a
proper check - since you have no way of knowing what to check for.
Hmm, I don't actually think many people test whether parsing parameters
failed, but they do test the return value. AFAIK, our "standard" is to
return NULL
for parameter parsing failures (at least that's what all the
examples in README.PARAMETER_PARSING_API do). I'd like to see that be
the case for all things, but changing it now could be quite a bit of a
BC break.
regards,
Derick
--
http://derickrethans.nl | http://ezcomponents.org | http://xdebug.org
twitter: @derickr
At 14:20 31/12/2009, Derick Rethans wrote:
Would changing the returning value to Null to indicate invalid
arguments (or any other error condition) would create a significant BC
surely? Just how many functions are documented with the entity
&return.falseforfailure;? (198 files using this so far vs Return
&null; which is used very rarely).There are different kinds of failure. There's failure for the
function (i.e.
function tried to do something and failed, like couldn't open the file) and
failure where function wheren't even executed (like you called
fopen without
giving it a filename). Some functions return the same in both
cases, some do
not. I'm sure half of the functions documented as &return.falseforfailure;
returnNULL
on incorrect args.As for BC - it may break some scripts that don't check properly but having
each function do different thing is worse, since you couldn't even write a
proper check - since you have no way of knowing what to check for.Hmm, I don't actually think many people test whether parsing parameters
failed, but they do test the return value. AFAIK, our "standard" is to
returnNULL
for parameter parsing failures (at least that's what all the
examples in README.PARAMETER_PARSING_API do). I'd like to see that be
the case for all things, but changing it now could be quite a bit of a
BC break.
I agree. The real problem is we'd be breaking applications that are
in fact checking the return values very properly - as properly as
they can according to the documentation. Many functions can return
values that are evaluated as boolean false, well as false/null to
indicate error; That results in === being used to check the return
value, and consequently changing from false to null or vice versa
would break the code. Some pedantic users probably use === even if
the function can't otherwise return a value that evaluates as boolean false.
Zeev
Hmm, I don't actually think many people test whether parsing parameters
failed, but they do test the return value. AFAIK, our "standard" is to
returnNULL
for parameter parsing failures (at least that's what all the
examples in README.PARAMETER_PARSING_API do). I'd like to see that be
the case for all things, but changing it now could be quite a bit of a
BC break.
This is also the documented as the general convention:
http://de.php.net/manual/en/functions.internal.php
While there are exceptions to this on purpose: For instance get_object
is documented to "Return FALSE
if /object/ is not an object" (this
exception was discussed in detail in the past)
johannes
Hmm, I don't actually think many people test whether parsing parameters
failed, but they do test the return value. AFAIK, our "standard" is to
returnNULL
for parameter parsing failures (at least that's what all the
examples in README.PARAMETER_PARSING_API do). I'd like to see that be
the case for all things, but changing it now could be quite a bit of a
BC break.This is also the documented as the general convention:
http://de.php.net/manual/en/functions.internal.phpWhile there are exceptions to this on purpose: For instance get_object
is documented to "ReturnFALSE
if /object/ is not an object" (this
exception was discussed in detail in the past)
Ideally the documentation would be explicit, as currently it says:
"In this case it will likely return NULL
but this is just a convention, and cannot be relied upon."
Knowing exactly which do (and do not) follow this rule would be great.
Regards,
Philip
Hi!
This is also the documented as the general convention:
http://de.php.net/manual/en/functions.internal.phpWhile there are exceptions to this on purpose: For instance get_object
is documented to "ReturnFALSE
if /object/ is not an object" (this
exception was discussed in detail in the past)
I would argue for is_object getting argument which is not an object is
not an args parsing error (since this function is supposed to accept
args of all types). Getting no argument, on the other hand, is. Though
is_object would return false in both cases. But is_numeric()
, for
example, would produce null when called with no arguments. It's a real
mess there.
Thanks however for bringing up that manual part - I didn't know it's
there. That makes me think case for NULL
is much stronger.
Stanislav Malyshev, Zend Software Architect
stas@zend.com http://www.zend.com/
(408)253-8829 MSN: stas@zend.com