Skip to content

Fix#1

Merged
dryfish09 merged 5 commits into
mainfrom
fix
May 31, 2026
Merged

Fix#1
dryfish09 merged 5 commits into
mainfrom
fix

Conversation

@dryfish09

Copy link
Copy Markdown
Owner

No description provided.

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request updates the project to target multiple frameworks, configures package metadata, and introduces new random generation utilities such as GUID, hex color, and console color generators. The review feedback highlights several critical areas for improvement, including resolving thread-safety issues with System.Random in multi-threaded environments, preventing integer and long overflows during range calculations, adding validation to ensure min <= max, fixing IRandomAlphabet to avoid returning non-letter characters, and caching the console colors array to eliminate redundant allocations.

Comment thread src/main/ILibRandom.cs
if (list == null || list.Count == 0)
throw new ArgumentException("List cannot be null or empty");
return list[_random.Next(list.Count)];
private static readonly System.Random _random = new System.Random();

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

System.Random is not thread-safe. Concurrent access from multiple threads can cause it to enter a state where it always returns 0. Since ILibRandom is a static utility class, it is highly likely to be used in multi-threaded contexts. We can use conditional compilation to use System.Random.Shared on .NET 6+ and [ThreadStatic] on older frameworks.

        #if NET6_0_OR_GREATER
        private static System.Random _random => System.Random.Shared;
        #else
        [ThreadStatic]
        private static System.Random? _localRandom;
        private static System.Random _random => _localRandom ??= new System.Random();
        #endif

Comment thread src/main/ILibRandom.cs
Comment on lines +46 to +49
public static int IRandomInt(int min, int max)
{
return _random.Next(min, max + 1);
}

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 overflows to int.MinValue, causing an ArgumentOutOfRangeException. Also, there is no validation that min <= max. We can resolve both issues by validating the input and using double arithmetic to calculate the range and offset safely.

        public static int IRandomInt(int min, int max)
        {
            if (min > max)
                throw new ArgumentException("min must be <= max");

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

Comment thread src/main/ILibRandom.cs
Comment on lines +111 to +117
public static long IRandomLong(long min, long max)
{
if (min > max) throw new ArgumentException("min must be <= max");
long range = max - min;
long rand = (long)(_random.NextDouble() * (range + 1));
return min + rand;
}

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

max - min can overflow long if min is negative and max is positive. Additionally, _random.NextDouble() only has 53 bits of precision, which leads to non-uniform distribution and skipped values for ranges larger than 2^53. We can use ulong to safely compute the range without overflow, and use NextBytes to get a full 64-bit random value.

        public static long IRandomLong(long min, long max)
        {
            if (min > max) throw new ArgumentException("min must be <= max");

            ulong range = (ulong)(max - min);
            if (range == ulong.MaxValue)
            {
                byte[] buf = new byte[8];
                _random.NextBytes(buf);
                return BitConverter.ToInt64(buf, 0);
            }

            byte[] bytes = new byte[8];
            _random.NextBytes(bytes);
            ulong uval = BitConverter.ToUInt64(bytes, 0);
            return min + (long)(uval % (range + 1));
        }

Comment thread src/main/ILibRandom.cs
Comment on lines +65 to +68
public static char IRandomChar(char min, char max)
{
return (char)_random.Next(min, max + 1);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If min > max, the method will throw an ArgumentOutOfRangeException from _random.Next. Adding a guard clause to validate that min <= max ensures consistency with other methods and provides a clearer error message.

        public static char IRandomChar(char min, char max)
        {
            if (min > max)
                throw new ArgumentException("min must be <= max");
            return (char)_random.Next(min, max + 1);
        }

Comment thread src/main/ILibRandom.cs
Comment on lines +76 to +82
public static char IRandomAlphabet(char min, char max)
{
if (!char.IsLetter(min) || !char.IsLetter(max))
throw new ArgumentException("Only alphabet characters allowed");

return (char)_random.Next(min, max + 1);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

If the range between min and max spans across the gap between uppercase and lowercase letters (e.g., IRandomAlphabet('A', 'z')), the method can return non-letter characters like [, \, ], ^, _, or ` because it simply selects a random character in the range without verifying if the generated character is actually a letter. We should use rejection sampling to ensure only letters are returned, and add a validation check for min > max.

        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");
                
            char result;
            do
            {
                result = (char)_random.Next(min, max + 1);
            } while (!char.IsLetter(result));
            
            return result;
        }

Comment thread src/main/ILibRandom.cs
Comment on lines +160 to +167
public static string IRandomConsoleColor()
{
string[] colors = { "black", "darkblue", "darkgreen", "darkcyan", "darkred",
"darkmagenta", "darkyellow", "gray", "grey", "darkgray",
"darkgrey", "blue", "green", "cyan", "red", "magenta",
"yellow", "white" };
return IRandomFromArray(colors);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

The colors array is allocated on every call to IRandomConsoleColor(). Moving it to a private static readonly field avoids unnecessary allocations and improves performance.

        private static readonly string[] ConsoleColors = {
            "black", "darkblue", "darkgreen", "darkcyan", "darkred", 
            "darkmagenta", "darkyellow", "gray", "grey", "darkgray", 
            "darkgrey", "blue", "green", "cyan", "red", "magenta", 
            "yellow", "white"
        };

        public static string IRandomConsoleColor()
        {
            return IRandomFromArray(ConsoleColors);
        }

@dryfish09 dryfish09 merged commit af74067 into main May 31, 2026
2 checks passed
@dryfish09 dryfish09 deleted the fix branch May 31, 2026 22:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant