Why does Single() not return directly when more than one element is found?

Question

I found (roughly) this code in the Enumerable.Single method while inspecting it with some decompiler:

foreach (TSource current in source)
{
    if (predicate(current))
    {
        result = current;
        num += 1L;
    }
}

if (num > 1L)
{
     throw Error.MoreThanOneMatch();
}

As you can see, it loops over all items before throwing. Why doesn't it break when num > 1?

Thank you for this question. It's really interesting why LINQ team decided to loop till the end anyway here. — Chuck Norris, Jul 19 '13 at 10:10
Vince, is there any difference between Enumerable.Single and Queryable.Single implementations at this part? — Chuck Norris, Jul 19 '13 at 10:14
Where did you find this implementation? Using Reflector, I see something completely different, which *does* return immediately if a second element is found. — Thomas Levesque, Jul 19 '13 at 10:29
Performancewise this might be not perfect. But at least the outcome is predictable. The predicate is executed for any element so if the predicate has sideeffects it does it predictable for all elements. If Single breaks at the second hit its hard to tell for what elements the predicate has been executed. So i would say this is more secure in some cases but less performing in most cases. — Ralf, Jul 19 '13 at 10:40
@ThomasLevesque in `Microsoft.NET\Framework\v4.0.30319\System.Core.dll` I see (something like) the code in the question. — CodeCaster, Jul 19 '13 at 10:46

Sergey Berezovskiy · Answer 1 · 2013-07-19T11:34:34.950

Agree, that it will be better from terms of performance (EDIT: if we are expecting more than one item matching our predicate, which we should not do):

foreach (TSource current in source)
{
    if (predicate(current))
    {
        result = current;
        num += 1L;

        if (num > 1L)
            throw Error.MoreThanOneMatch();
    }
}

if (num == 0L)
   throw Error.NoMatch();

return local;

Looks like they decided to make results analyzing more clear and separated it from enumerating source. But then I wonder why simple switch was not used:

switch((int)num)
{
   case 0: throw Error.NoMatch();
   case 1: return local;
   default:
       throw Error.MoreThanOneMatch();    
}

Regarding to performance issues - I think it's assumed that Single should be called when you are really expecting single result. Zero or more results is an exceptional path, which should not occur often (as any exception). So, it's more your program's logic error if source contain many items matching predicate.

I guess your switch solution needs to enumerate the whole source. But the current solution with if throws exception immediatelly when it knows there is some duplicity. — Ondra, Jul 19 '13 at 10:17
@Ondra current solution does not throw exception immediately. Take a look on source code. And my switch block is a refactored `num` analyzing block from original implementation. This switch should be placed after enumerating — Sergey Berezovskiy, Jul 19 '13 at 11:28

metadings · Answer 2 · 2013-07-19T11:04:28.443

0

By Single is meant, exactly one, not none and also not more than one.
It enumerates all items, to ensure that it's only one.
It throws an exception, if there is none or more than one.
SingleOrDefault instead throws if there are more, but returns default(T)/null if there is none.

What you're looking for is FirstOrDefault that breaks the enumeration, if it found the first one matching the predicate. First instead throws if there is none, and also breaks (directly returns from) it's foreach, if it found the first one.

FirstOrDefault's source

foreach (TSource current in source)
{
    if (predicate(current))
    {
        return current;
    }
}
return default(TSource);

Whereas First's source is instead of return default

throw Error.NoMatch();

edited Jul 19 '13 at 11:04

answered Jul 19 '13 at 10:15

metadings

3,798
2
28
37

This was my initial thought but the question is why, if 2 matching elements are found, is an exception not thrown at that point rather than iterating through the rest of the list and then throwing the exception? – keyboardP Jul 19 '13 at 10:18
Sorry my bad, `SingleOrDefault` does never throw, it returns null if there are more or none. `Single` is the one which throws. – metadings Jul 19 '13 at 10:20
1

My guess is: They wanted the iteration to finish before throwing and exception. If they would throw an exception in the middle of enumeration, it could cause some side-effect - eg your Enumerator.Next downloads an item from the internet and with the exception the connection is not properly closed – Ondra Jul 19 '13 at 10:23
@Ondra yes, that is an old problem but no, one should never rely on that the enumerator/yield method is "finished" (look for implementing IEnumerator's Reset method, there is often mentioned that no one specified this must be fired, also foreach doesn't; yield methods should also not handle opening/closing db connections, because they are not necessarily executed until their "end") – metadings Jul 19 '13 at 10:38
`SingleOrDefault` does throw an exception when there is more than one item, just like `Single`. The difference is when there are no items: `Single` throws, `SingleOrDefault` returns `default(T)` (which is null for reference types) – Hans Keﬆing Jul 19 '13 at 11:03
@HansKesting confusion corrected ;) – metadings Jul 19 '13 at 11:07

Why does Single() not return directly when more than one element is found?

2 Answers2

Linked