c# : loop prob with RandomClass

Question

public void machine()
{
    Random RandomClass = new Random();

    int a = RandomClass.Next(1,9);
    if(a==1)
    { 
        if (pictureBox1.Image == img0) { pictureBox1.Image = img2; }
    else 
    { 
        machine(); 
    }
}

erreur : Une exception non gérée du type 'System.StackOverflowException' s'est produite dans WindowsFormsApplication2xo.exe

Answer 1

Righ now, the probability that your loop exits is only about 10% -- it only happens when the random number between 1 and 9 is exactly 1. In all other cases, you call the function again. If this happens to often, you get the StackOverflowException. Sometimes you will be lucky and the recursion will end before that.

The obvious solution is to remove the recursion. This solves another problem with your code: every time you enter the function, you create a new random number generator. Ideally, you should create and seed it once, and call Next on that single instance every time you need a new random number.

Also, your names are slightly strange to a C# programmer's eye. Usually we name local variables with lower case and functions with upper case:

public void Machine()
{
    Random randomNumberGenerator = new Random();

    int a;
    while(true)
    { 
        a = randomNumberGenerator.Next(1,9);
        if(a == 1)
        {
          if (pictureBox1.Image == img0) { pictureBox1.Image = img2; }
          break;
        }
    }
}

Finally, this loop is not very useful. I am guessing you probably meant to extend the body to handle more numbers, in which case I would suggest a switch instead of this weird nested if-construction.

Answer 2

Since I haven't seen the right answer:

class Program
{
    private static Random rand = new Random();
    static void Main(string[] args)
    {
        machine();
    }
    private static void machine()
    {
        if (rand.Next(1, 9) == 1)
            Console.WriteLine("We did it!");
        else
            machine();
    }
}

Here you can specify a maximum depth so you don't overflow the stack.

Note: You shouldn't need to do this if you initialize your random properly.

private static void machine(int depth)
{
    if (rand.Next(1, 9) == 1 || depth > 1000)
        Console.WriteLine("We did it!");
    else
        machine(++depth);
}

Answer 3

Since you indicate in the comments, that you want to handle all the possible outcomes, you should not be using any kind of recursion or loop at all.

Then the only issue to fix, as pointed out at length, is that you have to re-use the random number generator.

Also note that the upper bound of Random.Next is exclusive:

The exclusive upper bound of the random number returned. Source

So all you need is something like

private Random randomNumberGenerator = new Random();
public void machine()
{
    int boxNumber = RandomClass.Next(1, 9);
    switch(boxNumber)
    { 
        case 1:
            if (pictureBox1.Image == img0) { pictureBox1.Image = img2; }
            // ...
            break;

        case 2:
            if (pictureBox1.Image == img0) { pictureBox1.Image = img2; }
            // ...
            break;

        // ...

        case 8: // a will be at most 8
          // ...
          break;
   }
}

Answer 4

First off, you have a parentheses mismatch. Are you trying to do:

public void machine()
{
    Random RandomClass = new Random();

    int a = RandomClass.Next(1,9);
    if(a==1)
    { 
        if (pictureBox1.Image == img0) { 
            pictureBox1.Image = img2; 
        }
    }
    else 
    { 
        machine(); 
    }
}

or this:

public void machine()
{
    Random RandomClass = new Random();

    int a = RandomClass.Next(1,9);
    if(a==1)
    { 
        if (pictureBox1.Image == img0) { 
           pictureBox1.Image = img2; 
        }
        else 
        { 
            machine(); 
        }
    }
}

As stated by many users, you don't need to keep creating a new Random object. In fact, you will be much better off with a loop than with recursion in this instance.

Here is an example:

public void machine()
    {
        Random RandomClass = new Random();
        // Not sure what the point of the random is but whatevs
        int a = RandomClass.Next(1,9);
        for (; a != 1; a = RandomClass.Next(1,9));
        if (pictureBox1.Image == img0) { 
            pictureBox1.Image = img2; 
        }
    }

Note: While highly unlikely, there is no guarantee that this function will ever end. However, using a loop rather than recursion will simply cause your computer to respond a little worse and the fan to go nuts rather than throw a Stack Overflow exception.

Hope this helps and good luck!