30

I need to check whether an item doesn't exist in a list of items in C#, so I have this line:

if (!myList.Any(c => c.id == myID))) 

Resharper is suggesting that I should change that to:

if (myList.All(c => c.id != myID))) 

I can see that they are equivalent, but why is it suggesting the change? Is the first implementation slower for some reason?

Fiona - myaccessible.website
  • 14,481
  • 16
  • 82
  • 117
  • 1
    Nope, it will run to same query, but it's more clear for me: `all not equal` vs `not any equal` – Kamil Budziewski Dec 20 '13 at 13:49
  • 3
    Second line is way more clear. – Max Dec 20 '13 at 13:49
  • Readability, which one is readable? – Sriram Sakthivel Dec 20 '13 at 13:51
  • @wudzik - they're not the same query. `Any` will go through the entire sequence, `All` will stop at the first one that fails, and is therefore a more efficient way to test. – Sean Dec 20 '13 at 13:53
  • @Sean Nope Any will break also. – Ralf Dec 20 '13 at 13:54
  • @Ralf - yes, but Any may have to go through the entire sequence in order to find one with a match, whereas All will stop as soon as one doesn't. – Sean Dec 20 '13 at 13:57
  • 4
    @GrantWinney Right. And since the boolean operator is inverted in the predicate both will break at the same element. – Ralf Dec 20 '13 at 14:02
  • 1
    @GrantWinney it's probably microoptimization, but "all not" will perform a [negation](http://stackoverflow.com/a/1030013/1037948) on every item until the predicate is triggered, whereas "not any" will perform a single negation after the predicate is triggered, so technically `Any` could be more efficient. EF may translate them both to the same query though, so the point could be moot. – drzaus Aug 11 '14 at 16:33
  • I personally don't think you should use 'Any' or 'All' when you're talking about identity properties. You're looking for a specific item, whereas 'Any' or 'All' imply collections / multiple possibilities. The better solution is to use mylist.FirstOrDefault(x => x.id == myId), and then handle the null result elsewhere. The chances are, if you are referring to a specific ID, you wanted to do something with the entity/object anyway. – Oriental Jun 08 '18 at 10:12

3 Answers3

31

The readability of the expression is to me a personal opinion.

I would read this

if (!myList.Any(c => c.id == myID))) 

as 'is my item not in the collection'. Where this

if (myList.All(c => c.id != myID))) 

reads as 'are all items in the collection different than my item'.

If the 'question' I want to ask -through my linq query- is 'is my item not in the list', then the first query better suits the question that I want to ask. The ! in front of the first query is not a problem.

Maarten
  • 22,527
  • 3
  • 47
  • 68
  • 11
    This is very interesting because I also find the first matches in english what I want to do, but others disagree – Fiona - myaccessible.website Dec 20 '13 at 14:07
  • 1
    @CodingKiwi We have just the generic one line of code. In that contex i would also select the All method. But you have more context than we have. In that complex context Any might feel better. And not only feel better but could also be more readable then. – Ralf Dec 20 '13 at 14:09
  • 2
    This is so true. If the code sort of flows with the internal narrative in my head, I like it better. For instance, !Any(==) makes a lot of sense when I'm thinking, "This is when I need to deal with the case where I can't find the one I want". All(!=) is more appropriate when you find yourself thinking, "This is where we need to verify that everything I have here is valid". It is all about matching the code to the internal narrative, so you don't have to think about it so much. – bowserm Feb 06 '17 at 23:38
  • 1
    Completely agree. If my thought process is that I want to check that my shopping list doesn't contain any peanuts (because I'm allergic), !myList.Any(x => x.IsPeanut) reflects my thought process. If I want to check that everything I am buying isn't too expensive, myList.All(x => !x.IsExpensive) makes more sense. The key difference in my mind is that in the first example, I care about a single instance occuring (Any). With the second example, I logically want to check all items (All). – Oriental Jun 08 '18 at 09:56
  • "not any" is more clear to me than "all not" – NicoJuicy Sep 02 '19 at 09:34
  • At any rate, given that it subjectively strikes different people in different ways, I think Resharper is overstepping its bounds and should shut up about it. I don't use Resharper but opened my project one day to find another developer who does, blindly replaced all of my code, which I find more clear with `!Any`, with Resharper's suggestions. – xr280xr Apr 23 '20 at 05:50
22

It's all too easy to miss the ! at the start of the expression in your first example. You are therefore making the expression difficult to read. In addition, the first example reads as "not any equal to", whereas the second is "all not equal to". It's no coincidence that the easier to read code can be expressed as easier to read English.

Easier to read code is likely to be less buggy, as it's easier to understand what it does before changing it. It's because the second example is clearer that ReSharper recommends changing your code.

David Arno
  • 42,717
  • 16
  • 86
  • 131
  • 16
    I personally find the first one easier to read, although I definitely take your point about overlooking the ! Maybe the first is also the easiest to misunderstand. – Fiona - myaccessible.website Dec 20 '13 at 14:08
  • 5
    Add another extension method, named `None`, if you think it's more readable? – Roger Lipscombe Dec 20 '13 at 14:51
  • Thats a nice idea @RogerLipscombe – Fiona - myaccessible.website Dec 20 '13 at 15:14
  • 1
    I've always found `!=` to be more readable than `!` and `==`. The exclamation point negation is too easy to overlook. This is one place that I think VB has an advantage: `Not` is tough to miss. – Michael Richardson Dec 21 '13 at 03:25
  • I think it really depends on the context. There are times where I find each approach to be more understandable depending on the context of the intended logic (how would you logically think about it) as well as the length of the LINQ query. If you are writing a LINQ query with multiple operations then it can be very easy to miss the exclamation point all the way back at the beginning. If `.Any()` is the only method on the set, then to me, it's the same question as whether `!someBool` or `someBool == false` is better. – xr280xr Apr 23 '20 at 05:45
  • However, if you have a large collection, and you do not want to add an item to it if there is already an existing duplicate item, .Any is more efficient as it would stop searching as soon as it finds the first instance. .All would look at every single item in the collection – wingyip Jul 21 '21 at 16:54
  • @wingyip, fear not; that isn't how it works. In both cases, the method returns as soon as a check fails. Have a look at the source for both methods at https://github.com/microsoft/referencesource/blob/master/System.Core/System/Linq/Enumerable.cs to see how they work. – David Arno Jul 23 '21 at 15:04
3

In general asking a positive question is more intuitive. If you asked the user "Do you really not want to delete this record?", guess how often he will hit the wrong button.

I personally like to turn constructs like this around:

// Not optimal
if (!x) {
    A();
} else }
    B();
}

// Better
if (x) {
    B();
} else }
    A();
}

An exception might be the test for not null where a != null might be perceived as positive.

Olivier Jacot-Descombes
  • 104,806
  • 13
  • 138
  • 188
  • The question asked here is 'is my item absent from the list' which I still see as a normal question, which you can see as the positive case. 'Yes, it is absent, then do this...' – Maarten Dec 20 '13 at 14:11
  • 2
    "Is not my item in the list" matches more closely the OS' original code. A hypothetical statement like `myList.Absent(c => c.id == myID)` would be more direct. I agree that here the `All` condition is not very intuitive as well. In my mind `Resharper` tends to be puritanical and overzealous. – Olivier Jacot-Descombes Dec 20 '13 at 14:23