Better name for method Randomizer::nextFloat() View as Markdown

Hi

I think the major pet peeve i have is that the "Randomizer" class is
encapsulating all random functions.
I would like to see each randomizer have its own class, since they are
under the Random namespace
anyway, I believe they should be Random/StringRandomizer,
Random/IntRandomizer,
Random/FloatRandomizer, so on so forth. Basically Ramdom/{Type}Randomizer.
This would
also allow more granular and specific functionality for each type.

For example, for this discussion, the class, Random/FloatRandomizer (and
IntRandomizer)
could be instatiated with its boundries.

$randomFloat = new Random/FloatRandomizer($lowerBound, $upperBound,
$boundryInterval)
$a = $randomFloat->get();
$b = $randomFLoat->next();

Is that not more intuitive?

Creating a separate class for each possible distribution would certainly
be the cleanest API from an "academic" point of view. I'm not sure if it
would be more intuitive, but I'm sure that it would not be easier to
use. The API of Random\Randomizer is probably not perfect, but I believe
it does quite a few things quite right:

• It is secure by default, if you do not provide an engine, it defaults
  to the CSPRNG.
• It is discoverable, your IDE will autocomplete the available methods
  and all the methods are listed right beneath each other in the
  documentation.
• It is succinct for common cases and (mostly) does what it says on the tin.
• It allows you to easily build additional (userland) APIs on top of it,
  thus serving as a "building block" (to reuse the phrasing from the
  Randomizer additions RFC). Such a userland API could look like the API
  you proposed (though it would probably make sense to also include the
  "Uniform" somewhere within the name, because you could also have a
  "Normal" distribution, so we're right in bike-shedding territory, API
  design is hard).

To give an example:

 // Inspired by Rust's API.
 $engine = new \Random\Engine\Secure();
 $oneToHundred = new \Random\Distribution\UniformInt(1, 100);
 $randomInt = $oneToHundred($engine);
 $anotherRandomInt = $oneToHundred($engine);

vs

 // The default PHP API.
 $randomizer = new \Random\Randomizer();
 $randomInt = $randomizer->getInt(1, 100);
 $anotherRandomInt = $randomizer->getInt(1, 100);

I would certainly prefer PHP's API for most of my applications, because
it's much less boilerplate. It also avoids constructing many short-lived
objects [1]: You can just throw both the Engine and Randomizer into your
dependency injection container.

Best regards
Tim Düsterhus

[1] I expect the performance impact of constructing and destructing
objects to be greater than a regular function call, but might be wrong here.

Am 15.10.2023 um 19:24 schrieb Tim Düsterhus tim@bastelstu.be:

Making getFloat(float $min = 0.0, float $max = 1.0, IntervalBoundary $boundary = IntervalBoundary::ClosedOpen) would seemingly make it legal to call ->getFloat(0.5), which I consider to be worse than nextFloat().

While I understand that you find getFloat(0.5) undesirable I would not consider it illegal, e.g. getFloat(max:5).
Additionally I would consider getInt(max:100) to be a very valid usage too.

Side note: The implementation of nextFloat() is much more efficient than that of getFloat(0.0, 1.0) and the output will differ even for the same seeded engine. The domain of legal return values is identical.

This sounds like an implementation detail as I'm pretty sure you could switch to that fast path easily enough if neither of the arguments were given.

Side-note: The case of nextInt() is a bit trickier and strikes me as a bit of a weird API and I'd consider dropping it too: Having the max value depend on the Randomizer engine makes the generated values unstable in regards to switch the Engine. And if I read the documentation correctly then it only generates 32 bit values (or should it be 31 bits as it returns positive values only?). I think it would be clearer to require the Engine to provide at least 4 bytes and then specify the default max value of getInt() to be 2^31, optionally overridable with something up to PHP_INT_MAX.

So in summary I'd support both adding default values to getInt()/getFloat() as well as dropping nextFloat() in favor of getFloat() (and possible nextInt() with getInt()).

Regards,

• Chris