groovy

Groovy getAt cannot be used with lazily initialized lists

Details

  • Number of attachments :
    0

Description

I'm not sure if this should even be logged as a bug, but here goes:

I was playing around with the Apache Commons ListUtils.lazyList. This list will automatically create an item if an index is not yet defined (or is null). Example code:

Example.groovy
@Grab(group='commons-collections', module='commons-collections', version='[1.3,)')
import org.apache.commons.collections.ListUtils
import org.apache.commons.collections.Factory
import groovy.transform.Canonical

@Canonical class Test {
    String name
    int amount
}

List<Test> t = ListUtils.lazyList([], { new Test() } as Factory)

// UNCOMMENT to make the example work:
// t.get(1)

// Thows NPE here:
t[1].name = 'Jim'
t[0].amount = 6

assert t == [new Test(null, 6), new Test('Jim', 0)]

However, I thought it was broken, because I had been running with without the commented t.get(1) line. The getAt code checks the size of the dynamic list, and since it is smaller, returns null. This causes an NPE to be thrown.

I realize that the lazy list breaks the List contract (by dynamically changing the list size). However, it seems like a fairly useful feature to have a lazily created list.

I'm not sure there is an acceptable solution, but returning null hides the problem in a way that is hard to debug! Maybe it would be better to have a withDefault method for lists, too, that works like the one for {{Map}}s. That would provide a usable solution without breaking the current design. Plus, you no longer have to include the commons library for that use case.

Activity

Ascending order - Click to sort in descending order
Hide
Permalink
Paul King added a comment -

Yes, I think it would be good to introduce a ListWithDefault in Groovy similar to MapWithDefault. We could mimic the behaviour in the commons collection LazyList as you suggest. Here is the javadoc for anyone interested:

http://commons.apache.org/collections/apidocs/org/apache/commons/collections/list/LazyList.html

We would also need to add a DGM#withDefault(List, Closure) method.

A variation we could potentially support is a boolean flag to indicate eager creation of intervening index entries, i.e. DGM#withDefault(List, Closure, Boolean) with default to false, i.e. we would mimic LazyList and not support null as a valid list value. With the boolean set to true, we could support nulls but if you had a list with 2 elements (index values 0 and 1), then calling get at index 4 would cause the closure to be called for index values 2, 3 and 4 - rather than calling it just for 4 and setting the others to null.

Show
Paul King added a comment - Yes, I think it would be good to introduce a ListWithDefault in Groovy similar to MapWithDefault. We could mimic the behaviour in the commons collection LazyList as you suggest. Here is the javadoc for anyone interested: http://commons.apache.org/collections/apidocs/org/apache/commons/collections/list/LazyList.html We would also need to add a DGM#withDefault(List, Closure) method. A variation we could potentially support is a boolean flag to indicate eager creation of intervening index entries, i.e. DGM#withDefault(List, Closure, Boolean) with default to false, i.e. we would mimic LazyList and not support null as a valid list value. With the boolean set to true, we could support nulls but if you had a list with 2 elements (index values 0 and 1), then calling get at index 4 would cause the closure to be called for index values 2, 3 and 4 - rather than calling it just for 4 and setting the others to null.
Hide
Permalink
Paul King added a comment -

Actually, not sure whether we should split out creation of ListWithDefault as a separate Jira issue. In theory, we might want to address the getAt problem separately though I can't think of a backwards compatible way to do that.

Show
Paul King added a comment - Actually, not sure whether we should split out creation of ListWithDefault as a separate Jira issue. In theory, we might want to address the getAt problem separately though I can't think of a backwards compatible way to do that.
Hide
Permalink
Andre Steingress added a comment -

The problem is that LazyList breaks the List contract as already mentioned above. I can't see how to handle this in DGM#getAt without introducing any direct/indirect references/checks for Commons LazyList.

Thus, I would vote for going with the DGM#withDefault approach.

Show
Andre Steingress added a comment - The problem is that LazyList breaks the List contract as already mentioned above. I can't see how to handle this in DGM#getAt without introducing any direct/indirect references/checks for Commons LazyList. Thus, I would vote for going with the DGM#withDefault approach.
Hide
Permalink
Phil DeJarnett added a comment -

If there is no way to "fix" getAt, I think that simply providing a withDefault is valid solution to this bug.

The real issue here is finding a way to support lazily-initialized lists via the subscript operator, not necessarily supporting the commons version.

I also think that using withDefault(Closure, Boolean) would be ugly (and have a "magic" boolean). It would make more sense to have it a second method with the functionality in the name, like withFilledDefault(Closure), which avoids the ambiguity.

Show
Phil DeJarnett added a comment - If there is no way to "fix" getAt , I think that simply providing a withDefault is valid solution to this bug. The real issue here is finding a way to support lazily-initialized lists via the subscript operator, not necessarily supporting the commons version. I also think that using withDefault(Closure, Boolean) would be ugly (and have a "magic" boolean). It would make more sense to have it a second method with the functionality in the name, like withFilledDefault(Closure) , which avoids the ambiguity.
Hide
Permalink
Paul King added a comment -

A potential gotcha compared to LazyList from commons collections is handling -ve indices. For most list-like things in Groovy we would allow index values from -1 through to (-size() + 1). I guess this is possible for ListWithDefault but needs to be thought through.

Show
Paul King added a comment - A potential gotcha compared to LazyList from commons collections is handling -ve indices. For most list-like things in Groovy we would allow index values from -1 through to (-size() + 1). I guess this is possible for ListWithDefault but needs to be thought through.
Hide
Permalink
Andre Steingress added a comment - - edited

this is the pull request for the ListWithDefault implementation and DGM extension:

https://github.com/groovy/groovy-core/pull/37

Show
Andre Steingress added a comment - - edited this is the pull request for the ListWithDefault implementation and DGM extension: https://github.com/groovy/groovy-core/pull/37
Hide
Permalink
Paul King added a comment -

ListWithDefault has been added. Thanks to all for the ideas and contributions.

Show
Paul King added a comment - ListWithDefault has been added. Thanks to all for the ideas and contributions.

People

  • Assignee:
    Paul King
    Reporter:
    Phil DeJarnett
Vote (0)
Watch (1)

Dates

  • Created:
    Updated:
    Resolved: