Generating a list of unique random longs

Question

I'm using the Bogus for .NET: C#, F#, and VB.NET library.

What would be a more efficient way to generate a list of unique random longs?

public IEnumerable<long> GenerateRandomNumbers(int count = 1)
{
      var f = new Faker();
      List<long> temporaryList = new List<long>();
                
      for (int i = 0; i < 10000; i++)
      {                
          var number = f.Random.Long(5000000000, 8000000000);
          temporaryList.Add(number );
      }
    
      return temporaryList.Distinct().Take(count).ToList();
}

Answer 1

I have to say that the 10000 iterations loop is both inefficient and incorrect. It always generate 10000 numbers no matter how many you need, so if you need less than 10000 you likely will throw away a good part of them (that's inneficient). And if you request more than 10000, you only get at most 10000 (that's a bug). The actual impact depends on your usage.

I would suggest to do what's suggested in comments: use a HashSet to store unique numbers and fill it up to until you've reached the count asked for, that way you generate the bare minimum required and nothing more than just to avoid possible duplicates.

Change it to something like this:

public IEnumerable<long> GenerateRandomNumbers(int count = 1)
{
    var f = new Faker();
    //Initialize the HashSet internal size to the final size (note the constructor parameter)
    //This prevents realocations as we add numbers.
    HashSet<long> numbers = new HashSet<long>(count);

    //Keep adding numbers until we reach the asked count
    //This ensures that we return no less than requested and we don't generate more numbers than needed
    while (numbers.Count < count)
    {
        var number = f.Random.Long(5000000000, 8000000000);
        //Add the generated number to the HashSet
        //It prevents duplicates and don't adds it if it happens to be a dupe
        //That's why we check the count every time
        numbers.Add(number);
    }
    
    return numbers;
}

That'll improve its efficiency and fixes the bug.

But we can still improve it somewhat more, by using another feature of C#, the iterator blocks. By using yield return we only generate numbers as they're consumed by calling code, so if the caller ask for 10 numbers and only iterates over 3 of them, we generate enough random numbers for returning 3, and not 10. This is the very same thing LINQ does all over the place, stream its results. Here is a possible implementation:

public IEnumerable<long> GenerateRandomNumbers(int count = 1)
{
    var f = new Faker();
    //We return numbers immediately, but keep track of them only to avoid duplicates
    HashSet<long> numbers = new HashSet<long>(count);

    //Only keep returning until we've meet the number asked for
    while (numbers.Count < count)
    {
        var number = f.Random.Long(5000000000, 8000000000);
        //Yield the generated value only if not seen before, we need to know the result of the `Add` operation for this
        bool added = numbers.Add(number);
        if (added) yield return number;
    }
}

As a small improvement, slightly unrelated to the question, you may want to add the argument validation in the method, so that if a negative count is passed, you immediately error out with an ArgumentOutOfRangeException.

Answer 2

I doubt that algorithmic efficiency can be improved but you may consider an implementation similar to this which leverages idiomatic LINQ.

public IEnumerable<long> GenerateRandomNumbers()
{
    var f = new Faker();

    return 
        Enumerable.Range(0, int.MaxValue)
        .Select(
            _ => f.Random.Long(5000000000, 8000000000))
        .Distinct();
}