Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:114567
MIME-Version: 1.0
References: <CABTDVhDRH8+0WG0nJ5QRYVT9-8m1u2VMe-gvY5cWra-OKQ4t+g@mail.gmail.com>
 <CAC5V6gi2e2UFWvC0x9t4VcR7H985q3mc3R0ryTwiYRvAbJAeCA@mail.gmail.com>
In-Reply-To: <CAC5V6gi2e2UFWvC0x9t4VcR7H985q3mc3R0ryTwiYRvAbJAeCA@mail.gmail.com>
Date: Mon, 24 May 2021 16:16:44 +0100
Message-ID: <CAGBsUrdteKe+o=83aQpSKq7jr68whDSFCZ9i8rTg5n4vtihHYA@mail.gmail.com>
To: Guilliam Xavier <guilliam.xavier@gmail.com>
Cc: Go Kudo <zeriyoshi@gmail.com>, PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="00000000000059aa0505c314e6aa"
Subject: Re: [PHP-DEV] Re: [RFC] [Draft] Add Randomizer class (before: Add RNG extension)
From: tekiela246@gmail.com (Kamil Tekiela)

--00000000000059aa0505c314e6aa
Content-Type: text/plain; charset="UTF-8"

Hi Go Kudo,

This proposal looks much better. Well done.
What would help is if you showed the examples from the introduction section
rewritten using the new API.
I don't think it makes sense to make parameters (?int $min = PHP_INT_MIN,
?int $max = PHP_INT_MAX) nullable.
Regarding the method names, I would be afraid of naming them using PHP
keywords. Maybe they should be called getInt and getBytes?
Why isn't the new class in a new extension? Is there a reason why it should
go into core?
Regarding the name, I think Random or Randomizer is fine, but I would avoid
acronyms.

Regards,
Kamil

--00000000000059aa0505c314e6aa--