[Cdk-devel] RFC: factoring out the IChemObject listener framework

[Egon W. <ego...@gm...>]

Hi all,

recently Stefan commited a patch (2961963: "Performance tuning for
NNChemObject", [0]) to further improve the performance of the
nonotification implementation of the CDK data model.

The current data model API requires the implementations to implement a
framework to send around notifications, which we know (discussed
somewhere in the email archives), that sending around notifications is
a performance drop. The introduction a long time ago of the
IChemObjectBuilder allowed the implementation of alternative data
models, resulting in the NoNotification module... now, that module
still has to implement the notification framework, so a hack was
introduced to mark a class as 'notifying' or not.

Now, last summer I wrote a patch to factor out the notification
framework into add-on the add on interface IChemObjectChangeNotifier,
so that data model implementations can choose if they want to support
it or not:

public interface IChemObjectChangeNotifier extends IChemObject{

       /**
        * Use this to add yourself to this IChemObject as a listener.
In order to do
        * so, you must implement the ChemObjectListener Interface.
        *
        * @param  col  the ChemObjectListener
        * @see         #removeListener
        */
       public void addListener(IChemObjectListener col);

       /**
        * Returns the number of ChemObjectListeners registered with this object.
        *
        * @return    the number of registered listeners.
        */
       public int getListenerCount();

       /**
        * Use this to remove a ChemObjectListener from the ListenerList of this
        * IChemObject. It will then not be notified of change in this
object anymore.
        *
        * @param  col  The ChemObjectListener to be removed
        * @see         #addListener
        */
       public void removeListener(IChemObjectListener col);

       /**
        * Set a flag to use or not use notification. By default it should be set
        * to true.
        *
        * @param bool if true, then notification messages are sent.
        * @see        #getNotification()
        */
       public void setNotification(boolean bool);

       /**
        * Returns the flag that indicates whether notification
messages are sent around.
        *
        * @return true if messages are sent.
        * @see    #setNotification(boolean)
        */
       public boolean getNotification();

       /**
        * This should be triggered by an method that changes the
content of an object
        * to that the registered listeners can react to it.
        */
       public void notifyChanged();

       /**
        * This should be triggered by an method that changes the
content of an object
        * to that the registered listeners can react to it. This is a version of
        * notifyChanged() which allows to propagate a change event
while preserving
        * the original origin.
        *
        * @param  evt  A ChemObjectChangeEvent pointing to the source of where
        *                      the change happend
        */
       public void notifyChanged(IChemObjectChangeEvent evt);

}

There methods are currently all in IChemObject.

The patch I wrote applies this to all data models, but I never
submitted it for review, as I wanted to update the nonotification
module first. But I never got around to that yet. Now, with Stefan's
patch working this way, and he clearly needs it for performance
reasons, I think it is time to make the transition.

The patch does not apply to the current master, so I kindly ask
everyone (Stefan, Christoph, Rajarshi, ...) to review the patch I
wrote last summer, which I have uploaded to GitHub, based on the
master at that time:

http://github.com/egonw/cdk/tree/32-extractChemobjectNotifierInterface

It basically consists of two patches, this one being the crucial one:

http://github.com/egonw/cdk/commit/bc0d5de0eb2867292812675a47a58d71af03e659

Looking forward to all positive and negative feedback!

Egon

0.http://sourceforge.net/tracker/?func=detail&aid=2961963&group_id=20024&atid=320024



-- 
Post-doc @ Uppsala University
Proteochemometrics / Bioclipse Group of Prof. Jarl Wikberg
Homepage: http://egonw.github.com/
Blog: http://chem-bla-ics.blogspot.com/
PubList: http://www.citeulike.org/user/egonw/tag/papers