get-random and -max

Topics: Developer Forum, User Forum
Apr 25, 2007 at 6:13 PM
I think there may be a bug in either get-random or its documention. The documentation reads, in part:

"-Max
The maximum value of the returned integer."

However, I noticed that attempting to simulate a die roll with:

get-random -min 1 -max 6

never seemed to produces any sixes. A quick test with:

for ($i = 0; $i > 1000; $i++)
{
$v = (get-random -min 1 -max 6)
if ($v -eq 6)
{
write-host "Six!"
}
}

Seems to confirm this. For myself, I'd much rather this was fixed in the cmdlet than in the documentation. I can supply a patch, if that'd be useful?
Coordinator
Apr 26, 2007 at 2:35 AM
This cmdlet is essentially just using the System.Random.Next(int min, int max) overload where max is the max "exclusive". Here's the docs on it:

maxValue
The exclusive upper bound of the random number returned. maxValue must be greater than or equal to minValue.

Perhaps not intuitive but there it is. Of course we could change the behavior of this cmdlet which is something I would entertain.
Apr 26, 2007 at 10:57 AM

rkeithhill wrote:
This cmdlet is essentially just using the System.Random.Next(int min, int max) overload where max is the max "exclusive". ,,, Perhaps not intuitive but there it is.

Well, that'd be fair enough - except it's not documented that way in the cmdlet help, since that's not the common meaning of "maximum", and I've got to figure a lot of non-developer scripters aren't going to have the MSDN docs handy.


Of course we could change the behavior of this cmdlet which is something I would entertain.

That's what I'd probably suggest, myself. It seems more intuitive to people scripting without a lot of references to the documentation, and goodness knows there's a lot of them out there for PowerShell to cater to.
Developer
Apr 27, 2007 at 1:52 PM
Perhaps adding an -Inclusive switchparameter would do the trick. Many PRNGs in various languages work using "exclusive" ranges by default., so I'd vote to leave it as is, but add the switch.
Apr 27, 2007 at 5:58 PM

oisin wrote:
Perhaps adding an -Inclusive switchparameter would do the trick. Many PRNGs in various languages work using "exclusive" ranges by default., so I'd vote to leave it as is, but add the switch.


I like the idea of a switch for this, but can also see that someone brand new to PSH, not knowing any better, would think -max means what you'd think it would... PSCX could possibly be held liable for someone not winning a lottery! ;-)

I got into this exact same issue lately during a PowerShell training session I gave.
Apr 27, 2007 at 7:22 PM
I like the idea of a switch for this, but can also see that someone brand new to PSH, not knowing any better, would think -max means what you'd think it would...

That's pretty much my thought - that, and that scripters who aren't developers, in my experience, work by trying things out first and referring to the documentation afterwards. And in this particular case, it's not obvious that -max doesn't mean its intuitive meaning unless you're bothered enough to actually run the trial.
Developer
Jun 3, 2007 at 2:43 AM
I think this cmdlet needs some refactoring anyway. I'd be willing to do it, but I don't want to step on any toes, so here's my thoughts first ...

My issues:
  1. If I call just get-random I get, quite logically, a random number between zero and 1 -- which is great, and consistent with just about every other language (incidentally, can we default alias this cmdlet to "rand"?).
  2. If I specify -Count, suddenly it switches over to returning "bytes" (aka: numbers between 0 and 255 (0xFF)). This parameter should be named -CountBytesArray (which would let you continue to just use -count) or just -BytesArray or -BytesArraySize ...
  3. Because -Count has Position = 0 specified, you always have to pass named parameters (or none) (ie: you can't just call get-random [-min] <integer> [-max] <integer>).
  4. It always creates a RNGCryptoServiceProvider instance, even though I've never, ever used it ;).

So, my feeling is, at least remove the Position from the count parameter, and rename it to CountBytesArray so it's clear what it actually does... that would enable get-random for a double between 0-1, and get-random 1 10 for ints within a range. To be honest, I question whether the ByteArray (and the cryptographic RNG) are actual use cases (from the command line or script) -- it almost seems those should be separate and/or integrated into cryptographic cmdlets for signing/encrypting/hashing/etc.

One use case which I see missing (and which I mentioned earlier) is picking a random item from a collection -- it's fairly trivial, and I think it's probably relatively common, so I'd actually like to add that as another parameter set (with the option to take the collection as a pipeline parameter). It does seem like it might make sense for this to be Select-Random, but "Select" isn't one of the official CLS verbs, so essentially, I'm thinking: Get-Random [[-min] <int>] [[-max] <int>] [-From <array>] That way, you can specify just the -From, or just the -min and -max, but if you specify all three, then the min and max should both be within the bounds of the array... make sense?

Now, as to the point of this thread, what if, instead of having a switch, have two named parameters for the max value ... -MaxExclusive and -InclusiveMax ... Get-Random [[-Min] <int>] [[-MaxExclusive] <int>] [-InclusiveMax <int>] [-From <array>] that way, if you want the current behavior, you can just use -min and -max (or even just put the numbers without the parameter names) if you want inclusive, you can specify -InclusiveMax.
Coordinator
Jun 5, 2007 at 5:42 AM
Sounds good. Just do this on the Trunk and not in the 1.1 release branch. We are just a few days away from releasing 1.1.1 and this is big enough that I don't want to try to slip this in.
Developer
Jun 6, 2007 at 2:05 AM
This discussion has been copied to a work item. Click here to go to the work item and continue the discussion.
Developer
Jun 6, 2007 at 2:06 AM
Edited Jun 6, 2007 at 2:22 AM
(whoops. This post deleted -- I have stubby fingers)