-9

I know there are many questions on the net about it ,but I would like to know why my method fails What Am i Doing wrong?

public class Generator
{

    private static readonly Random random = new Random();

    private static readonly object SyncLock = new object();

    public static int GetRandomNumber(int min, int max)
    {
        lock (SyncLock)
        { 
            return random.Next(min, max);
        }
    }

}


[TestFixture]
public class Class1
{
    [Test]
    public void SimpleTest()
    {

        var numbers=new List<int>();
        for (int i = 1; i < 10000; i++)
        {
            var random = Generator.GetRandomNumber(1,10000);
            numbers.Add(random);
        }

        CollectionAssert.AllItemsAreUnique(numbers);

    }
}

EDIT Test method is failing!! Sorry for not mentioning

Thanks for your time and suggestions

user9969
  • 15,632
  • 39
  • 107
  • 175
  • What is not working? Is it a compiler error? Does the function always return the same number? – Msonic Mar 06 '12 at 21:26
  • 2
    @Boo i got scared when your comment popped up ... :O – Alex Mar 06 '12 at 21:26
  • 3
    The recent influx of carelessly posed questions in enormous. – usr Mar 06 '12 at 21:26
  • 1
    Are you looking for some sort of shuffling algorithm that will give you a random redistribution of the numbers 1-10000? – Chris Farmer Mar 06 '12 at 21:32
  • +1 because he mixed random with unique. I'm sure he's not the only one. And a question cannot be so bad if the answers are upvoted insomuch. – Tim Schmelter Mar 06 '12 at 21:40
  • 1
    @TimSchmelter: Whah? He posted code and said basically "it doesn't work." It's the awesomeness of the people that bothered to interpret and answer the "question" that makes the answers so good. It has zero to do with the quality of this question. – Chris Farmer Mar 06 '12 at 21:43
  • @ChrisFarmer: Actually the solution was quite obvious with these few lines of code. He generates random numbers and asserts that they are unique(what's already stated in the title). – Tim Schmelter Mar 06 '12 at 21:45
  • 1
    @TimSchmelter: the OP claims that "the method fails" without specifying which method and in what manner it fails. If they had said that the test fails or the assert is tripped or something that accurately described the failure then I would agree with you. But there was no description of the failure beyond a "method", of which I count two that the OP owns and 6 that they do not own - any one of which could be "failing". – Sam Axe Mar 06 '12 at 21:51
  • 1
    I guess I agree with you there, but I guess we'll just have to disagree about the quality issue. :) The fact that the problem in the code wasn't particularly hard to discover doesn't rescue the question from the fact that no real question was actually asked and no expectations of what he hoped to see were asserted outside of the code itself. – Chris Farmer Mar 06 '12 at 21:52
  • My upvote was only a balance vote because i don't like how people downvote excessively only on formal aspects without even trying to understand the code itself. But if people don't add code they'll be downvoted due to this fact. **Code can be self-explanatory**. But don't midunserstand me, you're right, in general questions should be clearer. – Tim Schmelter Mar 06 '12 at 22:03
  • Why do you expect no repeats in 10000 random items? Random does not guarentee uniqueness. – Chriseyre2000 Mar 06 '12 at 21:28
  • @ChrisFarmer sorry for not being clear.Tired may be.I am working on a database in this company and they generate some unique numbers using the random.I cannot change the rest of the code but I need to avoid to generate same numbers.I thought by putting a lock and having only once instance of random would do it.Missing something for sure – user9969 Mar 06 '12 at 22:28

5 Answers5

14

How can you possibly expect a sequence of 10,000 random numbers from a set of 10,000 possible values to be all unique unless you are extremely lucky? What you are expecting is wrong.

Flip a coin twice. Do you truly expect TH and HT to be the only possible sequences?

What makes you think random numbers should work any differently?

This output from a random number generator is possible:

1, 1, 1, 1, 1, 1, ..., 1

So is this:

1, 2, 3, 4, 5, 6, ..., 10000

In fact, both of those sequences are equally likely!

jason
  • 236,483
  • 35
  • 423
  • 525
  • The row `1..10000` contains 10k unique numbers. Why is it impossible? – zerkms Mar 06 '12 at 21:26
  • 9
    I said "unless you're extremely lucky." – jason Mar 06 '12 at 21:28
  • 1
    @zerkms - Agreed. Not impossible. But you can be fairly confident that if you ran this program once a second for the lifetime of the universe that you'd never get a set without duplicates. This is a variant on the birthday problem: http://en.wikipedia.org/wiki/Birthday_problem – perfectionist Mar 06 '12 at 21:30
  • @perfectionist, Jason: oops, I missed the "unless you're extremely lucky" part – zerkms Mar 06 '12 at 21:32
  • @Jason .you are right I cannot go in all the details of the project but I need to produce unique numbers between 1 and 10000. – user9969 Mar 06 '12 at 22:33
2

Random != Unique

The point here is that your code should model your problem and yours really doesn't. Random is not equal to unique. If you want unique, you need to get your set of values and shuffle them.

If you truly want random numbers, you can't expect them to be unique. If your (P)RNG offers an even distribution, then over many trials you should see similar counts of every value (see the Law of Large Numbers). Cases may show up that seem "wrong" but you can't discount that you hit that case by chance.

Austin Salonen
  • 49,173
  • 15
  • 109
  • 139
  • -1 N unique numbers can be generated in O(n). OrderBy would run in O(n*log(n)) http://en.wikipedia.org/wiki/Fisher%E2%80%93Yates_shuffle – L.B Mar 06 '12 at 21:32
  • 1
    `Guid`s are guaranteed to be unique, but there's no guarantee that they produce values sufficient for shuffling a sequence. If it's a shuffle that you want, use a shuffling algorithm. – jason Mar 06 '12 at 21:35
  • 2
    It is ironic that your answer violates the principle stated in your first sentence. Jason is correct. **Guids are a source of uniqueness, not a source of randomness**. There is no guarantee whatsoever that you don't get ten thousand guids that are *already sorted in order*; in fact, there are some versions of the guid generation algorithm that do exactly that. – Eric Lippert Mar 06 '12 at 21:40
2

You seem to be under the misapprehension that the Random class generates a sequence of unique, though apparently random numbers. This is simply not the case; randomness implies that the next number could be any of the possible choices, not just any except one I've seen before.

That being the case, it is entirely unsurprising that your test fails: the probability that 10000 randomly generated integers (between 1 and 10000 no less) are unique is minuscule.

dlev
  • 48,024
  • 5
  • 125
  • 132
0

I think you failed to mention that your test method is failing.

It is failing because your random generator is not producing Unique numbers. I'm not sure how it would under it's current condition.

Erik Philips
  • 53,428
  • 11
  • 128
  • 150
0
public static void FisherYatesShuffle<T>(T[] array)
{
    Random r = new Random();
    for (int i = array.Length - 1; i > 0; i--)
    {
        int j = r.Next(0, i + 1);
        T temp = array[j];
        array[j] = array[i];
        array[i] = temp;
    }
}

int[] array = new int[10000];

for (int i = 0; i < array.Length; i++) array[i] = i;
FisherYatesShuffle(array);
L.B
  • 114,136
  • 19
  • 178
  • 224
  • @user231465 just be to able to shuffle any kind of array, not just integer arrays – L.B Mar 07 '12 at 07:58