This is something I'm interested in too. I thought that this was a design decision between a) and b) a) you can have single announcer to whom you would subscribe and then manually filter each fired event whether it something that interests the listener b) subscribe directly to the object you're interested in so when an event is fired you can act on it straight away (is there another option c), d), e) ... which I don't see?) Wouldn't be b) much, much, much faster? I can't imagine that every time one ValueHolder would change every one observer of common ValueHolder announcer would have to check whether it's event from the observed ValueHolder. And I would imagine a) being little bit more versatile... for example I have a model composed of elements (element has direct reference to the owning model) and every time an element changes I need model to know it - I can either subscribe to every one of them (and unsub when they are removed), or I could have one common announcer and then the model would check whether it owns said element.

But I would be interested in hearing other problems/advantages that would arise from choosing one over the other.

We could have a special valueholder that is held at the level of the applicationModel in spec. Now Nautilus leaking memory. when I open latest Pharo I get 30 then open nautilus once 700! then close nautilus there are not gone then open nautilus once more

Peter

On Fri, Nov 14, 2014 at 12:01 PM, stepharo <stepharo@free.fr> wrote:

I checked a bit deeper

Nautilus is not implemented in Spec. But senders and implementors are and they also leak memory. Around 400 Announcers after each opening and closing.

Now

whenChangedDo: aBlock

| block | block := [:announcement :ann | aBlock cull: announcement newValue cull: announcement oldValue cull: announcement cull: ann ]. announcer when: ValueChanged do: block

Stef, maybe for a quick test and see if the situation changes because of it, you can change that to whenChangedDo: aBlock | block subscription | block := [:announcement :ann | aBlock cull: announcement newValue cull: announcement oldValue cull: announcement cull: ann. *subscription subscriber removeSubscription: subscription* ]. *subscription := *announcer when: ValueChanged do: block. modulo error handling :)

can also be part of the cause of some of our problems.

There are 146 senders and many of them could just be replaced by whenChanged:send:to: and whenChanged:send:to:with:

Finally we were discussing with igor about the general API of spec model: We are thinking that the following patterns are bloating the interface for an not obvious gain.

whenWindowChanged: aBlock

window whenChangedDo: aBlock

whenShortcutsChanged: aBlock <api: #event> "Set a block to value when the shortcuts block has changed"

additionalKeyBindings whenChangedDo: aBlock

On 14 November 2014 16:25, Henrik Johansen <henrik.s.johansen@veloxit.no> wrote:

...

On 14 Nov 2014, at 12:01 , stepharo <stepharo@free.fr> wrote:

I checked a bit deeper

Nautilus is not implemented in Spec. But senders and implementors are and they also leak memory. Around 400 Announcers after each opening and closing.

Now

whenChangedDo: aBlock

| block | block := [:announcement :ann | aBlock cull: announcement newValue cull: announcement oldValue cull: announcement cull: ann ]. announcer when: ValueChanged do: block

can also be part of the cause of some of our problems.

There are 146 senders and many of them could just be replaced by whenChanged:send:to: and whenChanged:send:to:with:

Finally we were discussing with igor about the general API of spec model: We are thinking that the following patterns are bloating the interface for an not obvious gain.

whenWindowChanged: aBlock

window whenChangedDo: aBlock

whenShortcutsChanged: aBlock <api: #event> "Set a block to value when the shortcuts block has changed"

additionalKeyBindings whenChangedDo: aBlock

I'd argue the window and additionalKeyBindings' whenChangedDo: implementations are unnecessary bloat as well. Why on earth would you limit yourself to a an API where you have to use seperate methods to register for every single announcement type?

To me, myObject additionalKeyBindings when: ValueChanged do: aBlock is just as easy, if not easier to read compared to myObject whenShortCutsChanged: []

Not to mention, it easily lets you do unsubscription when you expect not to outlive the source (through the 3rd block parameter, whose *main purpose for even being there* is enabling easy unsubscription), rather than discarding it during useless indirection and leaving the task harder to do later on?

+1

