[groovy-user] Modifying a collection from within the "each" closure

2,678 views
Skip to first unread message

Matthias F. Brandstetter

unread,
Mar 26, 2015, 7:55:02 PM3/26/15
to Groovy User Mailinglist
Let's say I have a collection 'list' with some relements in it. Now I am iterating over all elements, to remove some of them:

list.each {
    println it
    if(it > 0) {
        list -= it
    }
}

From my experience it seems to be safe to do so, i.e. to remove an element directly from within the "each" closure. That is, I don't get any access modification exception.

I know from Java however that you usually don't want to do such a thing in a loop. Instead one should use an Iterator instead. Now, since there seems to be no issue in Groovy, is it safe to go that direct way here?

Kind regards, Matthias


--
Matthias F. Brandstetter
hai...@gmail.com

Tankerbay

unread,
Mar 26, 2015, 9:19:59 PM3/26/15
to us...@groovy.codehaus.org
Wow, I had no idea this would work:
def list = [2,4,6,7,5,9,10,11,12,14,15]
list.each { if ( it % 2 == 0 ) { list -= it } }
assert list == [7,5,9,11,15]

And I'm kind of shocked it does... I'm guessing that the each iterator has an immutable form of the original list in it as it traverses.

That said, I wouldn't do this, it violates functional programming's immutability principle in a big way.  I'd use:
def newlist = list.findAll { it % 2 != 0 }

But if you really wanted to modify the original list, I guess you can.



Dinko Srkoč

unread,
Mar 27, 2015, 5:06:35 AM3/27/15
to us...@groovy.codehaus.org
On 27 March 2015 at 02:18, Tankerbay <tank...@gmail.com> wrote:
> Wow, I had no idea this would work:
> def list = [2,4,6,7,5,9,10,11,12,14,15]
> list.each { if ( it % 2 == 0 ) { list -= it } }
> assert list == [7,5,9,11,15]

Looks can be deceiving - notice that the final list is not the list
you started with:

def list = [1, 2, 3, 4, 5]
def list2 = list

assert list2.is(list)

list.each { if (it % 2) list -= it }

assert !list2.is(list)

>
> And I'm kind of shocked it does... I'm guessing that the each iterator has
> an immutable form of the original list in it as it traverses.

The minus operator actually returns a *new* list with elements from
the original list minus the occurrences of the operand on the right.

>
> That said, I wouldn't do this, it violates functional programming's
> immutability principle in a big way. I'd use:
> def newlist = list.findAll { it % 2 != 0 }

Yes, that's the right way to do it.

>
> But if you really wanted to modify the original list, I guess you can.

`each` method does use Iterator, which allows elements to be removed
from the collection, but that iterator is inaccessible from the `each`
Closure block. If one wanted to mutate the original collection by
removing elements from it, then using `each` would not be the proper
way.

Cheers,
Dinko

>
>
>
>
> On Thu, Mar 26, 2015 at 4:53 PM, Matthias F. Brandstetter <hai...@gmail.com>
> wrote:
>>
>> Let's say I have a collection 'list' with some relements in it. Now I am
>> iterating over all elements, to remove some of them:
>>
>> list.each {
>> println it
>> if(it > 0) {
>> list -= it
>> }
>> }
>>
>> From my experience it seems to be safe to do so, i.e. to remove an element
>> directly from within the "each" closure. That is, I don't get any access
>> modification exception.
>>
>> I know from Java however that you usually don't want to do such a thing in
>> a loop. Instead one should use an Iterator instead. Now, since there seems
>> to be no issue in Groovy, is it safe to go that direct way here?
>>
>> Kind regards, Matthias
>>
>>
>> --
>> Matthias F. Brandstetter
>> hai...@gmail.com
>> @maflobra
>
>

---------------------------------------------------------------------
To unsubscribe from this list, please visit:

http://xircles.codehaus.org/manage_email


Matthias F. Brandstetter

unread,
Mar 27, 2015, 6:11:55 AM3/27/15
to Groovy User Mailinglist
Thanks for the clarifications!

Nelson, Erick [HDS]

unread,
Mar 27, 2015, 2:37:21 PM3/27/15
to us...@groovy.codehaus.org

Bitwise and op is cheaper than modulo to determine if even/odd

def newlist = list.findAll { (it & 1) == 1 }

Owen Rubel

unread,
Mar 28, 2015, 2:32:30 PM3/28/15
to us...@groovy.codehaus.org
You are effectively iterating from my experience. 'Each' acts as an iterator except in the fact that you can't 'break' from the loop .

I believe you are modifying the original object while looping through a copy that is being looped through. It would be impossible to modify the same object you are currently looping through. :)

I mean correct me if I'm wrong though.

Tankerbay

unread,
Mar 28, 2015, 6:57:10 PM3/28/15
to us...@groovy.codehaus.org

I think the end result of all this is:
- yes, it works (surprisingly, regardless of why)
- no, you shouldn't do this.
- really, don't do this.

Sent from my phone.  Please excuse the mess.

Bob Brown

unread,
Mar 29, 2015, 4:08:39 AM3/29/15
to us...@groovy.codehaus.org
Note that the Iterator interface actually supplies remove(), so there are situations where modification is OK.

This is a good discussion of Java:

http://stackoverflow.com/questions/1196586/calling-remove-in-foreach-loop-in-java

While not using an 'each', this is the ‘canonical’ (IMHO) way to do this:

def list = [1, 2, 3]

final iterator = list.iterator()
while (iterator.hasNext()) {
if (iterator.next() < 3)
iterator.remove()
}
println list

Also more groovy:

def list = [1, 2, 3]

list.removeAll(list.findAll { it < 3 })
assert list == [3]

There is also:

def list = [1, 2, 3]
assert list - [1, 2] == [3]


BOB

Dinko Srkoč

unread,
Mar 29, 2015, 11:34:26 AM3/29/15
to us...@groovy.codehaus.org
On 29 March 2015 at 10:07, Bob Brown <b...@transentia.com.au> wrote:
> [...]
> def list = [1, 2, 3]
>
> list.removeAll(list.findAll { it < 3 })
> assert list == [3]

Since Groovy's version of `removeAll` accepts a Closure, `findAll` is,
in the above code, not necessary:

list.removeAll { it < 3 }

There also exists its inverse variant:

list.retainAll { it >= 3 }

Cheers,
Dinko

Bob Brown

unread,
Mar 30, 2015, 12:50:02 AM3/30/15
to us...@groovy.codehaus.org, dinko...@gmail.com

did not know about these. thanks.

Sent from my Mi phone

Reply all
Reply to author
Forward
0 new messages