22

I want to check that an IEnumerable contains exactly one element. This snippet does work:

bool hasOneElement = seq.Count() == 1

However it's not very efficient, as Count() will enumerate the entire list. Obviously, knowing a list is empty or contains more than 1 element means it's not empty. Is there an extension method that has this short-circuiting behaviour?

guhou
  • 1,732
  • 12
  • 32
  • I think the logic error I made in my code may have thrown people with regards to *at least* and *exactly*. I think we've arrived at the best solution now. – guhou Sep 27 '10 at 08:38
  • 1
    Count() will only iterate the entire list if it determines that the passed IEnumerable doesn't cast to ICollection. So if you pass a List instance, or an array, it won't iterate. – Dave Van den Eynde Sep 27 '10 at 09:35
  • 1
    I think this is all premature optimization. – Dave Van den Eynde Sep 27 '10 at 09:41
  • I wouldn't consider a simple solution that takes O(1) instead of O(n) a 'premature optimization'; it's just a sensible algorithm choice. – guhou Sep 27 '10 at 13:04

5 Answers5

20

This should do it:

public static bool ContainsExactlyOneItem<T>(this IEnumerable<T> source)
{
    using (IEnumerator<T> iterator = source.GetEnumerator())
    {
        // Check we've got at least one item
        if (!iterator.MoveNext())
        {
            return false;
        }
        // Check we've got no more
        return !iterator.MoveNext();
    }
}

You could elide this further, but I don't suggest you do so:

public static bool ContainsExactlyOneItem<T>(this IEnumerable<T> source)
{
    using (IEnumerator<T> iterator = source.GetEnumerator())
    {
        return iterator.MoveNext() && !iterator.MoveNext();
    }
}

It's the sort of trick which is funky, but probably shouldn't be used in production code. It's just not clear enough. The fact that the side-effect in the LHS of the && operator is required for the RHS to work appropriately is just nasty... while a lot of fun ;)

EDIT: I've just seen that you came up with exactly the same thing but for an arbitrary length. Your final return statement is wrong though - it should be return !en.MoveNext(). Here's a complete method with a nicer name (IMO), argument checking and optimization for ICollection/ICollection<T>:

public static bool CountEquals<T>(this IEnumerable<T> source, int count)
{
    if (source == null)
    {
        throw new ArgumentNullException("source");
    }
    if (count < 0)
    {
        throw new ArgumentOutOfRangeException("count",
                                              "count must not be negative");
    }
    // We don't rely on the optimizations in LINQ to Objects here, as
    // they have changed between versions.
    ICollection<T> genericCollection = source as ICollection<T>;
    if (genericCollection != null)
    {
        return genericCollection.Count == count;
    }
    ICollection nonGenericCollection = source as ICollection;
    if (nonGenericCollection != null)
    {
        return nonGenericCollection.Count == count;
    }
    // Okay, we're finally ready to do the actual work...
    using (IEnumerator<T> iterator = source.GetEnumerator())
    {
        for (int i = 0; i < count; i++)
        {
            if (!iterator.MoveNext())
            {
                return false;
            }
        }
        // Check we've got no more
        return !iterator.MoveNext();
    }
}

EDIT: And now for functional fans, a recursive form of CountEquals (please don't use this, it's only here for giggles):

public static bool CountEquals<T>(this IEnumerable<T> source, int count)
{
    if (source == null)
    {
        throw new ArgumentNullException("source");
    }
    if (count < 0)
    {
        throw new ArgumentOutOfRangeException("count", 
                                              "count must not be negative");
    }
    using (IEnumerator<T> iterator = source.GetEnumerator())
    {
        return IteratorCountEquals(iterator, count);
    }
}

private static bool IteratorCountEquals<T>(IEnumerator<T> iterator, int count)
{
    return count == 0 ? !iterator.MoveNext()
        : iterator.MoveNext() && IteratorCountEquals(iterator, count - 1);
}

