Hi�Thierry,

Thanks for looking into this.
I have a slight preference for solution 2, just because code will not depend on the order of the announcements.

Cheers,
Andrei


On Tue, Feb 5, 2013 at 1:27 PM, Goubier Thierry <thierry.goubier@cea.fr> wrote:
Hi Andrei,

I tested with the following code which shows some interesting effects:


|list| �
list := ListComposableModel new.
list whenListChanged: [:aList|
Transcript crShow: aList = list listItems].
10 timesRepeat: [
� � � � list items: #(1 2).

� � � � list listItems. "Call listItems to set the cache"
� � � � list items: #(1 2 3).
� � � � list listItems].

And I either get true-true-true-true or, true-false-false-false ...

When I get true-true-true, in list listHolder announcer registry subscriptions, I have the action

� � � � [:aList| Transcript crShow: aList = list listItems]
set after the action
� � � � [self refreshListItems].

When I get true-false-false-false ad nauseam, in list listHolder announcer registry subscriptions, I have the action

� � � � [:aList| Transcript crShow: aList = list listItems]
set "before" the action
� � � � [self refreshListItems].

So, it seems that the use of an identitySet in subscriptions for an announcer has interesting side effects.

So, what should be done ?

1- Change subscription in Announcer SubscriptionRegistry from IdentitySet to OrderedCollection : it should keep the same behavior in all users of Announcers, since they have to be order independent (otherwise, they would fall for the same intermittent bug given the chance).

2- Change ListComposableModel to use a private event to update the cache before entertaining a #changedValue event.

I have a preference for 1-, because of my soft spot for programs that have a nice, predictable behavior :)

I've added a ticket

http://code.google.com/p/pharo/issues/detail?id=7420

Thierry

Le 05/02/2013 11:31, Andrei Vasile Chis a �crit :
Hi,

A very simple way to reproduce is to open a transcript and execute this
code:
50 timesRepeat: [|list|
list := ListComposableModel new.
list items: #(1 2).
list whenListChanged: [:aList|
Transcript crShow: aList = list listItems].
list listItems. "Call listItems to set the cache"
list items: #(1 2 3) ].

If should print both true and false.
Every time it prins false the events were triggered in the wrong order.

Andrei

On Tue, Feb 5, 2013 at 9:26 AM, Goubier Thierry <thierry.goubier@cea.fr
<mailto:thierry.goubier@cea.fr>> wrote:

� � Le 04/02/2013 17:27, Andrei Vasile Chis a �crit :

� � � � Hi,

� � � � While playing with spec I started to have some quite random and
� � � � strange behaviour.
� � � � I'll try to explain what happen, because it might be a nasty/not
� � � � that
� � � � nasty bug.

� � � � I had a ListComposableModel and some very simple code like:
� � � � � � � aListComposableModel whenListChanged: [ :aList |
� � � � aListComposableModel setSelectedItem: aList first ].

� � � � � From time to time this code seem to randomly fail because the
� � � � selection
� � � � wasn't set.
� � � � I manage to reduce the error to the following equivalent code:
� � � � � � �aListComposableModel whenListChanged: [ :aList |
� � � � aList = aListComposableModel listItems ].

� � � � What happen was that sometimes the list held by the model was the
� � � � previous list and not the new list,
� � � � which was actually correctly passes to the block.

� � � � This seem to be caused by the way ListComposableModel uses
� � � � announcers to
� � � � update its internal state.
� � � � In ListComposableModel>>__registerEvents there is the code:

� � � � � listHolder
� � � � whenChangedDo: [ self refreshListItems ].

� � � � This registers an announcement that should clear the cache:
� � � � ListComposableModel >>refreshListItems
� � � � listItemsCache := nil.
� � � � ^ self changed: #listElementAt:

� � � � In my code by using 'aListComposableModel whenListChanged:' I also
� � � � register an event that should be executed
� � � � when the elements of the list change.

� � � � So by now the listHolder containing the elements of the list has two
� � � � registered events.
� � � � The problem is that in order for this to work correctly they have to
� � � � execute in the order they
� � � � were added: first invalidate the cache, then the other events
� � � � registered
� � � � by the user.
� � � � However I do not think that announcers guarantee this. So from
� � � � time to
� � � � time they
� � � � execute in the wrong way. This means that the code in the
� � � � whenListChanged: block will
� � � � execute in a state in which internaly aListComposableModel still
� � � � contains the previous list.

� � � � Does this make sense?


� � Yes. It would mean the two announcers are kept in a Set... Maybe
� � having private and public events would be the way to go ? A private
� � one for clearing the cache and public ones, triggered after, for the
� � whenListChanged: block ?

� � It would be nice to have a test case checking that. I'd be
� � interested to know this can happen with announcements.

� � Thierry
� � --
� � Thierry Goubier
� � CEA list
� � Laboratoire des Fondations des Syst�mes Temps R�el Embarqu�s
� � 91191 Gif sur Yvette Cedex
� � France
� � Phone/Fax: +33 (0) 1 69 08 32 92 / 83 95




--
Thierry Goubier
CEA list
Laboratoire des Fondations des Syst�mes Temps R�el Embarqu�s
91191 Gif sur Yvette Cedex
France
Phone/Fax: +33 (0) 1 69 08 32 92 / 83 95