Skip to content

fix: add validation for alphabet random, fix performance issue and more #2

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
dryfish09 merged 1 commit into from
Jun 20, 2026
+140 −10
Merged

fix: add validation for alphabet random, fix performance issue and more #2

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
150 changes: 140 additions & 10 deletions src/main/ILibRandom.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,19 @@ public static class ILibRandom
"yellow", "white"
};

// Cache byte array for performance
[ThreadStatic]
private static byte[]? _byteBuffer;

private static byte[] GetByteBuffer()
{
if (_byteBuffer == null)
{
_byteBuffer = new byte[8];
}
return _byteBuffer;
}

/// <summary>
/// Returns a random element from the specified array
/// </summary>
Expand Down Expand Up @@ -56,9 +69,13 @@ public static int IRandomInt(int min, int max)
if (min > max)
throw new ArgumentException("min must be <= max");

#if NET6_0_OR_GREATER
return System.Random.Shared.Next(min, max + 1);
#else
Comment on lines +72 to +74

@gemini-code-assist gemini-code-assist Bot Jun 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

If max is int.MaxValue, max + 1 will overflow to int.MinValue. This causes System.Random.Shared.Next(min, max + 1) to throw an ArgumentOutOfRangeException because the upper bound is less than the lower bound.

By casting to long, we can safely calculate the range (long)max - min + 1 without any overflow, and then use NextInt64 to get a uniform random value.

#if NET6_0_OR_GREATER
            return (int)(min + System.Random.Shared.NextInt64((long)max - min + 1));
#else

double range = (double)max - (double)min;
int offset = (int)(_random.NextDouble() * (range + 1.0));
return min + offset;
#endif
}

/// <summary>
Expand Down Expand Up @@ -87,9 +104,36 @@ public static char IRandomAlphabet(char min, char max)
{
if (min > max)
throw new ArgumentException("min must be <= max");
if (!char.IsLetter(min) || !char.IsLetter(max))
throw new ArgumentException("Only alphabet characters allowed");

// Validate both characters are letters
if (!char.IsLetter(min) || !char.IsLetter(max))
throw new ArgumentException("Both min and max must be alphabet characters");

// Validate the range only contains letters
// Check if min and max are both uppercase or both lowercase
bool minIsUpper = char.IsUpper(min);
bool maxIsUpper = char.IsUpper(max);

if (minIsUpper != maxIsUpper)
throw new ArgumentException("min and max must be both uppercase or both lowercase");

// Validate range doesn't include non-letter characters
// For uppercase: A-Z only (65-90)
// For lowercase: a-z only (97-122)
int minValue = (int)min;
int maxValue = (int)max;

if (minIsUpper)
{
if (minValue < 'A' || maxValue > 'Z')
throw new ArgumentException("Range must be within A-Z for uppercase letters");
}
else
{
if (minValue < 'a' || maxValue > 'z')
throw new ArgumentException("Range must be within a-z for lowercase letters");
}

char result;
do
{
Expand Down Expand Up @@ -124,25 +168,44 @@ public static bool IRandomBool()
}

/// <summary>
/// Returns a random long between min and max
/// Returns a random long between min and max (inclusive)
/// </summary>
public static long IRandomLong(long min, long max)
{
if (min > max)
throw new ArgumentException("min must be <= max");

if (min == max)
return min;

#if NET6_0_OR_GREATER
// .NET 6+ has built-in method for long range
return System.Random.Shared.NextInt64(min, max + 1);
#else
// For older frameworks, use rejection sampling to avoid bias
ulong range = (ulong)(max - min);

// Handle full range (ulong.MaxValue)
if (range == ulong.MaxValue)
{
byte[] buf = new byte[8];
_random.NextBytes(buf);
return BitConverter.ToInt64(buf, 0);
byte[] buffer = GetByteBuffer();
_random.NextBytes(buffer);
return BitConverter.ToInt64(buffer, 0);
}

byte[] bytes = new byte[8];
_random.NextBytes(bytes);
ulong uval = BitConverter.ToUInt64(bytes, 0);

// Rejection sampling to eliminate bias
ulong limit = ulong.MaxValue - ulong.MaxValue % (range + 1);
byte[] bytes = GetByteBuffer();
ulong uval;

do
{
_random.NextBytes(bytes);
uval = BitConverter.ToUInt64(bytes, 0);
} while (uval > limit);

return min + (long)(uval % (range + 1));
#endif
Comment on lines +181 to +208

@gemini-code-assist gemini-code-assist Bot Jun 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This method has two issues:

  1. If max is long.MaxValue, max + 1 will overflow to long.MinValue, causing System.Random.Shared.NextInt64 to throw an ArgumentOutOfRangeException.
  2. The rejection sampling loop condition uval > limit accepts limit + 1 values, which is not a multiple of range + 1, introducing a tiny bias. Changing it to uval >= limit ensures perfect uniformity.

For .NET 6+, we can handle the max == long.MaxValue case using Span<byte> and stackalloc to avoid any thread-static buffer access or array overhead.

#if NET6_0_OR_GREATER
            if (max < long.MaxValue)
            {
                return System.Random.Shared.NextInt64(min, max + 1);
            }

            ulong range = (ulong)(max - min);
            if (range == ulong.MaxValue)
            {
                Span<byte> buffer = stackalloc byte[8];
                System.Random.Shared.NextBytes(buffer);
                return BitConverter.ToInt64(buffer);
            }

            ulong limit = ulong.MaxValue - ulong.MaxValue % (range + 1);
            Span<byte> bytes = stackalloc byte[8];
            ulong uval;
            do
            {
                System.Random.Shared.NextBytes(bytes);
                uval = BitConverter.ToUInt64(bytes);
            } while (uval >= limit);

            return min + (long)(uval % (range + 1));
#else
            ulong range = (ulong)(max - min);
            
            if (range == ulong.MaxValue)
            {
                byte[] buffer = GetByteBuffer();
                _random.NextBytes(buffer);
                return BitConverter.ToInt64(buffer, 0);
            }
            
            ulong limit = ulong.MaxValue - ulong.MaxValue % (range + 1);
            byte[] bytes = GetByteBuffer();
            ulong uval;
            
            do
            {
                _random.NextBytes(bytes);
                uval = BitConverter.ToUInt64(bytes, 0);
            } while (uval >= limit);
            
            return min + (long)(uval % (range + 1));
#endif

}

/// <summary>
Expand All @@ -156,6 +219,50 @@ public static double IRandomDouble(double min = 0.0, double max = 1.0)
return min + (_random.NextDouble() * (max - min));
}

