Runtime: The documentation for Enumerable.Intersect is incorrect about the order of iteration

Created on 15 Dec 2016  路  10Comments  路  Source: dotnet/runtime

The MSDN documentation for Enumerable.Intersect says:

When the object returned by this method is enumerated, Intersect\ enumerates first, collecting all distinct elements of that sequence. It then enumerates second, marking those elements that occur in both sequences. Finally, the marked elements are yielded in the order in which they were collected.

Consider this program:

```c#
using System;
using System.Collections.Generic;
using System.Linq;

class Program
{
static void Main()
{
foreach (int i in First().Intersect(Second()))
{
Console.WriteLine($"Got {i}.");
}
}

static IEnumerable<int> First()
{
    for (int i = 1; i <= 2; i++)
    {
        Console.WriteLine($"Yielding {i} from First.");
        yield return i;
    }
}

static IEnumerable<int> Second()
{
    for (int i = 1; i <= 2; i++)
    {
        Console.WriteLine($"Yielding {i} from Second.");
        yield return i;
    }
}

}

Given the above description, I would expect the output to be:

Yielding 1 from First.
Yielding 2 from First.
Yielding 1 from Second.
Yielding 2 from Second.
Got 1.
Got 2.

But the actual output (on both .Net Framework and .Net Core) is:

Yielding 1 from Second.
Yielding 2 from Second.
Yielding 1 from First.
Got 1.
Yielding 2 from First.
Got 2.
```

I believe this is an error in documentation: it should not promise in what order are first and second enumerated, only in what order are the items yielded from the resulting sequence. For example:

When the object returned by this method is enumerated, Intersect\ yields distinct elements occurring in both sequences in the order in which they appear in first.

area-System.Linq documentation

All 10 comments

I think it may have been describing the observable behaviour, but only considering the results given as what is observable.

Whatever the reason, I agree there shouldn't be a promise made there, though perhaps the entailed promise that the items are ordered as per their order in first should be spelled out instead now that it's not an implication of the removed promise.

@JonHanna I am not sure I fully understand your answer (sorry!), so let me ask you explicitly -- do you agree with @svick's proposed wording? Or do you propose changes to the new wording?

MSDN topic: Enumerable.Intersect

Old wording:

When the object returned by this method is enumerated, Intersect\ enumerates first, collecting all distinct elements of that sequence. It then enumerates second, marking those elements that occur in both sequences. Finally, the marked elements are yielded in the order in which they were collected.

New wording:

When the object returned by this method is enumerated, Intersect\ yields distinct elements occurring in both sequences in the order in which they appear in first.

Yes, that was a long-winded (a side-effect of thinking about things while I type instead of before I type) way of say "+1".

Thanks for confirmation @JonHanna.
@OmarTawfik @VSadov any opinion on the docs changes above? Based on the facts presented here it sounds reasonable to me ...

I agree. This promise shouldn't be made even if it was the observable behavior.

OK, looks like we have agreement on the docs change by area experts - see https://github.com/dotnet/corefx/issues/14541#issuecomment-267653927 for suggested wording change.

cc @mairaw

Thanks @karelz! Change is made for this page Enumerable.Intersect.
Should I do the same change to the other overload too?

I think so, yes. The two overloads are the same in this regard.

Thanks @mairaw for fast turnaround and for catching the overloads (again)! :)

That's what I thought, but I always prefer to double check. 馃槃
Both are fixed now! Thanks for also providing the suggested fix.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

jzabroski picture jzabroski  路  3Comments

omariom picture omariom  路  3Comments

aggieben picture aggieben  路  3Comments

matty-hall picture matty-hall  路  3Comments

sahithreddyk picture sahithreddyk  路  3Comments