286
votes

Often I want to check if a provided value matches one in a list (e.g. when validating):

if (!acceptedValues.Any(v => v == someValue))
{
    // exception logic
}

Recently, I've noticed ReSharper asking me to simplify these queries to:

if (acceptedValues.All(v => v != someValue))
{
    // exception logic
}

Obviously, this is logically identical, perhaps slightly more readable (if you've done a lot of mathematics), my question is: does this result in a performance hit?

It feels like it should (i.e. .Any() sounds like it short-circuits, whereas .All() sounds like it does not), but I have nothing to substantiate this. Does anyone have deeper knowledge as to whether the queries will resolve the same, or whether ReSharper is leading me astray?

8
Have you tried disassembling the Linq code to see what it's doing?RQDQ
In this case I would actually go with if(!acceptedValues.Contains(someValue)), but of course this was not the question :)csgero
@csgero I agree. The above was a simplification (perhaps over-simplification) of the real logic.Mark
"It feels like it should (i.e. .Any() sounds like it short-circuits, whereas .All() sounds like it does not)" -- Not to anyone with sound intuitions. The are logical equivalence you note implies that they are equally short-circuitable. A moment's thought reveals that All can quit as soon as a non-qualifying case is encountered.Jim Balter
I do not universally agree with ReSharper on this. Write sensible trains of thought. If you want to throw an exception if a required item is missing: if (!sequence.Any(v => v == true)). If you wish to continue only if everything conforms to a certain specification: if (sequence.All(v => v < 10)).Timo

8 Answers

354
votes

Implementation of All according to ILSpy (as in I actually went and looked, rather than the "well, that method works a bit like ..." I might do if we were discussing the theory rather than the impact).

public static bool All<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
    if (source == null)
    {
        throw Error.ArgumentNull("source");
    }
    if (predicate == null)
    {
        throw Error.ArgumentNull("predicate");
    }
    foreach (TSource current in source)
    {
        if (!predicate(current))
        {
            return false;
        }
    }
    return true;
}

Implementation of Any according to ILSpy:

public static bool Any<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate)
{
    if (source == null)
    {
        throw Error.ArgumentNull("source");
    }
    if (predicate == null)
    {
        throw Error.ArgumentNull("predicate");
    }
    foreach (TSource current in source)
    {
        if (predicate(current))
        {
            return true;
        }
    }
    return false;
}

Of course, there could be some subtle difference in the IL produced. But no, no there isn't. The IL is pretty much the same but for the obvious inversion of returning true on predicate match versus returning false on predicate mismatch.

This is linq-for-objects only of course. It's possible that some other linq provider treats one much better than the other, but then if that was the case, it's pretty much random which one got the more optimal implementation.

It would seem that the rule comes down solely to someone feeling that if(determineSomethingTrue) is simpler and more readable than if(!determineSomethingFalse). And in fairness, I think they've a bit of a point in that I often find if(!someTest) confusing* when there's an alternative test of equal verbosity and complexity that would return true for the condition we want to act upon. Yet really, I personally find nothing to favour one over the other of the two alternatives you give, and would perhaps lean very slightly toward the former if the predicate were more complicated.

*Not confusing as in I don't understand, but confusing as in I worry that there's some subtle reason for the decision that I don't understand, and it takes a few mental skips to realise that "no, they just decided to do it that way, wait what was I looking at this bit of code for again?..."

63
votes

You might find these extension methods make your code more readable:

public static bool None<TSource>(this IEnumerable<TSource> source)
{
    return !source.Any();
}

public static bool None<TSource>(this IEnumerable<TSource> source, 
                                 Func<TSource, bool> predicate)
{
    return !source.Any(predicate);
}

Now instead of your original

if (!acceptedValues.Any(v => v == someValue))
{
    // exception logic
}

you could say

if (acceptedValues.None(v => v == someValue))
{
    // exception logic
}
29
votes

Both would have identical performance because both stop enumeration after the outcome can be determined - Any() on the first item the passed predicate evaluates to true and All() on the first item the predicate evaluates to false.

21
votes

All short circuits on the first non-match, so it's not a problem.

One area of subtlety is that

 bool allEven = Enumerable.Empty<int>().All(i => i % 2 == 0); 

Is true. All of the items in the sequence are even.

For more on this method, consult the documentation for Enumerable.All.

9
votes

As other answers have well covered: this is not about performance, it's about clarity.

There's wide support for both of your options:

if (!acceptedValues.Any(v => v == someValue))
{
    // exception logic
}

if (acceptedValues.All(v => v != someValue))
{
    // exception logic
}

But I think this might achieve broader support:

var isValueAccepted = acceptedValues.Any(v => v == someValue);
if (!isValueAccepted)
{
    // exception logic
}

Simply computing the boolean (and naming it) before negating anything clears this up a lot in my mind.

8
votes

All() determines whether all elements of a sequence satisfy a condition.
Any() determines whether any element of a sequence satisfies the condition.

var numbers = new[]{1,2,3};

numbers.All(n => n % 2 == 0); // returns false
numbers.Any(n => n % 2 == 0); // returns true
7
votes

According to this link

Any – Checks for at least one match

All – Checks that all match

5
votes

If you take a look at the Enumerable source you'll see that the implementation of Any and All is quite close:

public static bool Any<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate) {
    if (source == null) throw Error.ArgumentNull("source");
    if (predicate == null) throw Error.ArgumentNull("predicate");
    foreach (TSource element in source) {
        if (predicate(element)) return true;
    }
    return false;
}

public static bool All<TSource>(this IEnumerable<TSource> source, Func<TSource, bool> predicate) {
    if (source == null) throw Error.ArgumentNull("source");
    if (predicate == null) throw Error.ArgumentNull("predicate");
    foreach (TSource element in source) {
        if (!predicate(element)) return false;
    }
    return true;
}

There is no way that one method be significantly faster than the other since the only difference lies in a boolean negation, so prefer readability over false performance win.