in general, i would prefer to see: myModel propertyXYZ whenChangedDo: [...] or myModel properyXYZ whenChangedSend: #foo to: bar . - you don't have to know that it uses ValueChanged announcement. In this regard, such information is excessive, since it is standard ValueHolder. - once you learned how to access/use single property, you can use any other, because it will have same API, and so you don't have to remember numerous 'whenPropertyXYZChanged:...' hoping it is there and spelled correctly. 1. because, how i see it, the idea is , that your model exposes certain _public_ property, which application can read or change (using value/value: accessors ) or wants to be notified when it changed by others. And this can be done by simply exposing the accessor to its ValueHolder to outside. 2. the extra API is just a source of confusion. And the above example clearly illustrates why: while internally, property is named 'additionalKeyBindings', but the method for subscribing to its changes named 'whenShortcutsChanged:' . One might expect that to access such property, there should be 'shortcuts' accessor, right? No! That would be too easy. It is additionalKeyBindings! P.S. The Demeter law is not a dogma. Cheers,

Henry

-- Best regards, Igor Stasenko.

Hi Henrik Thanks for your point :) I agree with you. Esteban prefered the current solution because of law of demeter but I do not think that this is good. So we will rewrite all these methods. If you want to help you are welcome. Stef On 14/11/14 16:25, Henrik Johansen wrote:

...

On 14 Nov 2014, at 12:01 , stepharo <stepharo@free.fr> wrote:

I checked a bit deeper

Nautilus is not implemented in Spec. But senders and implementors are and they also leak memory. Around 400 Announcers after each opening and closing.

Now

whenChangedDo: aBlock

| block | block := [:announcement :ann | aBlock cull: announcement newValue cull: announcement oldValue cull: announcement cull: ann ]. announcer when: ValueChanged do: block

can also be part of the cause of some of our problems.

There are 146 senders and many of them could just be replaced by whenChanged:send:to: and whenChanged:send:to:with:

Finally we were discussing with igor about the general API of spec model: We are thinking that the following patterns are bloating the interface for an not obvious gain.

whenWindowChanged: aBlock

window whenChangedDo: aBlock

whenShortcutsChanged: aBlock <api: #event> "Set a block to value when the shortcuts block has changed"

additionalKeyBindings whenChangedDo: aBlock I'd argue the window and additionalKeyBindings' whenChangedDo: implementations are unnecessary bloat as well. Why on earth would you limit yourself to a an API where you have to use seperate methods to register for every single announcement type?

To me, myObject additionalKeyBindings when: ValueChanged do: aBlock is just as easy, if not easier to read compared to myObject whenShortCutsChanged: []

Not to mention, it easily lets you do unsubscription when you expect not to outlive the source (through the 3rd block parameter, whose *main purpose for even being there* is enabling easy unsubscription), rather than discarding it during useless indirection and leaving the task harder to do later on?

Cheers, Henry

On 15 November 2014 22:49, stepharo <stepharo@free.fr> wrote:

I get dizzy (may be time to go to sleep) but I wonder why do we need to check first the the window changed and after that it got closed? Especially since whenWindowChanged: presuppose that we have access to window. Am I missing something?

self whenWindowChanged: [ :w | w whenClosedDo: [ self clear ] ].

ComposableModel>>whenWindowChanged: aBlock window whenChangedDo: aBlock

WindowModel>>whenClosedDo: aBlock

isClosedHolder whenChangedDo: [:value | value ifTrue: [ aBlock value ] ]

i am lost with this code. i cannot understand what is it for? because by construction you start from building GUI representation of your model.. in this case - you create a controller - and create a window (or widget AKA presenter) connected to it if you wanna present your model differently, you just start from fresh controller/presenter pair but changing the presenter while keeping controller intact (hence whenWindowChanged:) ? in that case, if you really want that, you will need to wire all events from widget once you set it up.. and not just close events.. but all e.g.: self whenWindowChanged: [ :w | w whenClosedDo: [ ]. w whenKeyPressedDo: [..]. w whenInvalidUserDo: [..]. w whenSomethingWentWrongDo: [..]. w whateverHappensDo: [..]. ]. i don't know, maybe i don't see it clearly. just trying to understand. -- Best regards, Igor Stasenko.

On 17 November 2014 12:08, Henrik Johansen <henrik.s.johansen@veloxit.no> wrote:

