Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:115936
MIME-Version: 1.0
References: <CABTDVhBRpD=nWBk2xGLEmdcWwOzCeqGeqH5vLSugPFOeEN9Mog@mail.gmail.com>
In-Reply-To: <CABTDVhBRpD=nWBk2xGLEmdcWwOzCeqGeqH5vLSugPFOeEN9Mog@mail.gmail.com>
Date: Fri, 3 Sep 2021 18:02:23 +0200
Message-ID: <CAF+90c-kLHe-M4jcMRa=_8Lhakcyrtam109WKga6XLQqmJYp0Q@mail.gmail.com>
To: Go Kudo <zeriyoshi@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="000000000000b8e5a705cb196d43"
Subject: Re: [PHP-DEV] [RFC] Random Extension 3.0
From: nikita.ppv@gmail.com (Nikita Popov)

--000000000000b8e5a705cb196d43
Content-Type: text/plain; charset="UTF-8"

On Thu, Sep 2, 2021 at 5:10 PM Go Kudo <zeriyoshi@gmail.com> wrote:

> Hi Internals.
>
> Expanded from the previous RFC and changed it to an RFC that organizes the
> whole PHP random number generator. Also, the target version has been
> changed to 8.2.
>
> https://wiki.php.net/rfc/rng_extension
> https://github.com/php/php-src/pull/7453
>
> Hopefully you will get some good responses.
>

This looks pretty nice to me. A few bits of feedback:

 * Unlike the previous versions of the RFC, this one also moves all of the
existing RNG-related functionality from ext/standard to ext/random. Why
does it do this? This doesn't really seem related to the problem this RFC
is solving.

 * Regarding the abstract class Random\NumberGenerator: You currently need
the abstract class, because php_random_ng is tied to the object. An
alternative design that would allow Random\NumberGenerator to be an
interface would be to make it independent of the object, such that the
structure can be allocated in the Random constructor for userland
implementations. Of course, internal implementations could still embed it
as part of their objects -- just don't make it a requirement.

 * The future scope mentions: "Changes random source to php_random_int() a
shuffle(), str_shuffle(), and array_rand()". I don't think it makes sense
to switch these functions to use cryptographic randomness by default...

Regards,
Nikita

--000000000000b8e5a705cb196d43--