Cannot evaluate expression c#

Question

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;
    }

I think you have an infinite recursion to `GetRandomColor` that is resulting in a stack overflow. Since you're creating a new `Random` each time the method is called, and `Random` is seeded based on the current time, your code keeps selecting the same color over and over again and eventually overflows the stack. — BJ Myers, May 10 '17 at 05:57
And entirely apart from the issue of re-using `Random`, if you generate too many colours, you'll always hit infinite recursion because every colour is found is `ColorsList`. This will happen even if you fix the problem with random. — Rob, May 10 '17 at 06:09
Definition for 'KnownColor' is? and '.Contains' always give array of values make sure it should not be null. — geminiousgoel, May 10 '17 at 06:27

score 2 · Answer 1 · edited May 23 '17 at 10:31

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?

score 2 · Answer 2 · edited May 23 '17 at 10:31

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;
}

score 0 · Answer 3 · answered May 10 '17 at 06:08

0

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

answered May 10 '17 at 06:08

Sowndarya

97
1
15

score 0 · Answer 4 · answered May 10 '17 at 09:59

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.

Cannot evaluate expression c#

4 Answers4