Re: [RFC] [Under Discussion] Random Extension Improvement

Hi On 6/22/22 16:35, Go Kudo wrote:

No additional comments seemed to be forthcoming, so the RFC was upgraded 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üsterhus