On Sun, Dec 20, 2020 at 6:45 AM zeriyoshi <zeriyo...@gmail.com> wrote:
> Thanks cmb. I have created a first draft of an RFC. I think I've covered > all the necessary requirements, but I'd like to know if there are any > problems. > https://wiki.php.net/rfc/object_scope_prng A few IMHO comments: * Namespaces are controversial. There were multiple discussions and RFCs about introducing them for new functionality but nothing came out of it. Still probably worth adding an option for \PHP\PRNG to the vote, or something like that. * The "Which set of PRNGs should be provided as standard?" vote seems vague to me. Are you proposing to add only one algorithm? Why? Also, the adding just interfaces option makes no sense because there's Composer for things that don't require a C implementation. * "One possible solution is to implement the PRNG in pure PHP. There is actually a userland library [1], but it is not fast enough for PHP at the moment. (However, this may be improved by JIT)" - this definitely needs a comparison because implementation in C is only needed for performance reasons. * Method names: you don't need to emulate the conventions from non-OOP code, e.g. string_shuffle() --> strShuffle and array_rand() --> arrayRand(). Use a more naturally readable convention like shuffleString(). * The return value in "function shuffle(array &$array): bool" means that if something goes wrong, callers will have no idea what it is. Just throw an exception. * If the classes are namespaced, don't prefix their names, e.g. PRNGInterface --> RandomGenerator (yay, so many opportunities for bikeshedding!) * Perhaps it's worth mentioning other languages using this pattern, e.g. Java[1] and C#[2]. * Do you have evidence of widespread demand for this functionality? Usage statistics for the userland implementation you mentioned[3] don't look that hot. ------ [1] https://docs.oracle.com/javase/8/docs/api/java/util/Random.html [2] https://docs.microsoft.com/en-us/dotnet/api/system.random?view=net-5.0 [3] https://packagist.org/packages/savvot/random
