[C#] Inconsistent form output...

[DDoSAttack]

Hello all,

I am confused by this issue...

Below is some quick code I threw together to test some random number generation and output. The method in the genNum class simply create a string of 9 unique random numbers from 1 to 9 and returns it.

On my button click event it creates 3 separate instances of the genNum class (g1, g2, g3) and then outputs the string to the corresponding textBox (i.e. g1 => textBox1)

Now if I simply run this code without breakpoints, when I click the button it fills the three textbox controls with the same value. I have attached an image of this occurring.
However, if I run it with a breakpoint on, let's say set on the button click, and step through all the code it runs just fine meaning that in the end it displays 3 separate strings/

This is pretty consistent for me. Every once in awhile (1 out of 1000 times) it will display a separate value in one of the textboxes, usually the last one if I run it without breakpoints. It is 100% fine if I run it with breakpoints.

Here is the code...


namespace TESTingRndGen
{
public partial class Form1 : Form
{
public Form1()
{
InitializeComponent();
}

private void button1_Click(object sender, EventArgs e)
{
genNum g1 = new genNum();
genNum g2 = new genNum();
genNum g3 = new genNum();

textBox1.Text = g1.GenerateStringofNumbers();
textBox2.Text = g2.GenerateStringofNumbers();
textBox3.Text = g3.GenerateStringofNumbers();
}
}

class genNum
{
public string GenerateStringofNumbers()
{
Random rnd = new Random();
string result = "";
int _randomNumHolder;

// Generate 9 random numbers
for(int x = 0; x < 9; x++)
{
_randomNumHolder = rnd.Next(1, 10);
if(x == 0)
result = _randomNumHolder.ToString();
else
{
if(result.Contains(_randomNumHolder.ToString()))
{
--x;
}
else
{
result += _randomNumHolder.ToString();
}
}
}

return result;
}
}
}

Any insight into this would be greatly appreciated!

[DDoSAttack]

*Note: Upon reflection I probably should have just added this to my earlier post. Sorry about that!

Ok now this is weird!

I was messing with something else and it did the same thing so I tried a couple of things to see what it would do.

It turns out that if you add Thread.Sleep(10) it pauses just long enough to add the values in properly.

In other words, it seems as if it was going too fast!:stare:

If that is true, well that would just be plain disturbing!

The altered code would look like...


textBox1.Text = g1.GenerateStringofNumbers();
System.Threading.Thread.Sleep(10);
textBox2.Text = g2.GenerateStringofNumbers();
System.Threading.Thread.Sleep(10);
textBox3.Text = g3.GenerateStringofNumbers();
System.Threading.Thread.Sleep(10);

[kirupa]

That's because Random isn't exactly threadsafe, so it has a tendency to act up when other things are going on. When you attempt to lock random as in:


lock (rnd)
{
_randomNumHolder = rnd.Next(1, 10);
}
...does your problem get resolved?

Cheers!
Kirupa =)

[DDoSAttack]

That's because Random isn't exactly threadsafe, so it has a tendency to act up when other things are going on. When you attempt to lock random as in:


lock (rnd)
{
_randomNumHolder = rnd.Next(1, 10);
}
...does your problem get resolved?

Cheers!
Kirupa =)


DUDE! You rock... HARD :thumb:

While that was only part of the solution, it helped immensely in starting me down the proper path.

As it turns out, like you said, the Random class is not threadsafe. This is an issue because apparently when it runs the first one executes just fine but the others kinda wander off and do their own thing usually corrupting the seed* with the first instance of the Random class. This is what causes all the results to be the same. *Note - This is a normal occurrence with C#'s Random class. A number generated using random.Next() isn't really a random number per say as it is generated using the first random number, which is a true random, as the seed for the next number and so on.

BUT using 'lock', locks the particular section of code down until it completes execution. I was essentially doing this same thing when I made the thread go to sleep.

The catch with lock is that it cannot be called for a public object (MSDN (http://msdn2.microsoft.com/en-us/library/c5kehkcz.aspx)). So just adding that lock in there doesn't produce the desired results. However I moved the Random instance outside the method and declared it private static and all works perfectly.

The new code would look like this...


class genNum
{
private static Random rnd = new Random(); // Declared private static

public string GenerateStringofNumbers()
{
int _randomNumHolder;
string result = "";

lock(rnd) // Lock down the call to create a random number
{
_randomNumHolder = rnd.Next(1, 10);
}

// Generate 9 random numbers
for(int x = 0; x < 9; x++)
{
_randomNumHolder = rnd.Next(1, 10);
if(x == 0)
result = _randomNumHolder.ToString();
else
{
if(result.Contains(_randomNumHolder.ToString()))
{
--x;
}
else
{
result += _randomNumHolder.ToString();
}
}
}

return result;
}

}

[kopo.fett]

I dont think it's the lock. The Random class isn't threadsafe but declaring it with static modifier makes it threadsafe. If you remove the lock it should still be random and not similar string results

[Jeff Wheeler]

Just to clarify, you mentioned in your orange note that the first number is "true random," which isn't exactly true. It's based off the seed number, which is no more random. All numbers here are pseudo-random.

See Pseudorandom number generator and Random number generator.

[DDoSAttack]

Just to clarify, you mentioned in your orange note that the first number is "true random," which isn't exactly true. It's based off the seed number, which is no more random. All numbers here are pseudo-random.

See Pseudorandom number generator and Random number generator.

I thought the first number generated is not based off a seed?:blush:


I dont think it's the lock. The Random class isn't threadsafe but declaring it with static modifier makes it threadsafe. If you remove the lock it should still be random and not similar string results
I didn't even think of that! Thanks for catching that :trout:

[Jeff Wheeler]

I thought the first number generated is not based off a seed?:blush:

Sorry, I meant to say that I figured you knew what you were talking about, but I was clarifying for anybody that might read the thread in the future, or to double-check that's what you meant.