Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120450
Message-ID: <71F586BF-E5AD-4FD2-B680-6E0EDCB4733D@gmail.com>
Content-Type: multipart/alternative;
	boundary="Apple-Mail=_81CEA6E1-C737-4127-A341-4F760F2DBC31"
Mime-Version: 1.0 (Mac OS X Mail 16.0 \(3731.600.7\))
Date: Tue, 30 May 2023 10:54:14 +0200
In-Reply-To: <CAH5C8xUeMM-vKCKOP9C27zF=OK4pZdY3RGYTWUW7dWA23pS02g@mail.gmail.com>
Cc: PHP Internals List <internals@lists.php.net>
To: =?utf-8?B?TcOhdMOpIEtvY3Npcw==?= <kocsismate90@gmail.com>
References: <CAH5C8xXtCVHaxwzmAW_eBxp_D6p7kww9svND=3AEeC9X6--YXg@mail.gmail.com>
 <5798AD17-3FA6-48E5-83E9-B5773BE94598@gmail.com>
 <CAH5C8xUeMM-vKCKOP9C27zF=OK4pZdY3RGYTWUW7dWA23pS02g@mail.gmail.com>
Subject: Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded
 signatures
From: claude.pache@gmail.com (Claude Pache)

--Apple-Mail=_81CEA6E1-C737-4127-A341-4F760F2DBC31
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
	charset=utf-8

Hi M=C3=A1t=C3=A9,

> Le 30 mai 2023 =C3=A0 01:41, M=C3=A1t=C3=A9 Kocsis =
<kocsismate90@gmail.com> a =C3=A9crit :
>=20
> Hi Claude,
>=20
>> The replacement methods for IntlCalendar::set() (namely =
IntlCalendar::setDate() and IntlCalendar::setDateTime()) must not have a =
return type of `void`, but `true`, like the original method, for the two =
following reasons:
>>=20
>> 1. By changing the returned value, you are introducing an unnecessary =
hazard when migrating code.
>>=20
>> 2. Per https://www.php.net/IntlCalendar, all modification methods of =
that class (clear(), roll(), setTime(), etc.) consistently return `true` =
on success and `false` on failure, even when the method is infallible =
(and thus would always return `true`). Don=E2=80=99t break consistency.
>=20
> 1.  I don't see much risk with this change, since we clearly indicate =
at the very beginning that the new methods return void, so programmers =
migrating their code
> should pay attention to the return types. IDEs and static analysers =
can surely warn them in case of inattention.

The very risk is that the programmer must be aware that the return =
convention has changed. Recall that not everyone run static analysers, =
especially for such a trivial migration, and that not every code is =
statically analysable (e.g.. `$foo->$bar()`).

Of course, the benefice of the change might be worth the risk; but that =
leads to the second point:

>=20
> 2. Some of the methods you mentioned have a "true" return type for a =
few weeks now. The biggest motivation for introducing these was to at =
least have some chance to
> make them void in the future. Doing so is a risky move indeed, so =
should be carried out very carefully, if it's possible at all... That's =
why I consider it beneficial to spare the
> effort and start with a clean slate by adding the new methods in =
question with their correct return type.

Concretely, if you change the return value (from `true` to `void/null`), =
you will break the following pattern:

<?php
if (!@$foo->setBar(...)) {
    // error handling (might be dead code, it doesn=E2=80=99t matter)
}
?>

Any change that introduce a BC break or a migration burden must be =
justified. (There has been enough rant around justified BC breaks, so =
let=E2=80=99s not introduce unjustified ones.)

What is the purpose of changing the return convention of IntlCalendar =
methods, and is it worth?

=E2=80=94Claude


--Apple-Mail=_81CEA6E1-C737-4127-A341-4F760F2DBC31--