From 19b0272158c7be5e0244182409b42445a56807da Mon Sep 17 00:00:00 2001 From: pougetat Date: Mon, 11 Mar 2019 14:21:58 +0100 Subject: [PATCH 1/6] Clean up Get-Random cmdlet --- .../commands/utility/GetRandomCommand.cs | 145 +++++++++--------- .../Get-Random.Tests.ps1 | 3 +- 2 files changed, 71 insertions(+), 77 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs index 4bb98b8bad0..f54dcceadfd 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs @@ -44,7 +44,7 @@ private MyParameterSet EffectiveParameterSet // cache MyParameterSet enum instead of doing string comparison every time if (_effectiveParameterSet == MyParameterSet.Unknown) { - if ((this.MyInvocation.ExpectingInput) && (this.Maximum == null) && (this.Minimum == null)) + if ((MyInvocation.ExpectingInput) && (Maximum == null) && (Minimum == null)) { _effectiveParameterSet = MyParameterSet.RandomListItem; } @@ -52,11 +52,11 @@ private MyParameterSet EffectiveParameterSet { _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; } else @@ -97,7 +97,7 @@ private void ThrowMinGreaterThanOrEqualMax(object min, object max) ErrorCategory.InvalidArgument, null); - this.ThrowTerminatingError(errorRecord); + ThrowTerminatingError(errorRecord); } #endregion @@ -140,7 +140,7 @@ private PolymorphicRandomNumberGenerator Generator { if (_generator == null) { - Guid runspaceId = this.Context.CurrentRunspace.InstanceId; + Guid runspaceId = Context.CurrentRunspace.InstanceId; bool needToInitialize = false; try @@ -155,7 +155,7 @@ private PolymorphicRandomNumberGenerator Generator if (needToInitialize) { - this.Generator = new PolymorphicRandomNumberGenerator(); + Generator = new PolymorphicRandomNumberGenerator(); } } @@ -165,7 +165,7 @@ private PolymorphicRandomNumberGenerator Generator set { _generator = value; - Runspace myRunspace = this.Context.CurrentRunspace; + Runspace myRunspace = Context.CurrentRunspace; try { @@ -283,7 +283,7 @@ private double ConvertToDouble(object o, double defaultIfNull) /// [Parameter(ParameterSetName = GetRandomCommand.RandomListItemParameterSet)] [ValidateRange(1, int.MaxValue)] - public int Count { get; set; } + public int Count { get; set; } = 1; #endregion @@ -291,7 +291,7 @@ private double ConvertToDouble(object o, double defaultIfNull) private double GetRandomDouble(double min, double max) { - double randomNumber; + double result; double diff = max - min; // I couldn't find a better fix for bug #216893 then @@ -306,23 +306,25 @@ private double GetRandomDouble(double min, double max) { do { - double r = this.Generator.NextDouble(); - randomNumber = min + r * max - r * min; + double r = Generator.NextDouble(); + result = min + r * max - r * min; } - while (randomNumber >= max); + while (result >= max); } else { do { - double r = this.Generator.NextDouble(); - randomNumber = min + r * diff; + double r = Generator.NextDouble(); + result = min + r * diff; diff = diff * r; } - while (randomNumber >= max); + while (result >= max); } - return randomNumber; + Debug.Assert(min <= result, "lower bound <= random number"); + Debug.Assert(result < max, "random number < upper bound"); + return result; } /// @@ -342,7 +344,7 @@ private Int64 GetRandomInt64(Int64 min, Int64 max) // When the difference is less than int.MaxValue, use Random.Next(int, int) if (bigIntegerDiff <= int.MaxValue) { - int randomDiff = this.Generator.Next(0, (int)(max - min)); + int randomDiff = Generator.Next(0, (int)(max - min)); return min + randomDiff; } @@ -361,13 +363,16 @@ private Int64 GetRandomInt64(Int64 min, Int64 max) do { // Randomly fill the buffer - this.Generator.NextBytes(buffer); + Generator.NextBytes(buffer); randomUint64 = BitConverter.ToUInt64(buffer, 0); // Get the last 'bitsToRepresentDiff' number of randon bits randomUint64 &= mask; } while (uint64Diff <= randomUint64); double result = min * 1.0 + randomUint64 * 1.0; + + Debug.Assert(min <= result, "lower bound <= random number"); + Debug.Assert(result < max, "random number < upper bound"); return (Int64)result; } @@ -376,15 +381,15 @@ private Int64 GetRandomInt64(Int64 min, Int64 max) /// 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 minOperand = ProcessOperand(this.Minimum); + object maxOperand = ProcessOperand(Maximum); + object minOperand = ProcessOperand(Minimum); if (IsInt(maxOperand) && IsInt(minOperand)) { @@ -393,14 +398,11 @@ protected override void BeginProcessing() if (min >= max) { - this.ThrowMinGreaterThanOrEqualMax(min, max); + ThrowMinGreaterThanOrEqualMax(min, max); } - int randomNumber = this.Generator.Next(min, max); - Debug.Assert(min <= randomNumber, "lower bound <= random number"); - Debug.Assert(randomNumber < max, "random number < upper bound"); - - this.WriteObject(randomNumber); + int randomNumber = Generator.Next(min, max); + WriteObject(randomNumber); } else if ((IsInt64(maxOperand) || IsInt(maxOperand)) && (IsInt64(minOperand) || IsInt(minOperand))) { @@ -409,65 +411,54 @@ protected override void BeginProcessing() if (min >= max) { - this.ThrowMinGreaterThanOrEqualMax(min, max); + ThrowMinGreaterThanOrEqualMax(min, max); } - Int64 randomNumber = this.GetRandomInt64(min, max); - Debug.Assert(min <= randomNumber, "lower bound <= random number"); - Debug.Assert(randomNumber < max, "random number < upper bound"); - - this.WriteObject(randomNumber); + Int64 randomNumber = GetRandomInt64(min, max); + WriteObject(randomNumber); } else { - double min = (minOperand is double) ? (double)minOperand : this.ConvertToDouble(this.Minimum, 0.0); - double max = (maxOperand is double) ? (double)maxOperand : this.ConvertToDouble(this.Maximum, double.MaxValue); + double min = (minOperand is double) ? (double)minOperand : ConvertToDouble(Minimum, 0.0); + double max = (maxOperand is double) ? (double)maxOperand : ConvertToDouble(Maximum, double.MaxValue); if (min >= max) { - this.ThrowMinGreaterThanOrEqualMax(min, max); + ThrowMinGreaterThanOrEqualMax(min, max); } - double randomNumber = this.GetRandomDouble(min, max); - Debug.Assert(min <= randomNumber, "lower bound <= random number"); - Debug.Assert(randomNumber < max, "random number < upper bound"); - - this.WriteObject(randomNumber); + double randomNumber = GetRandomDouble(min, max); + WriteObject(randomNumber); } } - else if (this.EffectiveParameterSet == MyParameterSet.RandomListItem) + else if (EffectiveParameterSet == MyParameterSet.RandomListItem) { _chosenListItems = new List(); _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 // 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]: // P_one_step(I) = 1 - ((K / I) * ((K - 1) / K) + ((I - K) / I) = (I - 1) / I // <--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) - // 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) - // 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) // - // 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] // 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) / 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) = K/N otherwise - see (1) in the code below // @@ -483,21 +474,21 @@ protected override void BeginProcessing() /// 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); } else { - Debug.Assert(_chosenListItems.Count == this.Count, "After processing K initial elements, the length of this.chosenItems should stay equal to K"); - if (this.Generator.Next(_numberOfProcessedListItems + 1) < this.Count) // (1) + Debug.Assert(_chosenListItems.Count == Count, "After processing K initial elements, the length of chosenItems should stay equal to K"); + 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; } } @@ -512,7 +503,7 @@ protected override void ProcessRecord() /// protected override void EndProcessing() { - if (this.EffectiveParameterSet == MyParameterSet.RandomListItem) + if (EffectiveParameterSet == MyParameterSet.RandomListItem) { // make sure the order is truly random // (all permutations with the same probability) @@ -521,9 +512,9 @@ protected override void EndProcessing() for (int i = 0; i < n; i++) { // 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. if (i != j) @@ -617,28 +608,32 @@ internal int Next(int maxValue) /// /// Returns a random integer that is within a specified range. /// - /// The inclusive lower bound of the random number returned. - /// The exclusive upper bound of the random number returned. maxValue must be greater than or equal to minValue. + /// The inclusive lower bound of the random number returned. + /// The exclusive upper bound of the random number returned. maxValue must be greater than or equal to minValue. /// - public int Next(int minValue, int maxValue) + public int Next(int min, int max) { - if (minValue > maxValue) + if (min > max) { throw new ArgumentOutOfRangeException("minValue", GetRandomCommandStrings.MinGreaterThanOrEqualMaxApi); } - long range = (long)maxValue - (long)minValue; + int result = 0; + + long range = (long)max - (long)min; if (range <= int.MaxValue) { - return ((int)(NextDouble() * range) + minValue); + result = ((int)(NextDouble() * range) + min); } else { double largeSample = InternalSampleLargeRange() * (1.0 / (2 * ((uint)Int32.MaxValue))); - int result = (int)((long)(largeSample * range) + minValue); - - return result; + result = (int)((long)(largeSample * range) + min); } + + Debug.Assert(min <= result, "lower bound <= random number"); + Debug.Assert(result < max, "random number < upper bound"); + return result; } /// diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 index d47280ca76d..429a7bdc7a1 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 @@ -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 = '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' } - ) $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 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} - ) # minimum is always set to the actual low end of the range, details refer to closed issue #887. @@ -87,6 +85,7 @@ Describe "Get-Random" -Tags "CI" { It "Should return a random number greater than -1 " { Get-Random | Should -BeGreaterThan -1 } + It "Should return a random number less than 100 " { Get-Random -Maximum 100 | Should -BeLessThan 100 Get-Random -Maximum 100 | Should -BeGreaterThan -1 From 88a1ca4cec5666b0fa3c4ac139b7a3100c2d3fd5 Mon Sep 17 00:00:00 2001 From: pougetat Date: Thu, 14 Mar 2019 12:43:24 +0100 Subject: [PATCH 2/6] Fix PR comments --- .../commands/utility/GetRandomCommand.cs | 130 +++++++++--------- 1 file changed, 65 insertions(+), 65 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs index f54dcceadfd..b3810652380 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs @@ -78,21 +78,21 @@ private MyParameterSet EffectiveParameterSet #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"); } - if (max == null) + if (maxValue == null) { throw PSTraceSource.NewArgumentNullException("max"); } ErrorRecord errorRecord = new ErrorRecord( new ArgumentException(string.Format( - CultureInfo.InvariantCulture, GetRandomCommandStrings.MinGreaterThanOrEqualMax, min, max)), + CultureInfo.InvariantCulture, GetRandomCommandStrings.MinGreaterThanOrEqualMax, minValue, maxValue)), "MinGreaterThanOrEqualMax", ErrorCategory.InvalidArgument, null); @@ -289,10 +289,10 @@ private double ConvertToDouble(object o, double defaultIfNull) #region Cmdlet processing methods - private double GetRandomDouble(double min, double max) + private double GetRandomDouble(double minValue, double maxValue) { - double result; - double diff = max - min; + double randomNumber; + double diff = maxValue - minValue; // I couldn't find a better fix for bug #216893 then // to test and retry if a random number falls outside the bounds @@ -307,45 +307,45 @@ private double GetRandomDouble(double min, double max) do { double r = Generator.NextDouble(); - result = min + r * max - r * min; + randomNumber = minValue + r * maxValue - r * minValue; } - while (result >= max); + while (randomNumber >= maxValue); } else { do { double r = Generator.NextDouble(); - result = min + r * diff; + randomNumber = minValue + r * diff; diff = diff * r; } - while (result >= max); + while (randomNumber >= maxValue); } - Debug.Assert(min <= result, "lower bound <= random number"); - Debug.Assert(result < max, "random number < upper bound"); - return result; + Debug.Assert(minValue <= randomNumber, "lower bound <= random number"); + Debug.Assert(randomNumber < maxValue, "random number < upper bound"); + return randomNumber; } /// /// Get a random Int64 type number. /// - /// - /// + /// + /// /// - 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 var buffer = new byte[sizeof(UInt64)]; 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) if (bigIntegerDiff <= int.MaxValue) { - int randomDiff = Generator.Next(0, (int)(max - min)); - return min + randomDiff; + int randomDiff = Generator.Next(0, (int)(maxValue - minValue)); + return minValue + randomDiff; } // The difference of two Int64 numbers would not exceed UInt64.MaxValue, so it can be represented by a UInt64 number. @@ -365,15 +365,15 @@ private Int64 GetRandomInt64(Int64 min, Int64 max) // Randomly fill the buffer Generator.NextBytes(buffer); randomUint64 = BitConverter.ToUInt64(buffer, 0); - // Get the last 'bitsToRepresentDiff' number of randon bits + // Get the last 'bitsToRepresentDiff' number of random bits randomUint64 &= mask; } while (uint64Diff <= randomUint64); - double result = min * 1.0 + randomUint64 * 1.0; + double randomNumber = minValue * 1.0 + randomUint64 * 1.0; - Debug.Assert(min <= result, "lower bound <= random number"); - Debug.Assert(result < max, "random number < upper bound"); - return (Int64)result; + Debug.Assert(minValue <= randomNumber, "lower bound <= random number"); + Debug.Assert(randomNumber < maxValue, "random number < upper bound"); + return (Int64)randomNumber; } /// @@ -393,41 +393,41 @@ protected override void BeginProcessing() if (IsInt(maxOperand) && IsInt(minOperand)) { - int min = minOperand != null ? (int)minOperand : 0; - int max = maxOperand != null ? (int)maxOperand : int.MaxValue; + int minValue = minOperand != null ? (int)minOperand : 0; + int maxValue = maxOperand != null ? (int)maxOperand : int.MaxValue; - if (min >= max) + if (minValue >= maxValue) { - ThrowMinGreaterThanOrEqualMax(min, max); + ThrowMinGreaterThanOrEqualMax(minValue, maxValue); } - int randomNumber = Generator.Next(min, max); + int randomNumber = Generator.Next(minValue, maxValue); WriteObject(randomNumber); } else if ((IsInt64(maxOperand) || IsInt(maxOperand)) && (IsInt64(minOperand) || IsInt(minOperand))) { - Int64 min = minOperand != null ? ((minOperand is Int64) ? (Int64)minOperand : (int)minOperand) : 0; - Int64 max = maxOperand != null ? ((maxOperand is Int64) ? (Int64)maxOperand : (int)maxOperand) : Int64.MaxValue; + Int64 minValue = minOperand != null ? ((minOperand is Int64) ? (Int64)minOperand : (int)minOperand) : 0; + Int64 maxValue = maxOperand != null ? ((maxOperand is Int64) ? (Int64)maxOperand : (int)maxOperand) : Int64.MaxValue; - if (min >= max) + if (minValue >= maxValue) { - ThrowMinGreaterThanOrEqualMax(min, max); + ThrowMinGreaterThanOrEqualMax(minValue, maxValue); } - Int64 randomNumber = GetRandomInt64(min, max); + Int64 randomNumber = GetRandomInt64(minValue, maxValue); WriteObject(randomNumber); } else { - double min = (minOperand is double) ? (double)minOperand : ConvertToDouble(Minimum, 0.0); - double max = (maxOperand is double) ? (double)maxOperand : ConvertToDouble(Maximum, double.MaxValue); + double minValue = (minOperand is double) ? (double)minOperand : ConvertToDouble(Minimum, 0.0); + double maxValue = (maxOperand is double) ? (double)maxOperand : ConvertToDouble(Maximum, double.MaxValue); - if (min >= max) + if (minValue >= maxValue) { - ThrowMinGreaterThanOrEqualMax(min, max); + ThrowMinGreaterThanOrEqualMax(minValue, maxValue); } - double randomNumber = GetRandomDouble(min, max); + double randomNumber = GetRandomDouble(minValue, maxValue); WriteObject(randomNumber); } } @@ -571,23 +571,23 @@ internal double NextDouble() /// A non-negative random integer. internal int Next() { - int result; + int randomNumber; // The CLR implementation just fudges // Int32.MaxValue down to (Int32.MaxValue - 1). This implementation // errs on the side of correctness. 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; } /// @@ -608,32 +608,32 @@ internal int Next(int maxValue) /// /// Returns a random integer that is within a specified range. /// - /// The inclusive lower bound of the random number returned. - /// The exclusive upper bound of the random number returned. maxValue must be greater than or equal to minValue. + /// The inclusive lower bound of the random number returned. + /// The exclusive upper bound of the random number returned. maxValue must be greater than or equal to minValue. /// - public int Next(int min, int max) + public int Next(int minValue, int maxValue) { - if (min > max) + if (minValue > maxValue) { throw new ArgumentOutOfRangeException("minValue", GetRandomCommandStrings.MinGreaterThanOrEqualMaxApi); } - int result = 0; + int randomNumber = 0; - long range = (long)max - (long)min; + long range = (long)maxValue - (long)minValue; if (range <= int.MaxValue) { - result = ((int)(NextDouble() * range) + min); + randomNumber = ((int)(NextDouble() * range) + minValue); } else { double largeSample = InternalSampleLargeRange() * (1.0 / (2 * ((uint)Int32.MaxValue))); - result = (int)((long)(largeSample * range) + min); + randomNumber = (int)((long)(largeSample * range) + minValue); } - Debug.Assert(min <= result, "lower bound <= random number"); - Debug.Assert(result < max, "random number < upper bound"); - return result; + Debug.Assert(minValue <= randomNumber, "lower bound <= random number"); + Debug.Assert(randomNumber < maxValue, "random number < upper bound"); + return randomNumber; } /// @@ -658,13 +658,13 @@ internal void NextBytes(byte[] buffer) /// A random integer, using the full range of Int32. private int InternalSample() { - int result; + int randomNumber; byte[] data = new byte[sizeof(int)]; NextBytes(data); - result = BitConverter.ToInt32(data, 0); + randomNumber = BitConverter.ToInt32(data, 0); - return result; + return randomNumber; } /// @@ -675,15 +675,15 @@ private int InternalSample() /// private double InternalSampleLargeRange() { - double result; + double randomNumber; do { - result = InternalSample(); - } while (result == Int32.MaxValue); + randomNumber = InternalSample(); + } while (randomNumber == Int32.MaxValue); - result += Int32.MaxValue; - return result; + randomNumber += Int32.MaxValue; + return randomNumber; } } } From 2d33ab333c8366494eb25fdf32f02fc491ee6e13 Mon Sep 17 00:00:00 2001 From: Reece Dunham Date: Thu, 14 Mar 2019 17:31:45 +0100 Subject: [PATCH 3/6] Update test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 Co-Authored-By: pougetat --- .../Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 index 429a7bdc7a1..f08fee5ce11 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 @@ -86,7 +86,7 @@ Describe "Get-Random" -Tags "CI" { 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 -BeGreaterThan -1 } From 5a88ce0208aa3cc2c43b7631fcd6bcd98d72eb1f Mon Sep 17 00:00:00 2001 From: Reece Dunham Date: Thu, 14 Mar 2019 17:31:56 +0100 Subject: [PATCH 4/6] Update test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 Co-Authored-By: pougetat --- .../Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 b/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 index f08fee5ce11..eaf5036af2f 100644 --- a/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 +++ b/test/powershell/Modules/Microsoft.PowerShell.Utility/Get-Random.Tests.ps1 @@ -82,7 +82,7 @@ Describe "Get-Random DRT Unit Tests" -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 } From 299d372c376d92de2a7c92b3ebc25ad92381bb24 Mon Sep 17 00:00:00 2001 From: pougetat Date: Fri, 15 Mar 2019 09:33:49 +0100 Subject: [PATCH 5/6] Fixing PR comment --- .../commands/utility/GetRandomCommand.cs | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs index b3810652380..5f91b2a5915 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs @@ -322,8 +322,6 @@ private double GetRandomDouble(double minValue, double maxValue) while (randomNumber >= maxValue); } - Debug.Assert(minValue <= randomNumber, "lower bound <= random number"); - Debug.Assert(randomNumber < maxValue, "random number < upper bound"); return randomNumber; } @@ -370,9 +368,6 @@ private Int64 GetRandomInt64(Int64 minValue, Int64 maxValue) } while (uint64Diff <= randomUint64); double randomNumber = minValue * 1.0 + randomUint64 * 1.0; - - Debug.Assert(minValue <= randomNumber, "lower bound <= random number"); - Debug.Assert(randomNumber < maxValue, "random number < upper bound"); return (Int64)randomNumber; } @@ -402,6 +397,9 @@ protected override void BeginProcessing() } int randomNumber = Generator.Next(minValue, maxValue); + Debug.Assert(minValue <= randomNumber, "lower bound <= random number"); + Debug.Assert(randomNumber < maxValue, "random number < upper bound"); + WriteObject(randomNumber); } else if ((IsInt64(maxOperand) || IsInt(maxOperand)) && (IsInt64(minOperand) || IsInt(minOperand))) @@ -415,6 +413,9 @@ protected override void BeginProcessing() } Int64 randomNumber = GetRandomInt64(minValue, maxValue); + Debug.Assert(minValue <= randomNumber, "lower bound <= random number"); + Debug.Assert(randomNumber < maxValue, "random number < upper bound"); + WriteObject(randomNumber); } else @@ -428,6 +429,9 @@ protected override void BeginProcessing() } double randomNumber = GetRandomDouble(minValue, maxValue); + Debug.Assert(minValue <= randomNumber, "lower bound <= random number"); + Debug.Assert(randomNumber < maxValue, "random number < upper bound"); + WriteObject(randomNumber); } } @@ -631,8 +635,6 @@ public int Next(int minValue, int maxValue) randomNumber = (int)((long)(largeSample * range) + minValue); } - Debug.Assert(minValue <= randomNumber, "lower bound <= random number"); - Debug.Assert(randomNumber < maxValue, "random number < upper bound"); return randomNumber; } From e1f2383e38d163a76f297a41062573ade1978056 Mon Sep 17 00:00:00 2001 From: Ilya Date: Fri, 15 Mar 2019 22:13:48 +0100 Subject: [PATCH 6/6] Update src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs Co-Authored-By: pougetat --- .../commands/utility/GetRandomCommand.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs index 5f91b2a5915..d746027c4ca 100644 --- a/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs +++ b/src/Microsoft.PowerShell.Commands.Utility/commands/utility/GetRandomCommand.cs @@ -363,6 +363,7 @@ private Int64 GetRandomInt64(Int64 minValue, Int64 maxValue) // Randomly fill the buffer Generator.NextBytes(buffer); randomUint64 = BitConverter.ToUInt64(buffer, 0); + // Get the last 'bitsToRepresentDiff' number of random bits randomUint64 &= mask; } while (uint64Diff <= randomUint64);