On 15 Nov 2014, at 10:49 , stepharo <stepharo@free.fr> wrote:

I get dizzy (may be time to go to sleep) but I wonder why do we need to check first the the window changed and after that it got closed? Especially since whenWindowChanged: presuppose that we have access to window. Am I missing something?

self whenWindowChanged: [ :w |

w whenClosedDo: [ self clear ] ].

ComposableModel>>whenWindowChanged: aBlock window whenChangedDo: aBlock

WindowModel>>whenClosedDo: aBlock

isClosedHolder whenChangedDo: [:value | value ifTrue: [ aBlock value ] ]

IMHO, APIs that, in essence, glue together announcers and non-self subscribers, are bad, it's an extra level of indirection with little gains. (And quite a few restrictions) Let the announcers be resolvable, and leave it to subscribers to do the actual subscribing.

To me, it looks like ValueHolders are being used for the sake of using ValueHolders, rather than delivering events in the simplest way possible... Say you do something like

WindowModel >> announcer windowAnnouncer := Announcer new

WindowModel >> createWindow window := blahblah. window announcer when: GenericWindowEvent do: [:event self announcer announce: event ]-

And all the modelling widgets are free to respond to any window events by subscribing to the models windowAnnouncer, rather than registering blocks for value holders linked to single type changes.

users of whenClosedDo: would instead do something like (usually in an initialize method where it's passed as a windowModel parameter) windowModel announcer when: WindowClosed do: aBlock and as such, the code that starts with self whenWindowChanged (I assume in the initialize method on a ComposableModel subclass?) window announcer when: WindowClosed do: [self clear]

After your post, i think i figured, where i see the design flaw. And can articulate it: - the widget model can expose certain properties , like extent, color etc to its controller - the controller model can expose certain properties of domain object to the widget (for example if we take a list model, it could be: list size, list items, selected item index etc) - both controller and widget models can expose various events to any interested party(es) via subscription mechanism To me it is clear that properties are not events.. They are conceptually distinct, and there's different mechanisms how and where you use them: you treat one as data, and other as behavior. Yes, smalltalk largely blurs the distinction between data & behavior since everything is an object, but i think this is the case where you want to keep distinction clear: - the fact that you should be able to customize your reaction on different events by implementing different behavior, don't means that you should turn that into property e.g. "property which holds a block that will be evaluated when something happens". No, that's complete nonsense.. when you having announcement mechanism, you can simply react to announced event the way you want. And you certainly don't want to react on events like 'what we gonna do when someone changes the block that will be evaluated when certain event triggered'. Because you want to react on user events, not to events that trigger when your application changes the way it wants to handle the events, nor the events that trigger when when your application changes the way it wants to handle the events, that trigger when your application changes the way it wants to handle the events, nor the events that trigger when when your application changes the way it wants to handle the events, that trigger when when your application changes the way it wants to handle the events, that trigger when your application changes the way it wants to handle the events ... (you get an idea :) For example look at: AbstractWidgetModel subclass: #CheckBoxModel instanceVariableNames: 'actionWhenActivatedHolder actionWhenDesactivatedHolder stateHolder labelClickableHolder labelHolder' classVariableNames: '' category: 'Spec-Core-Widgets' It is clear that actionWhenActivatedHolder and actionWhenDesactivatedHolder are events which should just be announced by widget, that anyone would want to handle. period. you don't need ValueHolder(s) to hold blocks running in case of such events. Phew...

On 15 Nov 2014, at 10:41 , stepharo <stepharo@free.fr> wrote:

initializePresenter "Used to specify the subwidgets, and/or to bind them together" "By default, do not do anything"

extentHolder whenChangedDo: [:ex | self widget ifNotNil: [:widget | (widget respondsTo: #extent:) ifTrue: [ widget extent: ex ]]].

may be by design the ifNotNil should not be needed and the respondsTo too....

pffff.

I don't get it, doesn't the presenter already have an initialize: method that gets passed in its model? So a more "proper" thing to do is PresenterClassesThatCaresAboutExtent >> initializeFromModel: myModel myModel extentHolder when: ChangedValue do: [:ann | self extent: ann newValue] Cheers, Henry