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
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()
asRandomizer::nextInt()
.Randomizer::getInt()
must requireint $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