[ DEFUSED ] Bug in View.notifyObservers()

[thomasvst]

I get a strange behavior when registering a mediator interested in a notification being processed by the View.notifyObservers() function.

I've a simple application that notify the observers of the application startup.

:

<?xml version="1.0" encoding="utf-8"?>
<mx:Application
creationComplete="handleCreationComplete()"
xmlns:mx="http://www.adobe.com/2006/mxml" layout="absolute">

<mx:Script>
<![CDATA[
import org.puremvc.patterns.observer.Notification;

private function handleCreationComplete() : void {
// notify application startup...
ApplicationFacade.getInstance().notifyObservers(
new Notification(ApplicationFacade.STARTUP, this));
}

]]>
</mx:Script>
</mx:Application>
The ApplicationFacade assign the StartupCommand to the STARTUP notification.

:

package {

import org.puremvc.patterns.facade.Facade;
import org.puremvc.interfaces.IFacade;

public class ApplicationFacade extends Facade implements IFacade {

public static const STARTUP : String = "STARTUP";

public static function getInstance() : ApplicationFacade {
if (instance == null) { instance = new ApplicationFacade();}
return instance as ApplicationFacade;
}

override protected function initializeController() : void  {
super.initializeController();
registerCommand(STARTUP, StartupCommand);
}
}
}
The StartupCommand create & register a mediator...

:

package {

import org.puremvc.patterns.command.SimpleCommand;
import org.puremvc.interfaces.INotification;

public class StartupCommand extends SimpleCommand {

override public function execute(note : INotification) : void {
// initilize & register a mediator
facade.registerMediator(new TestMediator());
}
}
}
The mediator created is interested in the STARTUP notification...

:

package {

import org.puremvc.patterns.mediator.Mediator;
import org.puremvc.interfaces.INotification;

public class TestMediator extends Mediator {

override public function listNotificationInterests() : Array {
return [ApplicationFacade.STARTUP];
}

override public function handleNotification(note : INotification) : void {
switch (note.getName()) {
case ApplicationFacade.STARTUP:
trace("Bug !!!");
break;
}
}

override public function getMediatorName() : String {
return "TestMediator";
}
}
}
The "handleNotification" should be called if a new STARTUP notification is lunched... BUT it is called direcly after the mediator creation... (because the for loop in the View.notifyObservers is not yet completed)

That not what the asdoc said :

All previously attached <code>IObservers</code> for this <code>INotification</code>'s list are notified and are passed a reference to the <code>INotification</code> in the order in which they were registered.

[puremvc]

This is a Jimi Hendrix-style feedback loop ready to happen.

Firstly, STARTUP should only be sent once. It should fire a command that creates the initially available proxies followed by the initially available mediators for the application. That's it, startup has happened.

If sent again, the same Command gets called again, which wants to create the Mediator again. and register it with the View, which, since it has the same name, simply overwrites the previous Mediator in the Vew, and the Observer that it creates for this second generation Mediator overwrites the Observer (again keyed by name).

In short, don't try to reuse / repurpose notifications, particularly your STARTUP notification.

-=Cliff>

[thomasvst]

I don't think you really understand what I mean...

Have a look at the notifyObserver() method in the View :

:

public function notifyObservers( notification:INotification ) : void
        {
            if( observerMap[ notification.getName() ] != null ) {
                var observers:Array = observerMap[ notification.getName() ] as Array;
                for (var i:Number = 0; i < observers.length; i++) {
                    var observer:IObserver = observers[ i ] as IObserver;
                    observer.notifyObserver( notification );
                }
            }
        }
If the line observer.notifyObserver( notification ); cause (in the observer) the creation and the registration of a mediator interested in the notification under process, the observer created by the registerMediator() function will be added in observerMap[ notification.getName() ] AND will be precessed in the current for loop because the test expression is observers.length that will dynamically change after the observer registration...

Instead you could use something like :

:

var observers:Array = observerMap[ notification.getName() ] as Array;
var stopIndex : int = observers.length;
for (var i:Number = 0; i < stopIndex; i++) {

[puremvc]

Thomas,

I do follow what you mean, but I'm not sure the framework needs to change because its possible to put it into a feedback loop.

Ordinary best practices in development should keep you from creating such a loop.

Just as Jimi had to physically walk over to the amp and hold his guitar up to the speaker to make his feedback happen,in your case its deliberately sending the STARTUP notification more than once.

In both cases my advice would be if it doesn't sound good, then don't do it.
-=Cliff>

[thomasvst]

but I'm not sure the framework needs to change because its possible to put it into a feedback loop.

I think that something must change : the ASDoc or the implementation... because the ASDoc saids :

All previously attached IObservers for this INotification's list are notified and are passed a reference to the <code>INotification</code> in the order in which they were registered.

and the implementation doesn't respect it.

Don't you agree ?

[puremvc]

I don't think the documentation or the code needs amendment at all.

Your original issue was this:

If the line observer.notifyObserver( notification ); cause (in the observer) the creation and the registration of a mediator interested in the notification under process, the observer created by the registerMediator() function will be added in observerMap[ notification.getName() ] AND will be precessed in the current for loop because the test expression is observers.length that will dynamically change after the observer registration...

...and you suggested that in order to stop this race condition, the notifiyObserver method should not iterate towards observers.length as it may change if we get loopy and register something that listens for the notification that registered it.

Where exactly is the framework failing to notify previously attached observers? It always notifies previously attached observers.

What you are asserting is that it should not notify observers that weren't previously registered, but were instead created during the act of the current notification. Something that can only happen if you have followed the bad practice of creating a feedback loop.

Logically, when will you ever need to have a Mediator listen for and respond to the event that creates it? It's like a doctor thinking if only he'd been able to help at his own birth, things would've gone better. He's not interested in his own birth even if he is interested in facilitating the birth of others.

If for some reason, you want to reuse a notification in this way but find yourself battling this race condition, then try using the 'type' parameter of the Notification constructor to disambiguate notifications that should or should not be responded to by a given Mediator. That is, inside handleNotification for the Mediator check to see notification.getType() is set to a value you can logically act on, otherwise ignore the notification and do nothing.

-=Cliff>