[ FIXED ] removeMediator (Version 1.5) bug

[Whitechapel]

Hello,

To justify breaking the rule 'thou shalt not remove a mediator'...

I have a use case that seems to encourage the indiscriminate creation and removal of mediators - (Skipping the details) a document based application with many specialized editors/monitors that come and go all over the place each subscribing to events on a particular data record.

In my quest to remove a mediator whenever I see one, I may have come across a bug in the  View.removeMediator function (Version 1.5).

The function sets the observerMap[ notificationName ] = null; when there are no more observers for a particular notification.

As noted in an earlier post the key is not removed from the observerMap.

Since the removeMediator function blindly iterates over the keys in the observerMap, in order to retrieve an Array of observers for the the current notificationName, an exception is thrown the next time a call to removeMediator is made.

Specifically ...

:

var observers:Array = observerMap[ notificationName ];
... observers will be null; and the following line will fail:

:

      for ( var i:int=0;  i< observers.length; i++ )

A possible solution is to delete the key and perhaps guard against null values  before the loop.

Here is a version that seems to get the job done - excuse the nasty trace statements I needed a quick visual way of checking the before and after states of the observerMap.

:

public function removeMediator( mediatorName:String ) : void
{
//
// nasty trace to see what we are dealing with
//
trace("BEFORE ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++");
var n = 0;
for ( var before:String in observerMap ) {
trace("++++++:[" + n + "]" + before + ":" + observerMap[before]);
n++;
}

// Remove all Observers with a reference to this Mediator
// also, when an notification's observer list length falls to
// zero, null it.
for ( var notificationName:String in observerMap ) {

var observers:Array = observerMap[ notificationName ];

                //
                // !!!! belt and braces test
                //
if(observers != null)
{
for ( var i:int=0;  i< observers.length; i++ ) {

if ( Observer(observers[i]).compareNotifyContext( retrieveMediator( mediatorName ) ) == true ) {

observers.splice(i,1);

if ( observers.length == 0 ) {

observerMap[ notificationName ] = null;

                                                //
                                                // !!!! remove the key from the map
                                                //
delete observerMap[ notificationName ];

break;

}
}
}
}
}

//
// more nasty trace to see what is left
//
trace("AFTER -------------------------------------------------------------------");
n = 0;
for ( var after:String in observerMap ) {
trace("------:[" + n + "]" + after + ":" + observerMap[after]);
n++;
}

// Remove the reference to the Mediator itself
// mediatorMap[ mediatorName ] = null;
mediatorMap[mediatorName] = null;
}

HTH and thanks for the great work.
J.

[puremvc]

J,

Well, it's not really written on a tablet or anything, but removeMediator isn't really that often used. But when you need to use it, you need to use it.

So, I'm having a look at your suggestion. The following:

:

//
// !!!! remove the key from the map
//
delete observerMap[ notificationName ];

is really not doing anything beyond the

:

observerMap[ notificationName ] = null;

that precedes it.

From Programming ActionScript 3.0 > Core ActionScript 3.0 Data Types and Classes > Working with Arrays

You may come across code that uses the delete operator on an array element. The delete operator sets the value of an array element to undefined, but it does not remove the element from the array.

The problem boils down to there being no proper way to remove a key from an associative array, you can only set its value to null. Short of copying the array element by element, leaving out the keys with null values, and replacing it. I'm still not sure I want to do that here. Primarily because I don't think it would be performant in those use cases which do drive us to use removeMediator a lot.

It is a trade off between a squeeky clean memory profile (where we remove every key in every map leading to zero build up) AND performance when we are adding and removingMediators a lot. Those keys are tiny and shouldn't ever really cause a problem it's the objects they point to that we're really worried about tossing to the GC.

I believe that that we will likely find that adding the

:

if(observers != null)
check will suffice, but I don't want to do it hastily. A soon as I can, I'll pound on this and arrive at a conclusion and likely a 1.6 maintenance release.

Thanks much for your input on this, I really appreciate it.
-=Cliff>

[Whitechapel]

Thanks for the response and please forgive the dumb question -  my knowledge of Actionscript is a bit rusty and I'm new to PureMVC.



My understanding of the Array class is that it's primary role is one of maintaining an ordered list of numerically indexed elements as opposed to dynamic instance variables. So out of curiosity are there any advantages to be gained by using an instance of Array class as opposed to one of Object when an associative array (or hash) is called for?

J.

[puremvc]

array.length

-=Cliff>

[Whitechapel]

I'm a bit sleep deprived so I may be missing something obvious, but the observerMap is only used as an associate array so Array.length will return 0 every time.

:

var map:Array = new Array();

map["key-a"] = ["thing 1a"];
map["key-a"].push("thing 1b");
map["key-a"].push("thing 1c");
map["key-b"] = ["thing 2a"];
map["key-b"].push("thing 2b");
map["key-b"].push("thing 2c");

trace(map.length); // 0;

So...

:

map[key] = null;
delete map[key];

... is still an appropriate method of removing keys from the observerMap as it is not numerically indexed. In fact the map could be a simple Object.

[puremvc]

Watching the mediatorMap and observerMap while stepping through a removeMediator, the delete operator appears to remove associative elements just fine. This despite:

Programming ActionScript 3.0 > Core ActionScript 3.0 Data Types and Classes > Working with Arrays

You may come across code that uses the delete operator on an array element. The delete operator sets the value of an array element to undefined, but it does not remove the element from the array.

The manual is only referring to numeric elements in the array and not to associative ones. Tricky, tricky.

So, based on Whitechapel's originally described scenario at the top of the thread, I've amended the logic of the removeMediator method to use the delete operator to remove observerMap and mediatorMap entries. No check for null arrays is necessary, because the key doesn't hang around anymore.

In the Framework API:
http://puremvc.org/component/option,com_wrapper/Itemid,118/

Examine the method:
org.puremvc.core.view.View.removeMediator

Also, a new test has been added to the PureMVC Framework Test Suite to test that successive register and removal of the same Mediator does not cause error, and a removeMediator on the same Mediator name when it has already been removed does not cause exception.

In the Framework Test Suite API:
http://puremvc.org/component/option,com_wrapper/Itemid,122/

Examine the test method:
org.puremvc.core.view.ViewTest.testSuccessiveRegisterAndRemoveMediator

And see the test pass on the download page for the Framework Test Suite API:
http://puremvc.org/content/view/14/49/

The new version of PureMVC sporting this fix is 1.6.

-=Cliff>