CODEWITHSUNDEEP
c#
Ram
Ram

Reputation: 21

Cannot evaluate expression c#

I'm getting the below error at var okColors = colors.ToArray();

Cannot evaluate expression because the current thread is in a stack overflow state.

Can you please help on this?

    private Color GetRandomColor()
    {
        Random randomGen = new Random();
        Color randomColor = Color.Red;

        KnownColor[] names = (KnownColor[])Enum.GetValues(typeof(KnownColor));
        KnownColor[] badColors = { KnownColor.AliceBlue };
        IEnumerable<KnownColor> colors = names.Except(badColors);

        var okColors = colors.ToArray();

        KnownColor randomColorName = okColors[randomGen.Next(okColors.Length)];
        randomColor = Color.FromKnownColor(randomColorName);
        if (!ColorsList.Contains(randomColor) && !randomColor.Name.Contains("Light"))
            ColorsList.Add(randomColor);
        else
            GetRandomColor();
        return randomColor;
    }

Upvotes: 0

Views: 316

Answers (4)

Prasanth V J
Prasanth V J

Reputation: 1196

The following function will work for you.

    List<Color> ColorsList = new List<Color>();
    private Color GetRandomColor(Random randomGen)
            {
                Color randomColor = Color.Red;
                KnownColor[] names = (KnownColor[])Enum.GetValues(typeof(KnownColor));
                KnownColor[] badColors = { KnownColor.AliceBlue };            
                IEnumerable<KnownColor> colors = names.Except(badColors);
                colors = colors.ToArray().Except(ColorsList.Select(x => x.ToKnownColor()));
                KnownColor[] okColors = colors.ToArray();
                KnownColor randomColorName = okColors[randomGen.Next(okColors.Length)];
                randomColor = Color.FromKnownColor(randomColorName);

                if (!ColorsList.Contains(randomColor))
                {
                    ColorsList.Add(randomColor);
                    if (okColors.Count() == 1)
                    {
                        ColorsList.Clear();
                    }
                }
                else
                {
                    GetRandomColor(randomGen);
                }

                return randomColor;
            }

To call this function

GetRandomColor(new Random())

As many stated above the issue was due to infinite recursion call of GetRandomColor function. To fix it, I have removed already fetched random colors from okColor list. Also after getting all colors, i have cleared the ColorsList to continue the randomization.

Upvotes: 0

sachin
sachin

Reputation: 2361

You have to use the same instance of Random for each recursive call, do not create a new one in each function call. If a new Random instance is created in the function every time it's called, you might keep getting the same values and not random values as you might expect.

Here is another thread you might want to look at:

randomNumber creates a new instance of Random each time... which in turn creates a new pseudo-random number generator based on the current time... which doesn't change as often as you might think.

In the linked thread, Jon Skeet also suggested against using a static Random variable.

Use the same instance of Random repeatedly... but don't "fix" it by creating a static Random variable. That won't work well either in the long term, as Random isn't thread-safe.

One option can be to pass an instance of Random as parameter to the function, this would ensure that the same Random instance is being passed along the recursive chain:

private Color GetRandomColor(Random randomGen = new Random())
{
    Color randomColor = Color.Red;

    KnownColor[] names = (KnownColor[])Enum.GetValues(typeof(KnownColor));
    KnownColor[] badColors = { KnownColor.AliceBlue };
    IEnumerable<KnownColor> okColors = names.Except(badColors).ToArray();

    KnownColor randomColorName = okColors[randomGen.Next(okColors.Length)];
    randomColor = Color.FromKnownColor(randomColorName);

    if (!ColorsList.Contains(randomColor) && !randomColor.Name.Contains("Light"))
    {
        ColorsList.Add(randomColor);
    }
    else
    {
        GetRandomColor(randomGen);
    }

    return randomColor;
}

Upvotes: 2

Sowndarya
Sowndarya

Reputation: 97

I think your code continuously hitting the else part. Please check your if()..else() condition

Upvotes: 0

Alexei Levenkov
Alexei Levenkov

Reputation: 100527

Your code throws StackOverflowException and when it happens debugger can no longer evaluate any variables resulting in error you see.

Why code have StackOverflowException:

  • code uses wrong way to generate random number and ends up with the same number for enough calls. Correct way - How do I generate a random int number in C#? - use static Random instance.

  • as result checks for .Contains constantly fail and code essentially becomes infinite recursion:

    private Color GetRandomColor()
{
  if (true) return GetRandomColor();
}

Additional reading: How do I prevent and/or handle a StackOverflowException?

Upvotes: 2

Related Questions