History | Log In     View a printable version of the current page.  
Issue Details (XML | Word | Printable)

Key: GROOVY-2411
Type: Bug Bug
Status: Closed Closed
Resolution: Fixed
Priority: Major Major
Assignee: Paul King
Reporter: David A. Mellis
Votes: 0
Watchers: 1
Operations

If you were logged in you would be able to see more operations.
groovy

[].sum() should return 0 not null.

Created: 17/Dec/07 02:42 PM   Updated: 22/Dec/07 06:32 AM
Component/s: groovy-jdk
Affects Version/s: 1.5
Fix Version/s: 1.5.2

Time Tracking:
Not Specified


 Description  « Hide
This would make computation more convenient, as I wouldn't have to explicitly check for the empty list.

 All   Comments   Work Log   Change History      Sort Order: Ascending order - Click to sort in descending order
Jeff Brown - 19/Dec/07 11:35 PM
GroovyMethodsTest currently asserts that [].sum() does in fact return null.
GroovyMethodsTest.groovy
class GroovyMethodsTest extends GroovyTestCase {
    
    void testSum() {
        assert [].sum() == null
        assert [null].sum() == null

        // ...

        assert [].sum {it.length()} == null
        assert [null].sum {it.toString()} == 'null'

        // ...
    }
}

Is there some history to that? I think it makes sense for sum() to return zero for an empty list and that probably also makes sense for [null].


Paul King - 20/Dec/07 07:25 PM - edited
I am not sure what the best correct behaviour is. Certainly if I assume the list would have contained numbers, then 0 would be the expected answer. If the list would have contained Strings, then perhaps "" would be the correct answer. If I assume the list would contain nested lists, then I may expect an empty list as the return value.

Perhaps null.intValue() or null.toInteger() or null.asInteger() should return an integer that fails groovy truth (i.e. 0) instead of return NPE? And null.asString() could return "" and null.asList() could return []. Then you could do assert [].sum().asInteger() == 0


David A. Mellis - 21/Dec/07 09:21 AM
Does it even make sense for sum() to work on anything that's not a number? It's not documented on the collections page (http://groovy.codehaus.org/JN1015-Collections) and I couldn't find sum() in the GDK API. For strings, you can always use join() instead. Alternatively, it seems like an explicit type declaration would help here, e.g. ((List<Integer>) []).sum(), but I'm guessing if I knew more about Groovy typing, I'd understand why that doesn't matter.

Paul King - 21/Dec/07 05:30 PM
Groovy just applies duck-typing here. So sum() will work on anything with a plus() method. This could be Arrays, Strings, dates, colors, bank accounts, etc. It even supports hetergeneous mixtures as long as the plus() methods of the individual items in the collection handle that.

It would also be good if Groovy did the expected thing when using static-typing as per your List<Integer> example. At the moment, this isn't catered for from typing information but through user supplied knowledge, i.e. by allowing an initial value, e.g.:

assert [].sum(0) == 0
assert [].sum('') == ''
assert [].sum([]) == []

If you also wanted to support achieving this through type introspection you could ask for a feature enhancement. However, Groovy currently does type erasure more aggressively than Java so at the moment the information won't be available to make this happen. Maybe a possibility for Groovy 2.0.


David A. Mellis - 21/Dec/07 10:14 PM
Ah, I didn't realize you could do {[].sum(0)}. That's a fine solution to the problem. In that case, I might suggest adding a couple more examples of sum() to the Collections page in the User's Guide: http://groovy.codehaus.org/JN1015-Collections. At least one that shows it's possible to sum() lists of values other than numbers. (Or maybe just a sentence that says you can sum() anything with a plus() method.)

Paul King - 21/Dec/07 11:56 PM - edited
Documentation amended. Nothing else to fix for now. Please reopen an enhancement request if you want the statically-typed example to work - but it won't be stragith away and will probably come about only through a major rehaul of other parts of Groovy nothing specially to do with the sum() method..