Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:105742
To: Kalle Sommer Nielsen <kalle@php.net>, Internals <internals@lists.php.net>
References: <CAJW__o0yRBpNTDXYV8Ns3SoyhYHOiBZXr8jPzZ+dBUXAt0Vk9Q@mail.gmail.com>
Message-ID: <1b95b6af-adc5-0f69-b277-9f7c6b30345f@gmx.de>
Date: Sat, 18 May 2019 12:00:04 +0200
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.6.1
MIME-Version: 1.0
In-Reply-To: <CAJW__o0yRBpNTDXYV8Ns3SoyhYHOiBZXr8jPzZ+dBUXAt0Vk9Q@mail.gmail.com>
Content-Type: text/plain; charset=utf-8
Content-Language: de-DE
Content-Transfer-Encoding: quoted-printable
Subject: Re: Changing the default value of SQLite3::enableExceptions()
From: cmbecker69@gmx.de ("Christoph M. Becker")

On 17.05.2019 at 21:42, Kalle Sommer Nielsen wrote:

> While reviewing a PR[1], which proposes to add two new methods to
> ext/sqlite3, one being an option to toggle on/off, I've noticed that
> there already is another method which also enables the extension to
> toggle on/off behavior, in this case; whether or not to throw
> exceptions instead of warnings.
>
> This method have a rather poor naming considering its default value.

ACK.

> Observe the current prototype:
> SQLite3::enableExceptions ([ bool $enableExceptions =3D FALSE ] ) : bool
>
> What this means is that code like this may not do what you actually
> think it does:
> $link->enableExceptions();
>
> Due to the $enableExceptions parameter being default to false. I'm
> writing this email to get some feedback on thoughts regarding changing
> this default value to true (current thinking is PHP8) or simply
> dropping the default value, forcing users to specify whether or not to
> enable exception mode.
>
> I personally think its better to simply deal with the default value
> rather than making a bigger potential BC break by renaming the method
> to something like: SQLite3::toggleExceptions()

ACK.

However, I don't think that we can change the default value in the near
future for BC reasons.  Deprecating (7.4) and removing support (8.0) for
calling the method without argument would be the way to go, I think.

[In a somewhat distant future, we might consider reintroducing support
for argument-less calls, but in this case it might make sense to let the
method work as getter (i.e. without argument, it would return the
current value without changing it).]
> [1] https://github.com/php/php-src/pull/4166

Thanks,
Christoph