I added a slice for https://pharo.fogbugz.com/f/cases/13985/OrderedCollection-do-and-reverseDo-are-not-consistent-with-the-other-enumerations

On 09 Sep 2014, at 12:53, Nicolai Hess <nicolaihess@web.de> wrote:

> fogbugz entry for fixing the menu builder
> 13979
> PragmaMenuBuilder relies on modifying a collection while iterating over it.
>
>
>
>
> 2014-09-03 22:59 GMT+02:00 stepharo <stepharo@free.fr>:
> Clement we should fix the code of the menu :)
>
> Stef
>
>
> On 3/9/14 15:25, Cl��ment Bera wrote:
>> Hello,
>>
>> Sven you are right, the old compiler was consistent in the sense that it always iterated over all the elements, including the ones added with #add: and #addLast: while iterating over the collection.
>> On the other hand, VW is consistent with the Opal implementation for #to:do: in the sense that they iterate only on the elements of the collection excluding the ones added while iterating.
>>
>> #add:after: and co do not work well if you edit the collection while iterating over it for sure :).
>>
>> It's too late for "don't modify a collection while iterating it" because the system does it, for example, to build the world menu. So I think the solution is in two steps:
>> - removing code which edit the collection while iterating over it. As most frameworks work both on Pharo and VW and that the behavior is different I think there shouldn't be that much, so fixing the Pharo image may be enough.
>> - be consistent in our collection protocol, basically by rewriting do: and reverseDo: like that:
>>
>> FROM:
>> do: aBlock
>>�� "Override the superclass for performance reasons."
>>�� | index |
>>�� index := firstIndex.
>>�� [index <= lastIndex]
>>�� whileTrue:
>>�� [aBlock value: (array at: index).
>>�� index := index + 1]
>> TO:
>> do: aBlock
>>�� "Override the superclass for performance reasons."
>>�� firstIndex to: lastIndex do: [ :index |
>>�� aBlock value: (array at: index) ]
>>
>>
>>
>>
>>
>> 2014-09-03 14:05 GMT+02:00 Camille Teruel <camille.teruel@gmail.com>:
>>
>> On 3 sept. 2014, at 11:42, Cl��ment Bera <bera.clement@gmail.com> wrote:
>>
>> > Hello guys,
>> >
>> > I was looking into the OrderedCollection protocols recently to see how well the sista optimizer perform with it methods, and I realized that this is completely broken.
>> >
>> > For example:
>> >
>> > col := #(1 2 3 4 5) asOrderedCollection.
>> > col do: [ :elem | elem trace .
>> >�� �� �� ��elem < 4 ifTrue: [ col add: col size + 1 ]].
>> >
>> > => '12345678'
>> >
>> > However:
>> >
>> > col := #(1 2 3 4 5) asOrderedCollection.
>> > col collect: [ :elem | elem trace .
>> >�� �� �� ��elem < 4 ifTrue: [ col add: col size + 1 ]].
>> >
>> > => '12345'
>> >
>> > This means that #do: and #reverseDo: iterate over all the elements of the collection,*including* the ones that you are adding while iterating over the collection, whereas all the other OrderedCollection protocols, such as #collect:, #select:, iterates over all the elements of the collection, *excluding* the ones you are adding while iterating over the collection.
>> >
>> > Marcus argued that one should not edit a collection while iterating over it, however this point is not valid as the World menu relies on this feature, using #do: to iterate over the elements of the OrderedCollection including the one it is adding while iterating over the collection. Changing the implementation makes the world menu display half of its items.
>> >
>> > I don't like this difference because it is inconsistent. For example, refactoring a #do: into a #collect: can simply not work because they do not iterate over the same elements if you are editing the collection while iterating over it.
>> >
>> > In VW, the protocols are consistent and iterating over a collection never iterates over the elements one is adding while iterating over it. Therefore, I believe most frameworks should expect this behavior (at least the ones cross smalltalk) which sounds the most correct.
>> >
>> > I think we should fix the world menu implementation and make the protocols consistent. Alternatively, we can let VW be a much more consistent Smalltalk environment than Pharo. What do you think ?
>>
>>
>> The thing is to find alternative implementations that are efficient enough (no copy). Maybe we can get some inspirations from VW for that (how do they do BTW?).
>> You should also check with other kinds of collection because they may act differently than OrderedCollection.
>> So in the end it depends on the amount of effort needed to make all collections consistent with this new requirement and on the resulting overhead.
>> If it's too much, I think that following the old advice "don't modify a collection while iterating it" is enough. If one really needs to do such kind of things he should consider an alternative design like using a zipper for example.
>>
>> Camille
>>
>> >
>> > Clement
>> >
>>
>>
>>
>
>