Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105627
MIME-Version: 1.0
References: <CAOkcCt-3OEKmsQVam=t-XSs7R-bNTDwLrLMT45QDauh_=nBCJA@mail.gmail.com>
In-Reply-To: <CAOkcCt-3OEKmsQVam=t-XSs7R-bNTDwLrLMT45QDauh_=nBCJA@mail.gmail.com>
Date: Tue, 7 May 2019 10:52:24 -0400
Message-ID: <CAPKYkKx3j1NLW09A2RLt6XYkATG_RmJG5A8fD3xUy41LYf0RvA@mail.gmail.com>
To: Gert <gertp93@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000002613b805884d5ef3"
Subject: Re: [PHP-DEV] Error instead of returning false
From: chasepeeler@gmail.com (Chase Peeler)

--0000000000002613b805884d5ef3
Content-Type: text/plain; charset="UTF-8"

On Tue, May 7, 2019 at 10:22 AM Gert <gertp93@gmail.com> wrote:

> Hello internals,
>
> I wanted to propose an RFC, but i'd like to check first if this idea
> has been considered before already.
>
> My idea, extremely summarized, would be to take the functions that
> return false/null when they 'error', and instead make them actually
> throw an error.
>
> Currently, functions like getcwd or json_decode will return false/null
> when an error occurs, and they populate a global error state, which
> can be retrieved by error_get_last, or similar functions. json_decode
> already got the JSON_THROW_ON_ERROR option in PHP 7.3, and this RFC
> would basically make that behaviour default. This RFC would target PHP
> 8.0, but i'd like to hear some opinions on this.
>
> This would be a pretty big BC break for those functions. That's not
necessarily a bad thing, but, I think the burden for justification is
larger than just adding a new feature. These are the questions I'd ask

1.) What is detrimental about the current behavior?
2.) What is beneficial about the new behavior?
3.) Are the items for #1 and/or #2 big enough to justify breaking existing
code?
4.) Is there a way to achieve the desired behavior without a BC break.

My initial thoughts are that impacts of the BC break would be pretty big.
EVERYONE using json_decode without JSON_THROW_ON_ERROR will have to modify
their code. It's not a quick find/replace change either. Instead of
checking the return, and then checking the error state (or, maybe they are
OK with just continuing if there is an error), now has to wrap the call
into a try/catch block.

I personally don't see anything that detrimental about the current behavior
or that beneficial about the new behavior. It's not like try/catch is that
much simpler from an implementation standpoint that checking a global error
state based on the return value:
$foo = json_decode($bar);
if(false === $foo){
  //get error and do whatever to handle it
}
vs
try {
  $foo = json_decode($bar);
} catch (\Throwable $t){
  //do whatver to handle it
}

It also appears we can easily provide the functionality you desire with
additional options. You mentioned that json_decode has already been given a
new option, JSON_THROW_ON_ERROR. What is the advantage to making this the
default behavior vs letting people specify the flag when they want? If
there a reason adding such an option as a new parameter wouldn't work for
other methods, like getcwd?



> Greetings,
> Gert de Pagter (BackEndTea)
>
> --
> PHP Internals - PHP Runtime Development Mailing List
> To unsubscribe, visit: http://www.php.net/unsub.php
>
>

-- 
Chase Peeler
chasepeeler@gmail.com

--0000000000002613b805884d5ef3--