# invalid params return value - Thread: https://externals.io/message/46578 - Original archive: https://news-web.php.net/php.internals/46578 - Started by Stanislav Malyshev on 2009-12-31T06:10:09+00:00 - Messages: 10 - Reply by email to internals@lists.php.net - More for agents: search with https://externals.io/search.md?q={query} · JSON API at https://externals.io/api · documentation at https://externals.io/llms.txt --- ## #46578 — Stanislav Malyshev — 2009-12-31T06:10:09+00:00 URL: https://externals.io/message/46578#46578 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 --- ## #46581 — Alexey Zakhlestin — 2009-12-31T09:44:34+00:00 URL: https://externals.io/message/46578#46581 · In reply to: #46578 > 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 --- ## #46583 — Richard Quadling — 2009-12-31T10:26:21+00:00 URL: https://externals.io/message/46578#46583 · In reply to: #46581 2009/12/31 Alexey Zakhlestin : > > > […] > > 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? ... ). 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 --- ## #46584 — Stanislav Malyshev — 2009-12-31T10:33:48+00:00 URL: https://externals.io/message/46578#46584 · In reply to: #46583 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 --- ## #46585 — Richard Quadling — 2009-12-31T10:46:56+00:00 URL: https://externals.io/message/46578#46585 · In reply to: #46584 2009/12/31 Stanislav Malyshev : > Hi! > > > […] > > 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 --- ## #46586 — Derick Rethans — 2009-12-31T12:20:44+00:00 URL: https://externals.io/message/46578#46586 · In reply to: #46584 > > […] > > 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 -- http://derickrethans.nl | http://ezcomponents.org | http://xdebug.org twitter: @derickr --- ## #46587 — Zeev Suraski — 2009-12-31T13:21:05+00:00 URL: https://externals.io/message/46578#46587 · In reply to: #46586 At 14:20 31/12/2009, Derick Rethans wrote: > > > > […] > function (i.e. > > […] > fopen without > > […] > cases, some do > > […] > >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 --- ## #46588 — johannes@php.net — 2009-12-31T13:22:08+00:00 URL: https://externals.io/message/46578#46588 · In reply to: #46586 > 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 --- ## #46589 — Philip Olson — 2009-12-31T18:01:56+00:00 URL: https://externals.io/message/46578#46589 · In reply to: #46588 > > > […] > > 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 --- ## #46591 — Stanislav Malyshev — 2010-01-01T02:35:40+00:00 URL: https://externals.io/message/46578#46591 · In reply to: #46588 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