Groovy collect vs spread-dot operator
Yesterday I was doing some Groovy code cleanup with the wonderful CodeNarc static analysis tool. One of the violations it found was UnnecessaryCollectCall.
The summary of the rule is "Some method calls to Object.collect(Closure) can be replaced with the spread operator."
e.g. Replace
But when I performed that refactoring and ran all the tests, some failed! Here's why:
I made the mistake of assuming that the spread operator behavior is always identical to the collect method. For a non-null collection, it is. e.g. The following code will produce the same result regardless of the technique you use:
tl;dr; Explicit and implicit spread operations are great, but be aware that they are less forgiving than the collect method
The summary of the rule is "Some method calls to Object.collect(Closure) can be replaced with the spread operator."
e.g. Replace
things.collect { it.name } with things*.name, or even things.name if what you're after is a property.
But when I performed that refactoring and ran all the tests, some failed! Here's why:
I made the mistake of assuming that the spread operator behavior is always identical to the collect method. For a non-null collection, it is. e.g. The following code will produce the same result regardless of the technique you use:
Loading ....
But if the collection you're operating on is null, the three techniques will result in different outcomes:
Loading ....
What this means is that if you're working with Collections that can potentially be null, you need to think about the consequences of the dot operations before using them. i.e. Don't ever use the implicit spread operator (things.a) if the collection can be null. And only use *. if it's ok for the result to be null.
tl;dr; Explicit and implicit spread operations are great, but be aware that they are less forgiving than the collect method
posted by Unknown at
5:17 PM
7 Comments:
Thanks for this :)
Cool stuff. If I thought that the instance could be null I'd use ?. (which works for the 1st and 3rd examples, but not *.) and in that case they both return null which agrees with *.
That's a real wat moment for groovy. Why doesn't #1 result in a NPE just like #3? After all things is null so how in the world is it calling collect() on a null reference? But what's surprising is that with a no typing information and no hint of a class it somehow found the collect() mixin method and called it. What if things hadn't intended to be a collection and was some other type? I can still call collect on it being null, and it will do this. Doesn't that seem awfully strange! That makes it seem like collect() is a global function which points to namespace collision issues with mixins.
Charlie - it's null-safe the same way each() and several other methods are. It's not particularly mystical - the compiler does a lot (use JD-GUI or another decompiler to see how much), and the MOP does the rest. As for the lack of typing information, that's pretty much the core of how Groovy works - Java does what it has to do based on the declared type at compile time, and Groovy does what it can based on the discovered type at runtime. This is true of every method call.
Bruce - I get the difference between Java and Groovy with regards to how it handles types. There is no compile time typing information sure. But that's not a problem because you can figure it out a runtime. What things is pointing to at runtime. Well that's null which means even at runtime there is not typing information. So if it's pointing at null it's guessing. So something is filling in the missing information and it's guessing is my point. And that it's a special case where trying to invoke a method on null doesn't always result in a NPE. It's inconsistent and unexpected.
Cameron - collect() is defined on the Collection interface - you can see that in the Groovy JDK (http://beta.groovy-lang.org/docs/latest/html/groovy-jdk/java/util/Collection.html#collect(groovy.lang.Closure)). So just by calling it you're telling Groovy what you expected the type to be. But yes, it is a little inconsistent, but that's consistent with how Groovy tends to be.
Crap sorry Burt, I missed typed. :-( Actually I had to check through where collect is defined. It's actually defined up on Object so it might be that it's guessing that no type must mean Object which makes this less inconsistent because it only means its inconsistent with these special methods registered on Object. But it's inconsistent because if you change this to anything other than a method on Object like things.flatten() and it throws a NPE which is closer to expectations.
Post a Comment
Subscribe to Post Comments [Atom]
<< Home