FAQ
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.

2. 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

Search Discussions

  • Alexey Zakhlestin at Dec 31, 2009 at 9:44 am

    On Thu, Dec 31, 2009 at 7:10 AM, Stanislav Malyshev wrote:
    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.

    2. 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?
    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
  • Richard Quadling at Dec 31, 2009 at 10:26 am

    2009/12/31 Alexey Zakhlestin <indeyets@gmail.com>:
    On Thu, Dec 31, 2009 at 7:10 AM, Stanislav Malyshev wrote:
    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.

    2. 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?
    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

    --
    PHP Internals - PHP Runtime Development Mailing List
    To unsubscribe, visit: http://www.php.net/unsub.php
    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
  • Stanislav Malyshev at Dec 31, 2009 at 10:34 am
    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
  • Richard Quadling at Dec 31, 2009 at 10:47 am

    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;
    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.
    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
  • Derick Rethans at Dec 31, 2009 at 12:20 pm

    On Thu, 31 Dec 2009, Stanislav Malyshev 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;
    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.
    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
  • Zeev Suraski at Dec 31, 2009 at 1:21 pm

    At 14:20 31/12/2009, Derick Rethans wrote:
    On Thu, 31 Dec 2009, Stanislav Malyshev 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;
    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.
    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.
    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
  • Johannes Schlüter at Dec 31, 2009 at 1:22 pm

    On Thu, 2009-12-31 at 12:20 +0000, Derick Rethans wrote:
    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.
    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
  • Philip Olson at Dec 31, 2009 at 6:02 pm

    On Dec 31, 2009, at 5:22 AM, Johannes Schlüter wrote:
    On Thu, 2009-12-31 at 12:20 +0000, Derick Rethans wrote:
    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.
    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)
    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
  • Stanislav Malyshev at Jan 1, 2010 at 2:35 am
    Hi!
    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)
    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

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupphp-internals @
categoriesphp
postedDec 31, '09 at 6:10a
activeJan 1, '10 at 2:35a
posts10
users7
websitephp.net

People

Translate

site design / logo © 2022 Grokbase