Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:118069
MIME-Version: 1.0
References: <CAMs36HcGzvwwTTU+nUO+U_M6o4qpOAHES7RbHEJWm6iot+-9=Q@mail.gmail.com>
 <CABTDVhCp6081B=+yGM2pAkTJXhA8=3f4W=J6Bo7axr9tkyjHAQ@mail.gmail.com> <2dde46be-bc0d-b0dd-90d9-1128757cfe82@bastelstu.be>
In-Reply-To: <2dde46be-bc0d-b0dd-90d9-1128757cfe82@bastelstu.be>
Date: Thu, 23 Jun 2022 20:18:41 +0900
Message-ID: <CAMs36Hdbj_0h=FtQwwBQk+J1a_znvMS8D59H=nCskogPaGdeAA@mail.gmail.com>
To: =?UTF-8?Q?Tim_D=C3=BCsterhus?= <tim@bastelstu.be>, internals@lists.php.net
Content-Type: multipart/alternative; boundary="0000000000005b01ed05e21b9e5c"
Subject: Re: [PHP-DEV] [RFC] [Under Discussion] Random Extension Improvement
From: g-kudo@colopl.co.jp (Go Kudo)

--0000000000005b01ed05e21b9e5c
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

2022=E5=B9=B46=E6=9C=8823=E6=97=A5(=E6=9C=A8) 0:02 Tim D=C3=BCsterhus <tim@=
bastelstu.be>:

> Hi
>
> On 6/22/22 16:35, Go Kudo wrote:
> > No additional comments seemed to be forthcoming, so the RFC was upgrade=
d
> to
> > 1.5.
> > The following changes have been made
> >
> > https://wiki.php.net/rfc/random_extension_improvement
> >
> > 1. Add: `Refine classnames`
> > 2. Add: `Random\SerializableEngine is outdated`
> > 3. Add `Add Randomizer::pickArrayKeys(array $array, int $num): array
> > method` *1
> > 4. Add `Random\SerializableEngine is outdated`
> > 5. Remove: `PCG64 is ambiguous` (replaced by 1)
> > 6. Remove: `Mersenne Twister is ambiguous` (replaced by 1)
> > 7. Remove: `Randomizer lacks array_rand() replacement method` (replaced
> by
> > 3)
> >
> > *1: Added with a little sample code.
>
> Thank you for the update. The grouping makes sense to me and it looks
> very organized.
>
> Let me just propose some wording changes:
>
> a)
>
>  > Random\SerializableEngine is outdated
>
> I would rename the headline to "Random\SerializableEngine is not
> useful", that's a little more fitting.
>
> b)
>
>  > CombinedLCG is outdated
>
> I would rename the headline to "Random\Engine\CombinedLCG is low
> quality", that's a little more accurate.
>
> c)
>
> In the "Refine classnames" section:
>
>  >  To make it more readable and regular, the class name is changed as
> follows:
>
> I would reword this as:
>
> To clearly identify the implemented algorithm the PCG64 and
> MersenneTwister twister engines should be renamed to their canonical
> upstream name:
>
> The issue with the previous wording is it's not clear what "more
> regular" means.
>
> d)
>
> For the vote titles I propose the following changes for a more
> consistent wording that succinctly describes the change to avoid voter
> confusion:
>
> Engine implementations to final
>    to
> Make all implemented engines 'final'?
>
> Remove Random\SerializableEngine
>    to
> Remove the SerializableEngine interface?
>
> Drop Random\Engine\CombinedLCG
>    to
> Remove the CombinedLCG engine?
>
> Add Random\Randomizer::pickArrayKeys(array $array, int $num): array
>    to
> Add the pickArrayKeys() method to the Randomizer?
>
> Rename Random\Randomizer::shuffleString() to
> Random\Randomizer::shuffleBytes()
>    to
> Rename Randomizer::shuffleString() to Randomizer::shuffleBytes()?
>
> Change classnames
>    to
> Rename PCG64 and MersenneTwister?
>
> Implement Random\Engine\Xoshiro256StarStar
>    to
> Add the Xoshiro256StarStar engine?
>
> Best regards
> Tim D=C3=BCsterhus
>

Hi Tim

Thanks for the suggestion! It looked much better to me and I have reflected
it in the RFC.

"Remove the CombinedLCG engine?" replaced by "Remove the CombinedLCG
class?". The reason is that the implementation will still remain for
backward compatibility. (only the class will be removed).

Regards
Go Kudo

--0000000000005b01ed05e21b9e5c--