Clean up Get-Random cmdlet (#9133)

This commit is contained in:
pougetat 2019-03-16 09:54:35 +01:00 committed by Ilya
parent 4253127a56
commit 5277c79d67
2 changed files with 101 additions and 104 deletions

View file

@ -44,7 +44,7 @@ namespace Microsoft.PowerShell.Commands
// cache MyParameterSet enum instead of doing string comparison every time // cache MyParameterSet enum instead of doing string comparison every time
if (_effectiveParameterSet == MyParameterSet.Unknown) if (_effectiveParameterSet == MyParameterSet.Unknown)
{ {
if ((this.MyInvocation.ExpectingInput) && (this.Maximum == null) && (this.Minimum == null)) if ((MyInvocation.ExpectingInput) && (Maximum == null) && (Minimum == null))
{ {
_effectiveParameterSet = MyParameterSet.RandomListItem; _effectiveParameterSet = MyParameterSet.RandomListItem;
} }
@ -52,11 +52,11 @@ namespace Microsoft.PowerShell.Commands
{ {
_effectiveParameterSet = MyParameterSet.RandomListItem; _effectiveParameterSet = MyParameterSet.RandomListItem;
} }
else if (this.ParameterSetName.Equals(GetRandomCommand.RandomNumberParameterSet, StringComparison.OrdinalIgnoreCase)) else if (ParameterSetName.Equals(GetRandomCommand.RandomNumberParameterSet, StringComparison.OrdinalIgnoreCase))
{ {
if ((this.Maximum != null) && (this.Maximum.GetType().IsArray)) if ((Maximum != null) && (Maximum.GetType().IsArray))
{ {
this.InputObject = (object[])this.Maximum; InputObject = (object[])Maximum;
_effectiveParameterSet = MyParameterSet.RandomListItem; _effectiveParameterSet = MyParameterSet.RandomListItem;
} }
else else
@ -78,26 +78,26 @@ namespace Microsoft.PowerShell.Commands
#region Error handling #region Error handling
private void ThrowMinGreaterThanOrEqualMax(object min, object max) private void ThrowMinGreaterThanOrEqualMax(object minValue, object maxValue)
{ {
if (min == null) if (minValue == null)
{ {
throw PSTraceSource.NewArgumentNullException("min"); throw PSTraceSource.NewArgumentNullException("min");
} }
if (max == null) if (maxValue == null)
{ {
throw PSTraceSource.NewArgumentNullException("max"); throw PSTraceSource.NewArgumentNullException("max");
} }
ErrorRecord errorRecord = new ErrorRecord( ErrorRecord errorRecord = new ErrorRecord(
new ArgumentException(string.Format( new ArgumentException(string.Format(
CultureInfo.InvariantCulture, GetRandomCommandStrings.MinGreaterThanOrEqualMax, min, max)), CultureInfo.InvariantCulture, GetRandomCommandStrings.MinGreaterThanOrEqualMax, minValue, maxValue)),
"MinGreaterThanOrEqualMax", "MinGreaterThanOrEqualMax",
ErrorCategory.InvalidArgument, ErrorCategory.InvalidArgument,
null); null);
this.ThrowTerminatingError(errorRecord); ThrowTerminatingError(errorRecord);
} }
#endregion #endregion
@ -140,7 +140,7 @@ namespace Microsoft.PowerShell.Commands
{ {
if (_generator == null) if (_generator == null)
{ {
Guid runspaceId = this.Context.CurrentRunspace.InstanceId; Guid runspaceId = Context.CurrentRunspace.InstanceId;
bool needToInitialize = false; bool needToInitialize = false;
try try
@ -155,7 +155,7 @@ namespace Microsoft.PowerShell.Commands
if (needToInitialize) if (needToInitialize)
{ {
this.Generator = new PolymorphicRandomNumberGenerator(); Generator = new PolymorphicRandomNumberGenerator();
} }
} }
@ -165,7 +165,7 @@ namespace Microsoft.PowerShell.Commands
set set
{ {
_generator = value; _generator = value;
Runspace myRunspace = this.Context.CurrentRunspace; Runspace myRunspace = Context.CurrentRunspace;
try try
{ {
@ -283,16 +283,16 @@ namespace Microsoft.PowerShell.Commands
/// </summary> /// </summary>
[Parameter(ParameterSetName = GetRandomCommand.RandomListItemParameterSet)] [Parameter(ParameterSetName = GetRandomCommand.RandomListItemParameterSet)]
[ValidateRange(1, int.MaxValue)] [ValidateRange(1, int.MaxValue)]
public int Count { get; set; } public int Count { get; set; } = 1;
#endregion #endregion
#region Cmdlet processing methods #region Cmdlet processing methods
private double GetRandomDouble(double min, double max) private double GetRandomDouble(double minValue, double maxValue)
{ {
double randomNumber; double randomNumber;
double diff = max - min; double diff = maxValue - minValue;
// I couldn't find a better fix for bug #216893 then // I couldn't find a better fix for bug #216893 then
// to test and retry if a random number falls outside the bounds // to test and retry if a random number falls outside the bounds
@ -306,20 +306,20 @@ namespace Microsoft.PowerShell.Commands
{ {
do do
{ {
double r = this.Generator.NextDouble(); double r = Generator.NextDouble();
randomNumber = min + r * max - r * min; randomNumber = minValue + r * maxValue - r * minValue;
} }
while (randomNumber >= max); while (randomNumber >= maxValue);
} }
else else
{ {
do do
{ {
double r = this.Generator.NextDouble(); double r = Generator.NextDouble();
randomNumber = min + r * diff; randomNumber = minValue + r * diff;
diff = diff * r; diff = diff * r;
} }
while (randomNumber >= max); while (randomNumber >= maxValue);
} }
return randomNumber; return randomNumber;
@ -328,22 +328,22 @@ namespace Microsoft.PowerShell.Commands
/// <summary> /// <summary>
/// Get a random Int64 type number. /// Get a random Int64 type number.
/// </summary> /// </summary>
/// <param name="min"></param> /// <param name="minValue"></param>
/// <param name="max"></param> /// <param name="maxValue"></param>
/// <returns></returns> /// <returns></returns>
private Int64 GetRandomInt64(Int64 min, Int64 max) private Int64 GetRandomInt64(Int64 minValue, Int64 maxValue)
{ {
// Randomly generate eight bytes and convert the byte array to UInt64 // Randomly generate eight bytes and convert the byte array to UInt64
var buffer = new byte[sizeof(UInt64)]; var buffer = new byte[sizeof(UInt64)];
UInt64 randomUint64; UInt64 randomUint64;
BigInteger bigIntegerDiff = (BigInteger)max - (BigInteger)min; BigInteger bigIntegerDiff = (BigInteger)maxValue - (BigInteger)minValue;
// When the difference is less than int.MaxValue, use Random.Next(int, int) // When the difference is less than int.MaxValue, use Random.Next(int, int)
if (bigIntegerDiff <= int.MaxValue) if (bigIntegerDiff <= int.MaxValue)
{ {
int randomDiff = this.Generator.Next(0, (int)(max - min)); int randomDiff = Generator.Next(0, (int)(maxValue - minValue));
return min + randomDiff; return minValue + randomDiff;
} }
// The difference of two Int64 numbers would not exceed UInt64.MaxValue, so it can be represented by a UInt64 number. // The difference of two Int64 numbers would not exceed UInt64.MaxValue, so it can be represented by a UInt64 number.
@ -361,14 +361,15 @@ namespace Microsoft.PowerShell.Commands
do do
{ {
// Randomly fill the buffer // Randomly fill the buffer
this.Generator.NextBytes(buffer); Generator.NextBytes(buffer);
randomUint64 = BitConverter.ToUInt64(buffer, 0); randomUint64 = BitConverter.ToUInt64(buffer, 0);
// Get the last 'bitsToRepresentDiff' number of randon bits
// Get the last 'bitsToRepresentDiff' number of random bits
randomUint64 &= mask; randomUint64 &= mask;
} while (uint64Diff <= randomUint64); } while (uint64Diff <= randomUint64);
double result = min * 1.0 + randomUint64 * 1.0; double randomNumber = minValue * 1.0 + randomUint64 * 1.0;
return (Int64)result; return (Int64)randomNumber;
} }
/// <summary> /// <summary>
@ -376,98 +377,93 @@ namespace Microsoft.PowerShell.Commands
/// </summary> /// </summary>
protected override void BeginProcessing() protected override void BeginProcessing()
{ {
if (this.SetSeed.HasValue) if (SetSeed.HasValue)
{ {
this.Generator = new PolymorphicRandomNumberGenerator(this.SetSeed.Value); Generator = new PolymorphicRandomNumberGenerator(SetSeed.Value);
} }
if (this.EffectiveParameterSet == MyParameterSet.RandomNumber) if (EffectiveParameterSet == MyParameterSet.RandomNumber)
{ {
object maxOperand = ProcessOperand(this.Maximum); object maxOperand = ProcessOperand(Maximum);
object minOperand = ProcessOperand(this.Minimum); object minOperand = ProcessOperand(Minimum);
if (IsInt(maxOperand) && IsInt(minOperand)) if (IsInt(maxOperand) && IsInt(minOperand))
{ {
int min = minOperand != null ? (int)minOperand : 0; int minValue = minOperand != null ? (int)minOperand : 0;
int max = maxOperand != null ? (int)maxOperand : int.MaxValue; int maxValue = maxOperand != null ? (int)maxOperand : int.MaxValue;
if (min >= max) if (minValue >= maxValue)
{ {
this.ThrowMinGreaterThanOrEqualMax(min, max); ThrowMinGreaterThanOrEqualMax(minValue, maxValue);
} }
int randomNumber = this.Generator.Next(min, max); int randomNumber = Generator.Next(minValue, maxValue);
Debug.Assert(min <= randomNumber, "lower bound <= random number"); Debug.Assert(minValue <= randomNumber, "lower bound <= random number");
Debug.Assert(randomNumber < max, "random number < upper bound"); Debug.Assert(randomNumber < maxValue, "random number < upper bound");
this.WriteObject(randomNumber); WriteObject(randomNumber);
} }
else if ((IsInt64(maxOperand) || IsInt(maxOperand)) && (IsInt64(minOperand) || IsInt(minOperand))) else if ((IsInt64(maxOperand) || IsInt(maxOperand)) && (IsInt64(minOperand) || IsInt(minOperand)))
{ {
Int64 min = minOperand != null ? ((minOperand is Int64) ? (Int64)minOperand : (int)minOperand) : 0; Int64 minValue = minOperand != null ? ((minOperand is Int64) ? (Int64)minOperand : (int)minOperand) : 0;
Int64 max = maxOperand != null ? ((maxOperand is Int64) ? (Int64)maxOperand : (int)maxOperand) : Int64.MaxValue; Int64 maxValue = maxOperand != null ? ((maxOperand is Int64) ? (Int64)maxOperand : (int)maxOperand) : Int64.MaxValue;
if (min >= max) if (minValue >= maxValue)
{ {
this.ThrowMinGreaterThanOrEqualMax(min, max); ThrowMinGreaterThanOrEqualMax(minValue, maxValue);
} }
Int64 randomNumber = this.GetRandomInt64(min, max); Int64 randomNumber = GetRandomInt64(minValue, maxValue);
Debug.Assert(min <= randomNumber, "lower bound <= random number"); Debug.Assert(minValue <= randomNumber, "lower bound <= random number");
Debug.Assert(randomNumber < max, "random number < upper bound"); Debug.Assert(randomNumber < maxValue, "random number < upper bound");
this.WriteObject(randomNumber); WriteObject(randomNumber);
} }
else else
{ {
double min = (minOperand is double) ? (double)minOperand : this.ConvertToDouble(this.Minimum, 0.0); double minValue = (minOperand is double) ? (double)minOperand : ConvertToDouble(Minimum, 0.0);
double max = (maxOperand is double) ? (double)maxOperand : this.ConvertToDouble(this.Maximum, double.MaxValue); double maxValue = (maxOperand is double) ? (double)maxOperand : ConvertToDouble(Maximum, double.MaxValue);
if (min >= max) if (minValue >= maxValue)
{ {
this.ThrowMinGreaterThanOrEqualMax(min, max); ThrowMinGreaterThanOrEqualMax(minValue, maxValue);
} }
double randomNumber = this.GetRandomDouble(min, max); double randomNumber = GetRandomDouble(minValue, maxValue);
Debug.Assert(min <= randomNumber, "lower bound <= random number"); Debug.Assert(minValue <= randomNumber, "lower bound <= random number");
Debug.Assert(randomNumber < max, "random number < upper bound"); Debug.Assert(randomNumber < maxValue, "random number < upper bound");
this.WriteObject(randomNumber); WriteObject(randomNumber);
} }
} }
else if (this.EffectiveParameterSet == MyParameterSet.RandomListItem) else if (EffectiveParameterSet == MyParameterSet.RandomListItem)
{ {
_chosenListItems = new List<object>(); _chosenListItems = new List<object>();
_numberOfProcessedListItems = 0; _numberOfProcessedListItems = 0;
if (this.Count == 0) // -Count not specified
{
this.Count = 1; // default to one random item by default
}
} }
} }
// rough proof that when choosing random K items out of N items // rough proof that when choosing random K items out of N items
// each item has got K/N probability of being included in the final list // each item has got K/N probability of being included in the final list
// //
// probability that a particular item in this.chosenListItems is NOT going to be replaced // probability that a particular item in chosenListItems is NOT going to be replaced
// when processing I-th input item [assumes I > K]: // when processing I-th input item [assumes I > K]:
// P_one_step(I) = 1 - ((K / I) * ((K - 1) / K) + ((I - K) / I) = (I - 1) / I // P_one_step(I) = 1 - ((K / I) * ((K - 1) / K) + ((I - K) / I) = (I - 1) / I
// <--A--> <-----B-----> <-----C-----> // <--A--> <-----B-----> <-----C----->
// A - probability that I-th element is going to be replacing an element from this.chosenListItems // A - probability that I-th element is going to be replacing an element from chosenListItems
// (see (1) in the code below) // (see (1) in the code below)
// B - probability that a particular element from this.chosenListItems is NOT going to be replaced // B - probability that a particular element from chosenListItems is NOT going to be replaced
// (see (2) in the code below) // (see (2) in the code below)
// C - probability that I-th element is NOT going to be replacing an element from this.chosenListItems // C - probability that I-th element is NOT going to be replacing an element from chosenListItems
// (see (1) in the code below) // (see (1) in the code below)
// //
// probability that a particular item in this.chosenListItems is NOT going to be replaced // probability that a particular item in chosenListItems is NOT going to be replaced
// when processing input items J through N [assumes J > K] // when processing input items J through N [assumes J > K]
// P_removal(J) = Multiply(for I = J to N) P(I) = // P_removal(J) = Multiply(for I = J to N) P(I) =
// = ((J - 1) / J) * (J / (J + 1)) * ... * ((N - 2) / (N - 1)) * ((N - 1) / N) = // = ((J - 1) / J) * (J / (J + 1)) * ... * ((N - 2) / (N - 1)) * ((N - 1) / N) =
// = (J - 1) / N // = (J - 1) / N
// //
// probability that when processing an element it is going to be put into this.chosenListItems // probability that when processing an element it is going to be put into chosenListItems
// P_insertion(I) = 1.0 when I <= K - see (3) in the code below // P_insertion(I) = 1.0 when I <= K - see (3) in the code below
// P_insertion(I) = K/N otherwise - see (1) in the code below // P_insertion(I) = K/N otherwise - see (1) in the code below
// //
@ -483,21 +479,21 @@ namespace Microsoft.PowerShell.Commands
/// </summary> /// </summary>
protected override void ProcessRecord() protected override void ProcessRecord()
{ {
if (this.EffectiveParameterSet == MyParameterSet.RandomListItem) if (EffectiveParameterSet == MyParameterSet.RandomListItem)
{ {
foreach (object item in this.InputObject) foreach (object item in InputObject)
{ {
if (_numberOfProcessedListItems < this.Count) // (3) if (_numberOfProcessedListItems < Count) // (3)
{ {
Debug.Assert(_chosenListItems.Count == _numberOfProcessedListItems, "Initial K elements should all be included in this.chosenListItems"); Debug.Assert(_chosenListItems.Count == _numberOfProcessedListItems, "Initial K elements should all be included in chosenListItems");
_chosenListItems.Add(item); _chosenListItems.Add(item);
} }
else else
{ {
Debug.Assert(_chosenListItems.Count == this.Count, "After processing K initial elements, the length of this.chosenItems should stay equal to K"); Debug.Assert(_chosenListItems.Count == Count, "After processing K initial elements, the length of chosenItems should stay equal to K");
if (this.Generator.Next(_numberOfProcessedListItems + 1) < this.Count) // (1) if (Generator.Next(_numberOfProcessedListItems + 1) < Count) // (1)
{ {
int indexToReplace = this.Generator.Next(_chosenListItems.Count); // (2) int indexToReplace = Generator.Next(_chosenListItems.Count); // (2)
_chosenListItems[indexToReplace] = item; _chosenListItems[indexToReplace] = item;
} }
} }
@ -512,7 +508,7 @@ namespace Microsoft.PowerShell.Commands
/// </summary> /// </summary>
protected override void EndProcessing() protected override void EndProcessing()
{ {
if (this.EffectiveParameterSet == MyParameterSet.RandomListItem) if (EffectiveParameterSet == MyParameterSet.RandomListItem)
{ {
// make sure the order is truly random // make sure the order is truly random
// (all permutations with the same probability) // (all permutations with the same probability)
@ -521,9 +517,9 @@ namespace Microsoft.PowerShell.Commands
for (int i = 0; i < n; i++) for (int i = 0; i < n; i++)
{ {
// randomly choose j from [i...n) // randomly choose j from [i...n)
int j = this.Generator.Next(i, n); int j = Generator.Next(i, n);
this.WriteObject(_chosenListItems[j]); WriteObject(_chosenListItems[j]);
// remove the output object from consideration in the next iteration. // remove the output object from consideration in the next iteration.
if (i != j) if (i != j)
@ -580,23 +576,23 @@ namespace Microsoft.PowerShell.Commands
/// <returns>A non-negative random integer.</returns> /// <returns>A non-negative random integer.</returns>
internal int Next() internal int Next()
{ {
int result; int randomNumber;
// The CLR implementation just fudges // The CLR implementation just fudges
// Int32.MaxValue down to (Int32.MaxValue - 1). This implementation // Int32.MaxValue down to (Int32.MaxValue - 1). This implementation
// errs on the side of correctness. // errs on the side of correctness.
do do
{ {
result = InternalSample(); randomNumber = InternalSample();
} }
while (result == Int32.MaxValue); while (randomNumber == Int32.MaxValue);
if (result < 0) if (randomNumber < 0)
{ {
result += Int32.MaxValue; randomNumber += Int32.MaxValue;
} }
return result; return randomNumber;
} }
/// <summary> /// <summary>
@ -627,18 +623,20 @@ namespace Microsoft.PowerShell.Commands
throw new ArgumentOutOfRangeException("minValue", GetRandomCommandStrings.MinGreaterThanOrEqualMaxApi); throw new ArgumentOutOfRangeException("minValue", GetRandomCommandStrings.MinGreaterThanOrEqualMaxApi);
} }
int randomNumber = 0;
long range = (long)maxValue - (long)minValue; long range = (long)maxValue - (long)minValue;
if (range <= int.MaxValue) if (range <= int.MaxValue)
{ {
return ((int)(NextDouble() * range) + minValue); randomNumber = ((int)(NextDouble() * range) + minValue);
} }
else else
{ {
double largeSample = InternalSampleLargeRange() * (1.0 / (2 * ((uint)Int32.MaxValue))); double largeSample = InternalSampleLargeRange() * (1.0 / (2 * ((uint)Int32.MaxValue)));
int result = (int)((long)(largeSample * range) + minValue); randomNumber = (int)((long)(largeSample * range) + minValue);
return result;
} }
return randomNumber;
} }
/// <summary> /// <summary>
@ -663,13 +661,13 @@ namespace Microsoft.PowerShell.Commands
/// <returns>A random integer, using the full range of Int32.</returns> /// <returns>A random integer, using the full range of Int32.</returns>
private int InternalSample() private int InternalSample()
{ {
int result; int randomNumber;
byte[] data = new byte[sizeof(int)]; byte[] data = new byte[sizeof(int)];
NextBytes(data); NextBytes(data);
result = BitConverter.ToInt32(data, 0); randomNumber = BitConverter.ToInt32(data, 0);
return result; return randomNumber;
} }
/// <summary> /// <summary>
@ -680,15 +678,15 @@ namespace Microsoft.PowerShell.Commands
/// <returns></returns> /// <returns></returns>
private double InternalSampleLargeRange() private double InternalSampleLargeRange()
{ {
double result; double randomNumber;
do do
{ {
result = InternalSample(); randomNumber = InternalSample();
} while (result == Int32.MaxValue); } while (randomNumber == Int32.MaxValue);
result += Int32.MaxValue; randomNumber += Int32.MaxValue;
return result; return randomNumber;
} }
} }
} }

View file

@ -44,7 +44,6 @@ Describe "Get-Random DRT Unit Tests" -Tags "CI" {
@{ Name = 'max is double with plus sign and enclosed in quote'; Maximum = '+100.0'; Minimum = 0; GreaterThan = -1.0; LessThan = 100.0; Type = 'System.Double' } @{ Name = 'max is double with plus sign and enclosed in quote'; Maximum = '+100.0'; Minimum = 0; GreaterThan = -1.0; LessThan = 100.0; Type = 'System.Double' }
@{ Name = 'both set to the special numbers as 1.0e+xx '; Maximum = $null; Minimum = 1.0e+100; GreaterThan = 1.0e+99; LessThan = ([double]::MaxValue); Type = 'System.Double' } @{ Name = 'both set to the special numbers as 1.0e+xx '; Maximum = $null; Minimum = 1.0e+100; GreaterThan = 1.0e+99; LessThan = ([double]::MaxValue); Type = 'System.Double' }
@{ Name = 'max is Double.MaxValue, min is Double.MinValue'; Maximum = ([double]::MaxValue); Minimum = ([double]::MinValue); GreaterThan = ([double]::MinValue); LessThan = ([double]::MaxValue); Type = 'System.Double' } @{ Name = 'max is Double.MaxValue, min is Double.MinValue'; Maximum = ([double]::MaxValue); Minimum = ([double]::MinValue); GreaterThan = ([double]::MinValue); LessThan = ([double]::MaxValue); Type = 'System.Double' }
) )
$testDataForError = @( $testDataForError = @(
@ -57,7 +56,6 @@ Describe "Get-Random DRT Unit Tests" -Tags "CI" {
@{ Name = 'Min is greater than max and all are negative double-precision number'; Maximum = -20.0; Minimum = -10.0} @{ Name = 'Min is greater than max and all are negative double-precision number'; Maximum = -20.0; Minimum = -10.0}
@{ Name = 'Min and Max are same and all are negative double-precision number'; Maximum = -20.0; Minimum = -20.0} @{ Name = 'Min and Max are same and all are negative double-precision number'; Maximum = -20.0; Minimum = -20.0}
@{ Name = 'Max is a negative number, min is the default number '; Maximum = -10; Minimum = $null} @{ Name = 'Max is a negative number, min is the default number '; Maximum = -10; Minimum = $null}
) )
# minimum is always set to the actual low end of the range, details refer to closed issue #887. # minimum is always set to the actual low end of the range, details refer to closed issue #887.
@ -84,10 +82,11 @@ Describe "Get-Random DRT Unit Tests" -Tags "CI" {
} }
Describe "Get-Random" -Tags "CI" { Describe "Get-Random" -Tags "CI" {
It "Should return a random number greater than -1 " { It "Should return a random number greater than -1" {
Get-Random | Should -BeGreaterThan -1 Get-Random | Should -BeGreaterThan -1
} }
It "Should return a random number less than 100 " {
It "Should return a random number less than 100" {
Get-Random -Maximum 100 | Should -BeLessThan 100 Get-Random -Maximum 100 | Should -BeLessThan 100
Get-Random -Maximum 100 | Should -BeGreaterThan -1 Get-Random -Maximum 100 | Should -BeGreaterThan -1
} }