EDIT: Note that for something like LINQ to SQL, you should use the simple Count() approach - because that'll allow it to be done at the database instead of after fetching actual results.

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • 1
    Yes it's ugly, and yes, it's beautiful :-) But no, it's not very obvious when you first see this code. – Philippe Leybaert Sep 27 '10 at 08:30
  • 3
    LOL @Jon for his `return iterator.MoveNext() && !iterator.MoveNext(); `. The answer from me is "both"! – mauris Sep 27 '10 at 08:32
  • What will happen if you call ContainsExactlyOneItem on a linq-to-sql query? Will the data be fetched from the database? – Tomasi Sep 27 '10 at 09:29
  • @diamandiev: Yes. For LINQ to SQL I'd suggest using `Count()` instead, to do it at the database. – Jon Skeet Sep 27 '10 at 09:37
  • 1
    You can add a test for ICollection, just like Count() does, and skip all the iterating. – Dave Van den Eynde Sep 27 '10 at 09:38
  • You need to check `count > 0` in the second part of the recursive function, otherwise you'll count the entire list, not just the relevant part. `(count == 0 && !iterator.MoveNext()) ||| (count > 0 && iterator.MoveNext() && IteratorCountEquals(iterator, count-1));` Also, your parentheses mismatched. And I'd like to emphasize the _please don't use this_ part, since it won't always work - for long iterations it will blow the stack resulting in an uncatchable exception. – configurator Sep 27 '10 at 11:14
  • @configurator: Good catch - will edit. In fact, I've decided to use the conditional operator for this bit, as it's probably more appropriate (and closer to what the F# would look like). Of course, in F# I'd hope it would be tail-recursive, avoiding the stack issue. But yes, please don't use it. – Jon Skeet Sep 27 '10 at 11:18
  • @Jon Cannot say I agree with your assessment that the 2nd version of ContainsExactlyOneItem is nasty. By definition that's what an AndAlso (&&) operator does, it performs short-circuiting logical conjunction on two expressions. Its arguable that the 1st version confers the meaning better than the 2nd. – Bear Monkey Sep 27 '10 at 12:39
  • @Bear: Opinion on Twitter has certainly been divided. I think it would at least require a comment, which is always a bit of a warning sign... – Jon Skeet Sep 27 '10 at 12:55
  • @Jon No more comments than you 1st version ;) – Bear Monkey Sep 27 '10 at 13:01
  • @Bear: Sure... but I wouldn't feel *too* bad about removing the comments in my first version. I think the "smart" version would cause readers to pause for a sufficient length of time to make the "dumb" version better in terms of readability. – Jon Skeet Sep 27 '10 at 13:18
  • @Jon, in 3.5 your: "ArgumentOutOfRangeException("count must not be negative")" should read: "ArgumentOutOfRangeException("count", "count must not be negative")" -- I think. – grenade Oct 06 '10 at 06:28
7

No, but you can write one yourself:

 public static bool HasExactly<T>(this IEnumerable<T> source, int count)
 {
   if(source == null)
      throw new ArgumentNullException("source");

   if(count < 0)
      return false;

   return source.Take(count + 1).Count() == count;
 }

EDIT: Changed from atleast to exactly after clarification.

For a more general and efficient solution (which uses only 1 enumerator and checks if the sequence implements ICollection or ICollection<T> in which case enumeration is not necessary), you might want to take a look at my answer here, which lets you specify whether you are looking forExact,AtLeast, orAtMost tests.

Community
  • 1
  • 1
Ani
  • 111,048
  • 26
  • 262
  • 307
  • don't you still end up enumerating the entire list in that snippet? (due to the `Take` and then `Count`) – guhou Sep 27 '10 at 08:15
  • I'm not asking for *at least*, I'm asking for *exactly*. – guhou Sep 27 '10 at 08:24
  • However, you will double iterate the items won't you? Firstly with the take and then the count. Only the length of items we're interested in, but still twice the number of iterations required. – SamStephens Sep 27 '10 at 08:38
  • @SamStephens: Yes, that's right. Which is why a more efficient (but ugly) solution might be more appropriate. – Ani Sep 27 '10 at 08:42
4

seq.Skip(1).Any() will tell you if the list has zero or one elements.

I think the edit you made is about the most efficient way to check the length is n. But there's a logic fault, items less than length long will return true. See what I've done to the second return statement.

    public static bool LengthEquals<T>(this IEnumerable<T> en, int length)
    {
        using (var er = en.GetEnumerator())
        {
            for (int i = 0; i < length; i++)
            {
                if (!er.MoveNext())
                    return false;
            }
            return !er.MoveNext();
        }
    }
SamStephens
  • 5,721
  • 6
  • 36
  • 44
  • Yes, I was just noticing that myself. I prefer your method name, but my variable names :) - actually I've decided I prefer CountEquals after all, as it matches up with the Count() method better :) – Jon Skeet Sep 27 '10 at 08:31
1

How about this?

public static bool CountEquals<T>(this IEnumerable<T> source, int count) {
    return source.Take(count + 1).Count() == count;
}

The Take() will make sure we never call MoveNext more than count+1 times.

I'd like to note that for any instance of ICollection, the original implementation source.Count() == count should be faster because Count() is optimised to just look at the Count member.

configurator
  • 40,828
  • 14
  • 81
  • 115
0

I believe what you're looking for is .Single(). Anything other than exactly one will throw InvalidOperationException that you can catch.

http://msdn.microsoft.com/nb-no/library/bb155325.aspx

danijels
  • 5,211
  • 4
  • 26
  • 36
  • 1
    I suppose what I'm trying to do could be achieved with this but I don't like throwing exceptions for control flow. – guhou Sep 27 '10 at 08:23
  • 1
    Then use SingleOrDefault() and check if return value is null yourself. – danijels Sep 27 '10 at 08:25
  • SingleOrDefault will still will throw an exception if there is more than one item in the list. I don't think this is the way to go, the code with the enumerator should be more efficient. – SamStephens Sep 27 '10 at 08:29
  • @SamStephens my bad, I did not take that into account. I agree that throwing/catching exceptions in this case is not the way to go. – danijels Sep 27 '10 at 08:36