Re: [Cdk-devel] Core JUnit coverage: what about stateChanged?

[rich a. <che...@ya...>]

Although I haven't tested the code, it should work
;-).

There are no threading issues I can find. Here are the
relevant ChemObject methods that I was basing the
suggestion on:

	/**
	 *  This should be triggered by an method that
changes the content of an object
	 *  to that the registered listeners can react to it.
	 */
	protected void notifyChanged() {
        if (getListenerCount() > 0) {
            Vector listeners =
lazyChemObjectListeners();
            for (int f = 0; f < listeners.size(); f++)
{
                ((ChemObjectListener)
listeners.elementAt(f)).stateChanged(new
ChemObjectChangeEvent(this));
            }
        }
	}


	/**
	 *  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
	 */
	protected void notifyChanged(ChemObjectChangeEvent
evt) {
        if (getListenerCount() > 0) {
            Vector listeners =
lazyChemObjectListeners();
            for (int f = 0; f < listeners.size(); f++)
{
                ((ChemObjectListener)
listeners.elementAt(f)).stateChanged(evt);
            }
        }
	}

Because neither of these methods produce a new thread,
there should be no threading problem. (IMO, it would
actually be quite chaotic if a notify method produced
a new thread to do notifications - you would never
know when clients get notified!). As a result, all
listeners (i.e. the private inner class listener in
the suggested unit test) will have their stateChanged
method called before the notifyChanged method exits.

Of course, this mechanism relies on the sublcass of
ChemObject (AtomContainer, etc.) doing the right thing
and actually invoking notifyChanged when its internal
state is changed. But this unit test will prove
whether or not this is really happening.

As an aside, in Octet (http://octet.sf.net), this
notification issue has been eliminated altogether by
applying the Immutable Pattern (see for example:
http://www.javalobby.org/articles/immutable/index.jsp)
to Molecule, Atom, AtomPair, and BondingSystem. In
other words, there are no mutator methods in the
interfaces, and the interface contracts specify
immutability. As a result, there is no need for
listeners, no need for clients to remember to add
themselves as listeners, and no possibility of
creating a memory leak by forgetting to remove a
listener. There is also no need to defensively copy
any of these objects - their state in the future is
guaranteed to be the same as it is now, so why bother?
This also eliminates synchronization issues in
threading situations becasue there is nothing to be
synchronized. Immutable objects can also be safely
used as keys in HashMaps and other hashing schemes.

The use of the Immutable Pattern drives a whole host
of downstream design decisions, which may be good or
bad, depending on the particulars of the system.

Anyway, if the suggested unit test fails, I'd be happy
to help track down any problems with it.

best,
rich

--- "E.L. Willighagen" <e.w...@sc...>
wrote:

> Op Thursday 30 December 2004 15:53, schreef rich
> apodaca:
> > Here's one way:
> >
> > public void
> testStateChanged_ChemObjectChangeEvent()
> > {
> >   ChemObjectListener listener =
> >     new ChemObjectListenerImpl();
> >   ChemObject chemObject = ... // get the
> ChemObject
> >
> >   chemObject.addListener(listener);
> >
> >   // now change chemObject state
> >
> >   assertTrue(listener.changed);
> > }
> 
> Does this actually work? I'd guessed that this would
> not work because of 
> timing conditions, but apparently event are passed
> by to all listeners in the 
> *same* thread as where the test is done?
> 
> If the use the same thread, then the above should
> work.
> 
> Many thanx for your help.
> 
> Egon
> 
> 
>
-------------------------------------------------------
> The SF.Net email is sponsored by: Beat the
> post-holiday blues
> Get a FREE limited edition SourceForge.net t-shirt
> from ThinkGeek.
> It's fun and FREE -- well,
> almost....http://www.thinkgeek.com/sfshirt
> _______________________________________________
> Cdk-devel mailing list
> Cdk...@li...
>
https://lists.sourceforge.net/lists/listinfo/cdk-devel
> 



		
__________________________________ 
Do you Yahoo!? 
Meet the all-new My Yahoo! - Try it today! 
http://my.yahoo.com