Why doesn't this generate the lucky numbers?

Question

I am trying to write a function which generates the lucky numbers,

static IEnumerable<int> LuckyNumbers()
{
  IEnumerable<int> luckyNumbers = Enumerable.Range(1, int.MaxValue);
  int counter = 1;
  while (true)
  {
    int number = luckyNumbers.ElementAt(counter++);
    yield return number;
    luckyNumbers = luckyNumbers.Where((_, index) => (index + 1) % number != 0);
  }
}

but this generates:

2,5,7,11,13,17,21,...

which are not the lucky numbers.

Why doesn't my code work? I am trying to:

1. start with all the natural numbers:

   IEnumerable<int> luckyNumbers = Enumerable.Range(1, int.MaxValue);
   int counter = 1;
   

2. iterate through them and return the next lucky number:

   while (true)
   {
     int number = luckyNumbers.ElementAt(counter++);
     yield return number;
   

3. remove all nth numbers from the sequence:

   luckyNumbers = luckyNumbers.Where((_, index) => (index + 1) % number != 0);
   

I don't see why this doesn't work as I intend.

Answer 1

There are a couple of reasons why your code does not work:

1. Collections in programming are zero-indexed. This is why the first number you generate is 2, because its the number with index 1. You should rather initialize counter to 0.
2. I think you initialized counter to 1 to avoid canceling the 1-th number out of the sequence (which will effectively kill all numbers and therefore taking the element at a given position is doomed to fail). The problem lies in the definition of the lucky numbers here: While the first lucky number is 1, the first iteration is to strike every second number. So you have to take Math.Min(number, 2).

Finally, you arrive at the following:

    static IEnumerable<int> LuckyNumbers()
    {
        IEnumerable<int> luckyNumbers = Enumerable.Range(1, int.MaxValue);
        int counter = 0;
        while (true)
        {
            int number = luckyNumbers.ElementAt(counter++);
            yield return number;
            int moduloCheck = Math.Max(number, 2);
            luckyNumbers = luckyNumbers.Where((_, index) => (index + 1) % moduloCheck != 0);
        }
    }

From a performance perspective, though, I think the solution is horrible for large numbers as you will repeatedly check the first numbers forever at the ElementAt. Because a where-expression is not indexable, this will always start checking each number for several where conditions. The good thing though is that you can simply use it as LuckyNumbers().Take(n) to get the first n lucky numbers.