Should "possible multiple enumerations of enumerable" inspection count .Any() ?
Hi, i've posted a bug recently RSRP-206071:
Example:
var res = ((IEnumerable<int>)new int [0]);
if (res.Any()) res = res.Where(i => i == 1);
In the code above there will be an inspection warning "Possible multiple enumerations of enumerable" with
res.Any and res.Where highlighted.
While this is technically correct inspection (.Any(), does use enumeration inside), its a bit misleading.
BTW it would be nice instead detect places that will use Count() == 1 or Count() > 0 and suggest replacing them with .Any()
That has been closed as wont fix with comment: ".Any() IS enumeration.".
Well, thats exactly what is stated in bug description and the bug itself is not about wrongness of the inspection - I find it very useful apart from this one exception. IMHO people use .Any() if they know about its implications.
At least it would be nice to see option to count/not count .Any as a separate preference?
Please, prove me wrong if I'm missing something.
Regards,
Sergei
Please sign in to leave a comment.
I haven't caught your idea. For IEnumerable type, calls to .ANy(), .Count() etc will lead to enumeration. ReSharper tries to guess if the underlying type is more specific type rather than IEnumerable, and if succeeded, then message isn't shown. If you have to call for these methods, it will be good to change variable type from IEnumerable to ICollection
Hi,
I'll try to explain my point here.
1) .Any does execute enumeration - but it executes e.MoveNext() only once (O(1) operations) - yes, it is unperformant comparing to collection.count (O(0) operations << O(1)) but its not that bad comparing to Count() that enumerates it through (O(n) operations) which is the most common extension method misuse I've seen.
2) Inspection message appears only when there were at least one more enumeration in the code, so total cost will be O(1) + O(x) where x >=1 that is O(x) >> O(1).
3) In some cases it's much easier to use IEnumerable<> forgetting about actual underlying classes by the cost of the difference between O(1) and O(0) that could be ignored comparing to O(x) anyway. Consider following code:
4) IMHO readability:
if (itemList.Any())
DoStuff();
vs.
if (itemList.Count > 0)
DoStuff();
if (itemList.Length > 0)
DoStuff();
To sum up, its not really life-death issue to me and I'll be satisfied with any decision. However, I want to add that detection of Count()==0 or Count()>0 could be a good inspection in any case :).
Regards,
Sergei
You haven't counted lazy computations and linq expressions passed as IEnumerable variable. They make the point.
In case of lazy comutations there wont be an ICollection underlying class anyway. And basicly any enumeration method will trigger them - that is - 2nd usage of enumeration.
>any enumeration method will trigger them That what this message is about
As far as i understand this inspection is triggered only in case of 2+ usages of enumeration, and therefore, if there are lazy underlying calculations they will be triggered only once anyway. I may lack some knowledge on linq internals but I hope that's how it works. As for linq2sql - i can only guess here..
Regards,
Sergei
No. lazy computation will be triggered multiple times
My bad then :). But I hope they will change it in future...
Thanks for enlightening,
regards,
Sergei