Hi, I've just run into quite a nasty surprise ~~~~~~~~~~~~~~~~ col := #(1 2 3 4 5) asOrderedCollection. col do: [ :each | col remove: each. ]. col ~~~~~~~~~~~~~~~~ it throws "NotFound: nil not found in OrderedCollection" I tested it also in Pharo3 and there it just silently removed first, third and fifth element. Looking at the implementation of OrderedCollection>>do: it seems that doing what I'm doing is a bad idea. So what shall one do:? col copy do: [...] ? Why can't do: be a bit smarter? Optimization? Thanks, Peter
Peter, In general, removing from a Collection while iterating through it leads to undefined results. The same is true for adding. Just never do it Joachim
Am 02.04.2015 um 08:22 schrieb Peter Uhnák <i.uhnak@gmail.com>:
Hi,
I've just run into quite a nasty surprise ~~~~~~~~~~~~~~~~ col := #(1 2 3 4 5) asOrderedCollection. col do: [ :each | col remove: each. ]. col ~~~~~~~~~~~~~~~~ it throws "NotFound: nil not found in OrderedCollection" I tested it also in Pharo3 and there it just silently removed first, third and fifth element.
Looking at the implementation of OrderedCollection>>do: it seems that doing what I'm doing is a bad idea.
So what shall one do:? col copy do: [...] ?
Why can't do: be a bit smarter? Optimization?
Thanks, Peter
On Thu, Apr 2, 2015 at 8:30 AM, Joachim Tuchel <jtuchel@objektfabrik.de> wrote:
In general, removing from a Collection while iterating through it leads to undefined results. The same is true for adding.
That is a question of approach; apart from mentioned #copy I could also do something like ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ col := #(1 2 3 4 5) asOrderedCollection. [col isNotEmpty] whileTrue: [ col remove: col first ]. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ if I had an Iterator I could also reset it after each change. What I am asking is what the best approach would be - I want do some processing for each item and remove it from the collection after it is done. Peter
On Thu, Apr 2, 2015 at 9:33 AM, Peter Uhnák <i.uhnak@gmail.com> wrote:
On Thu, Apr 2, 2015 at 8:30 AM, Joachim Tuchel <jtuchel@objektfabrik.de> wrote:
In general, removing from a Collection while iterating through it leads to undefined results. The same is true for adding.
That is a question of approach; apart from mentioned #copy I could also do something like ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ col := #(1 2 3 4 5) asOrderedCollection. [col isNotEmpty] whileTrue: [ col remove: col first ]. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ if I had an Iterator I could also reset it after each change.
What I am asking is what the best approach would be - I want do some processing for each item and remove it from the collection after it is done.
"If process indicates if it is worth removing the item" processed:= col select: [ :each | col process: each ]. col removeAll: processed. It is an interesting dicussion actually :-) Phil
Peter
Peter, several approaches have been shown already. I'd like to add: * Iterate over a copy and remove from the original (slower because of lookup in the original) * If possible, sort the collection and use something like [col atEnd] whileTrue:/whileFalse: with removeFirst/removeLast * Select: a collection of items to be removed and then do a removeAll: * Use #- to remove one collection from the other * Iterate over the original and add only the wanted objects to a new collection which will then be used in the future - Streams may be helpful here * I have never used XStreams, but there may even be some nice options to find in it HTH Joachim Am 02.04.15 um 09:33 schrieb Peter Uhnák:
On Thu, Apr 2, 2015 at 8:30 AM, Joachim Tuchel <jtuchel@objektfabrik.de <mailto:jtuchel@objektfabrik.de>> wrote:
In general, removing from a Collection while iterating through it leads to undefined results. The same is true for adding.
That is a question of approach; apart from mentioned #copy I could also do something like ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ col := #(1 2 3 4 5) asOrderedCollection. [col isNotEmpty] whileTrue: [ col remove: col first ]. ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ if I had an Iterator I could also reset it after each change.
What I am asking is what the best approach would be - I want do some processing for each item and remove it from the collection after it is done.
Peter
On Thu, Apr 2, 2015 at 8:22 AM, Peter Uhnák <i.uhnak@gmail.com> wrote:
Hi,
I've just run into quite a nasty surprise ~~~~~~~~~~~~~~~~ col := #(1 2 3 4 5) asOrderedCollection. col do: [ :each | col remove: each. ]. col ~~~~~~~~~~~~~~~~ it throws "NotFound: nil not found in OrderedCollection" I tested it also in Pharo3 and there it just silently removed first, third and fifth element.
Looking at the implementation of OrderedCollection>>do: it seems that doing what I'm doing is a bad idea.
So what shall one do:? col copy do: [...] ?
Yes, as a general rule, do not modify a collection you are iterating on, so, doing col copy do: looks right. removeAllSuchThat: aBlock "Evaluate aBlock for each element and remove all that elements from the receiver for that aBlock evaluates to true. Use a copy to enumerate collections whose order changes when an element is removed (i.e. Sets)." self copy do: [:each | (aBlock value: each) ifTrue: [self remove: each]] If the collection is huge, that can be an issue.
Why can't do: be a bit smarter? Optimization?
If you have the group of things to remove you can do aCollection removeAll: anotherCollection Phil
Thanks, Peter
col := #(1 2 3 4 5) asOrderedCollection. col copy do: [ :each | col remove: each. ]. col will work instead Alexandre -- _,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;: Alexandre Bergel http://www.bergel.eu ^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;._,.;:~^~:;.
On Apr 2, 2015, at 3:22 AM, Peter Uhnák <i.uhnak@gmail.com> wrote:
Hi,
I've just run into quite a nasty surprise ~~~~~~~~~~~~~~~~ col := #(1 2 3 4 5) asOrderedCollection. col do: [ :each | col remove: each. ]. col ~~~~~~~~~~~~~~~~ it throws "NotFound: nil not found in OrderedCollection" I tested it also in Pharo3 and there it just silently removed first, third and fifth element.
Looking at the implementation of OrderedCollection>>do: it seems that doing what I'm doing is a bad idea.
So what shall one do:? col copy do: [...] ?
Why can't do: be a bit smarter? Optimization?
Thanks, Peter
On 04/01/2015 11:22 PM, Peter Uhnák wrote:
col := #(1 2 3 4 5) asOrderedCollection. col do: [ :each | col remove: each. ]. col
As a general rule in Smalltalk, a lot of hassle can be avoided avoiding loops. If you think you need a loop, stop and find a better way, it likely already exists, in this case as someone else already mentioned either #removeAll: or #removeSuchThat:. Smalltalk isn't procedural, loops aren't your bread and butter. -- Ramon Leon
participants (6)
-
Alexandre Bergel -
Joachim Tuchel -
jtuchel＠objektfabrik.de
-
Peter Uhnák -
phil＠highoctane.be -
Ramon Leon