On Fri, Apr 2, 2021, at 4:56 PM, Kamil Tekiela wrote:
> Hi Go Kudo,
>
> First, let me say that I believe we need such implementation in PHP and I
> would like to see object scoped RNG as part of the standard. However, I
> have voted no for a number of reasons. Let me list them from the
> perspective of a noob PHP user.
>
> - I really do not understand why we are introducing new functions. Can't
> the classes implement the necessary methods to get integers, doubles, and
> string of bytes? As a new user I would be completely overwhelmed by all
> these functions: rand(), mt_rand(), rng_int(), rng_next(), rng_next64().
> Which one should I use? What is the difference between rng_next()
> and rng_next64?
> - As soon as I left the RFC page I forgot what the new classes were called.
> I still can't tell from memory what's their name. I understand what they
> mean, but they are definitely not friendly names.
> - What's the difference between MT19937 and XorShift128Plus? They are
> different algorithms but which one should I pick? I tested the
> implementation locally and I see no difference in performance.
> - I am not a fan of adding a new optional parameter to shuffle() and
> friends. I'd prefer to have a method in the class that I can pass an array
> to.
> - What is the default seed? Do I have to provide a seed each time? Why
> can't the seed be done automatically?
> - Signed? Unsigned? As far as I know, PHP doesn't have unsigned integers.
> What's the real-life purpose of this flag?
> - I don't see any use in supporting userland implementations. Why can't
> they create separate libraries? I don't know about performance, but if
> someone wants to have custom RNG then I don't think they worry about
> performance.
> - When using the functions the performance was 50% worse than when calling
> ->next() directly. Is this right or is the implementation going to be
> optimized further? The fastest way to get a random number seems to be
> mt_rand() based on my tests.
>
> I would rather like to see a single class called RNG/Random that implements
> RNG/RandomInterface. The constructor of the class would take 2 arguments.
> The first is the algorithm with a default either MT or XORShift. The second
> is an optional seed. If no seed is provided then the seed is generated
> automatically like in mt_srand(). The class would then implement methods
> like: nextInt(), nextDouble(), nextBytes(), arrayShuffle(),
> stringShuffle(), randomArrayKeys(). I would keep the standard functions as
> they are. Let them use MT by default. We could even deprecate them in
> future if this takes off.
>
> This would make it painfully obvious what the class does and how to use it.
> No more procedural code. I would also make the class final so that you
> can't inherit from it, but that is highly opinion-based.
> Now that I have written this, I read previous conversations and it looks to
> me like what I would like is what you had previously.
>
> I'm sorry if I complain too much, but I would like to see something like
> this implemented, just not like you are proposing right now. It is too
> messy for me and I know I wouldn't like it if I had to use it.
>
> Regards,
> Kamil
I also didn't pay close attention to the previous discussion, but reading the RFC I agree with
all of this. The functionality proposed is good and needed, but the API is a mess. What Kamil
suggests is far better, and fully commits to being OOPy. Given that the use case is for situations
where you need a predictable and repeatable random sequence, such as Fibers or threads or such,
going all-in on an object seems like the correct approach.
One thing I'm not 100% clear on from the RFC, does this also deprecate
random_int()/random_bytes()? Those are (AFAIK) unseeded, so they seem like they'd continue to
serve their current purpose, but it's not clear from the RFC.
Voting no for now, but I would welcome a resubmission with a cleaner API, even in this cycle.
--Larry Garfield