LINQ: Not Any vs All Don't

Question

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?

Solution 1

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?..."

Solution 2

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
}

Solution 3

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.

Solution 4

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.

Solution 5

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.

View more solutions

Comments

[RQDQ]

Have you tried disassembling the Linq code to see what it's doing?

[cancit]

In this case I would actually go with if(!acceptedValues.Contains(someValue)), but of course this was not the question :)

[Mark]

@csgero I agree. The above was a simplification (perhaps over-simplification) of the real logic.

[Kevin Coulombe]

I think "if there is not any value equal to x" is clearer than "if all values are different than x". I wish JetBrains would split the "Simplify LINQ expression" inspection into different types of inspections so I could disable this specific one.

[Jim Balter]

"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.

[Timo]

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)).

[Caltor]

Be careful with multiple comparisons though as !Any(v => v.Value == someValue && v.Key == anotherValue) is not equivalent to All(v => v.Value != someValue && v.Key != anotherValue) but is equivalent to All(v => (!(v.Value == someValue && v.Key == anotherValue))

[Jon Hanna]

Yes, but bool allEven = !Enumerable.Empty<int>().Any(i => i % 2 != 0) is true too.

[Rune FS]

@Jon semantically none != all. So semantically you either have none or all but in the case of .All() none is just a subset of all the collections that return true for all and that discrepancy can result in bugs if you are unaware of it. +1 for that Anthony

[Jon Hanna]

@RuneFS I don't follow. Semantically and logically "none where it is untrue that..." is indeed the same as "all where it is true that". E.g. "where none of the accepted projects from our company?" will always have the same answer as "where all of the accepted projects from other companies?"...

[Jon Hanna]

... Now, it's true that you can have bugs from assuming "all the items are..." means there is at least one item that is at least one item that fulfils the test, since the "all the items..." is always true for the empty set, I don't dispute that at all. I added though that the same problem can happen with assuming "none of the items..." means at least one items doesn't fulfill the test, since "none of the items..." is also always true for the empty set. It's not that I disagree with Anthony's point, its that I think it also holds for the other of the two constructs under discussion.

[Rune FS]

@Jon you are talking logic and I am talking linguistics. The human brain can't process a negative (before it process the positive at which point it can then negate it) so in that sense there's quite a difference between the two. That doesn't make the logic you propose incorrect

[Jon Hanna]

@RuneFS I grant you that 100% (well, 98% because the human brain can process negatives at some levels or else we wouldn't be able to have this conversation). My point isn't that the two constructs are logically the same (though it is part of my point), but that if someone fails to grok the implications of the first in terms of the empty set then they are just as likely to grok the second in the same way.

[Anthony Pegram]

I will note that I mentioned the area of subtlety because it has specifically tripped people up in the past here, and it was even a source of a bug in the application I am presently working on (wasn't my bug, not that I haven't accidentally contributed any). At any rate, the important thing is that both the early return and the zero-element behavior are in the linked documentation.

[Mark]

Thanks - I was already thinking of implementing these in our commons library, but I haven't decided if it is a good idea yet. I agree that they make the code more readable, but I am concerned that they don't add sufficient value.

[Jon Hanna]

Yep, certainly worth mentioning it.

[VikciaR]

I'm not sure what is done behind the lines, but for me is much more readable is: if (not any) than if (all not equal).

[Jon Hanna]

@VikciaR Yes, I agree. That said, in general, I find if(something) more readable than if(not somethingElse). Hence my guess that this is what the rule is looking for, without anything to look for the more specific case where "something" is "all not equal" and "somethingElse" is "any". Of course, I can only guess as to what re-sharper's developers thinking on it was.

[Arnaud]

There is a BIG difference when your enumeration has no values. 'Any' would always return FALSE, and 'All' always return TRUE. So saying than one is the logical equivalent of the other is not entirely true!

[Niall Connaughton]

@Arnaud I still don't see the difference. In the context of this example, Any will return false and then be negated to true by the ! in the if statement. All will return true, and so both are still equivalent.

[Jon Hanna]

@Arnaud Any will return false and hence !Any will return true, so they're identical.

[Jim Balter]

@Arnaud There is not any person who commented who said that Any and All are logically equivalent. Or to put it another way, all persons who commented did not say that Any and All are logically equivalent. The equivalence is between !Any(predicate) and All(!predicate).

[Macs Dickinson]

I know this is an old thread but I thought I'd point out a key difference between !Any and All when comparing more than one property. Take the following dictionary var test = new Dictionary<int, int> { { 1, 1 }, { 2, 1 }, { 3, 2 } };. As expected !test.Any(x => x.Key == 3 && x.Value == 1) will return true as there is no entry with key of 3 and value 1. However test.All(x => x.Key != 3 && x.Value != 1) will return false as it matches the key of 3 and value of 1 separately. With .All() you would need to use test.All(x => x.Key != 3 || x.Value != 1)

[Jon Hanna]

@MacsDickinson that's not a difference at all, because you're not comparing opposite predicates. The equivalent to !test.Any(x => x.Key == 3 && x.Value == 1) that uses All is test.All(x => !(x.Key == 3 && x.Value == 1)) (which is indeed equivalent to test.All(x => x.Key != 3 || x.Value != 1)).

[Anthony Mason]

I would not say that they are identical by any means, however, they both short circuit once a condition is met. So that being said in some cases, let's say a sorted list, Any() may perform better as the value is found to be true earlier, otherwise the entire list MAY be iterated. Personally, I prefer Any() over All(), but I feel there is little difference and is entirely conditional on the size of the list/array as well as the context.

[Jon Hanna]

@AnthonyMason no, they will both sort-circuit on the same instance. Size of collection would have zero impact.

[WPFKK]

you are right but they stop at the same time for a given collection. All breaks when condition fails and Any Breaks when it matches your predicate. So Technically no different except scenatically

[Rhyous]

I looked for None and didn't find it. It is very much more readable.

[Rhyous]

I had to add null checks: return source == null || !source.Any(predicate);

[Caltor]

@NetMage no as pointed out by Jon Hanna they will both short-circuit on the same instance. Ordering of data makes no difference.

[Dib]

I am all for well-named "explaining variables" when evaluating conditions, and frequently use them in my code, yet I still come up against resistance sometimes in code reviews as it adds more "unnecessary" lines to the member. However, in your case I would negate the explaining variable so you can read if (valueIsNotAccepted) { ... } to get rid of the ! negation in the parenthesis.