Random() is only random if you are using it right

I like the quote "With great power comes great responsibility" when used in regards to .NET - .NET gives one great powers, but use it wisely and know how this stuff works.

Recently I saw code (it was written by a guy interviewing to our team) that demonstrated interesting problem with incorrect usage of Random class. The code generated a random position for an object, tested if it satisfied some condition; if it did not work - generated another random position, tested the new position and so on until it generated something that did work. The code to generate each position was pretty simple: create new instance of Random class, use it to generate 4 random integers. The percentage of failed position was not very big (but noticeable), the code to test the position was rather fast, and we needed just 10 good positions to create complete configuration (all this was about a variant of Battleship game).

But it took very long to generate the whole configuration - about a second on average.

Playing with this code, I suddenly found that moving Random object to a member variable, so it is re-used, rather than creating new Random object for each position, made the code 10000 times faster! Well, obviously I expected some savings since it creates less objects, but ten thousand times? My first reaction - is the Random really such a heavy object?

It turned out the problem was more interesting, and caused by the usage of default constructor. As MSDN puts it, if you use default constructor the "seed is initialized to a value based on the current time." It actually uses GetTickCount() as the seed. Can you now spot the problem? The Random() constructor was very quick, but during the period of time when GetTickCount() returns the same value (which is about 20ms for most chipsets), all the Random objects created this way will have the same seed and generate the same positions! Instead of trying new positions, the code tried the same position again and again until the GetTickCount() returned a new value. So don't generate too many Random instances - just one will behave much better in most cases.