On Wed, Jul 20, 2022, at 9:24 AM, Go Kudo wrote:
> Hi Internals.
>
> The Random Extension 5.x and Random Extension Improvement RFCs have been
> passed and recently merged into the master branch. However, the proposal is
> still problematic and implementation fixes are needed.
>
> Currently, the mt_rand() function is overloading its arguments, and I have
> designed a Randomizer::getInt() signature based on this. However, argument
> overloading has several problems, including reflection, and is now not
> recommended.
>
> I wish I had been aware of it during the RFC proposal, but that did not
> happen.
>
> To solve the problem, I thought of separating the argumentless
> `Randomizer::getInt()` as `Randomizer::nextInt()`. `Randomizer::getInt()`
> must require `int $min, int $max` arguments.
>
> I have received support for this proposal from several people, but there
> will be a discrepancy between the RFC and the implementation.
>
> I believe this should be corrected to avoid future BC Break, what do you
> think about this?
>
> Discussions:
> - https://externals.io/message/118163#118269
> - https://github.com/php/php-src/pull/8094/files#r919693108 (just scroll a
> little please)
>
> Fix PR: https://github.com/php/php-src/pull/9057
>
> Best Regards
> Go Kudo
This is ultimately for the branch maintainers to decide. I would personally favor fixing the signature now, even post-freeze; the whole point of betas is to find problems and edge cases while they're still semi-fixable, and this sounds like one.
Splitting it into getInt(int $min, int $max) and nextInt() would also be acceptable for me; if the branch maintainers are OK with it, I would favor that as methods doing double-duty is just all around bad news. If not, handling it like touch() is the next-best option.
--Larry Garfield