/// <summary>
/// Returns a random decimal between min and max
/// </summary>
public static decimal IRandomDecimal(decimal min, decimal max)
{
if (min > max)
throw new ArgumentException("min must be <= max");

if (min == max)
return min;

// Get a random double and convert to decimal
// Using 28-29 digits of precision (maximum for decimal)
double randomDouble = _random.NextDouble();
decimal randomDecimal = (decimal)randomDouble;

// Scale to the range
decimal range = max - min;
return min + (randomDecimal * range);
}

/// <summary>
/// Returns a random decimal between min and max with specified precision
/// </summary>
public static decimal IRandomDecimal(decimal min, decimal max, int precision)
{
if (min > max)
throw new ArgumentException("min must be <= max");

if (precision < 0 || precision > 28)
throw new ArgumentException("precision must be between 0 and 28");

if (min == max)
return min;

// Generate random integer with specified precision
long multiplier = (long)Math.Pow(10, precision);
long minScaled = (long)(min * multiplier);
long maxScaled = (long)(max * multiplier);

long randomScaled = IRandomLong(minScaled, maxScaled);
return randomScaled / (decimal)multiplier;
Comment on lines +257 to +263

@gemini-code-assist gemini-code-assist Bot Jun 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

Using Math.Pow(10, precision) and casting to long will overflow when precision >= 19 (since 10^19 exceeds long.MaxValue). Additionally, min * multiplier or max * multiplier can easily overflow long even for smaller precisions if the values are large.

A much safer and simpler approach is to generate the random decimal using IRandomDecimal(min, max) and then round it to the desired precision using Math.Round.

            decimal randomValue = IRandomDecimal(min, max);
            return Math.Round(randomValue, precision, MidpointRounding.AwayFromZero);

}

/// <summary>
/// Returns a random item from a list
/// </summary>
Expand Down Expand Up @@ -190,5 +297,28 @@ public static string IRandomConsoleColor()
{
return IRandomFromArray(ConsoleColors);
}

/// <summary>
/// Returns a random element from an enumeration
/// </summary>
public static T IRandomEnum<T>() where T : Enum
{
var values = Enum.GetValues(typeof(T));
return (T)values.GetValue(_random.Next(values.Length))!;
}

/// <summary>
/// Returns a random element from an enumeration with exclusion
/// </summary>
public static T IRandomEnum<T>(T exclude) where T : Enum
{
var values = Enum.GetValues(typeof(T));
T result;
do
{
result = (T)values.GetValue(_random.Next(values.Length))!;
} while (result.Equals(exclude));
return result;
}
Comment on lines +301 to +322

@gemini-code-assist gemini-code-assist Bot Jun 20, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

This implementation has two main issues:

  1. Calling Enum.GetValues(typeof(T)) on every invocation is very slow because it uses reflection and allocates a new array each time. We can cache the values in a generic static class EnumCache<T> to avoid allocations and reflection overhead.
  2. In IRandomEnum<T>(T exclude), if the enum has only one value and it is excluded (or if all values are excluded), the do-while loop will run infinitely, causing a denial of service. We should validate that there is at least one non-excluded value before looping.
        private static class EnumCache<T> where T : Enum
        {
            public static readonly T[] Values = (T[])Enum.GetValues(typeof(T));
        }

        /// <summary>
        /// Returns a random element from an enumeration
        /// </summary>
        public static T IRandomEnum<T>() where T : Enum
        {
            var values = EnumCache<T>.Values;
            if (values.Length == 0)
                throw new ArgumentException("Enum has no values");
            return values[_random.Next(values.Length)];
        }

        /// <summary>
        /// Returns a random element from an enumeration with exclusion
        /// </summary>
        public static T IRandomEnum<T>(T exclude) where T : Enum
        {
            var values = EnumCache<T>.Values;
            if (values.Length == 0)
                throw new ArgumentException("Enum has no values");

            bool hasOther = false;
            foreach (var val in values)
            {
                if (!val.Equals(exclude))
                {
                    hasOther = true;
                    break;
                }
            }
            if (!hasOther)
                throw new ArgumentException("All values in the enum are excluded");

            T result;
            do
            {
                result = values[_random.Next(values.Length)];
            } while (result.Equals(exclude));
            return result;
        }

}
}