Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:120622
MIME-Version: 1.0
References: <CAH5C8xXtCVHaxwzmAW_eBxp_D6p7kww9svND=3AEeC9X6--YXg@mail.gmail.com>
 <eaaa805e-86b2-ce8c-5a24-c301260d16ac@php.net> <CAOWwgpn3PY=4DQ7s2PxjET7__fx09wYqQOJR76L2owa9PX9tFA@mail.gmail.com>
 <017db925-da75-8054-7a35-c65457cd03ae@php.net>
In-Reply-To: <017db925-da75-8054-7a35-c65457cd03ae@php.net>
Date: Mon, 19 Jun 2023 22:12:07 +0200
Message-ID: <CAOWwgpniYotForXo_UCdkriOTo-jz0HhdydLLNo18bBt3HKJqA@mail.gmail.com>
To: PHP Internals List <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000c9d6d605fe812675"
Subject: Re: [PHP-DEV] [RFC] [Discussion] Deprecate functions with overloaded signatures
From: nicolas.grekas+php@gmail.com (Nicolas Grekas)

--000000000000c9d6d605fe812675
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

> > > I'm going to nitpick on the newly suggested names and argument order
> for
> > > the
> > > DatePeriod factory methods =E2=80=94 althoughI do agree that they nee=
d to get
> > > created:
> > >
> > > createFromInterval(DateTimeInterface $start, DateInterval $interval,
> > > DateTimeInterface $end, int $options =3D 0)
> > > =E2=86=92 createWithRange(DateTimeInterface $begin, DateTimeInterface=
 $end,
> > > DateTimeInterface $int, int $options =3D 0)
> > >
> > > createFromRecurrences(DateTimeInterface $start, DateInterval $interva=
l,
> > > int $recurrences, int $options =3D 0)
> > > =E2=86=92 createWithRecurrences(DateInterval $begin, int $recurrences=
,
> > > DateInterval $interval, int $options =3D 0)
> > >
> > > We also should fix the argument names. Either $start/$finish, or
> > > $begin/$end. I
> > > prefer the latter.
> > >
> > > createFromIso8601(string $specification, int $options =3D 0)
> > > -> createFromISO8601String
> > >
> > > I am open to bike shedding about this :-)
> > >
> >
> > On my side, I'd very much prefer keeping the constructor of DatePeriod
> and
> > thus making it non-overloaded with this signature:
> >
> > public function __construct(DateTimeInterface $start, DateInterval
> > $interval, DateTimeInterface|int $end, int $options =3D 0) {}
>
> That still has an overloaded third argument =E2=80=94 DateTimeInterface|i=
nt.
>
Where you trying to suggest to change the current existing __construct()
> to:
>
> public function __construct(DateTimeInterface $start, DateInterval
> $interval, DateTimeInterface $end, int $options =3D 0)
>
> and then add these two new factory methods?
>
> createFromRecurrences(DateTimeInterface $start, DateInterval $interval,
> int $recurrences, int $options =3D 0)
>
> createFromISO8601String(string $specification, int $options =3D 0)
>

I really meant DateTimeInterface|int, not an overloaded signature but a
union type.

My concern is about providing the best path forward.
Both deprecating and providing the alternative in the same version creates
a lot of friction for end users.
After a quick chat with Mate, I think we should do this union type *in
addition* to adding the new static constructors.
Then, later on, if future us think it's worth it, we might want to consider
deprecating passing an integer.
This would provide a path for BC/FC I would fully support.

Nicolas

--000000000000c9d6d605fe812675--