RE: RemoteNotificationClientHandler fix

Hi Brian,

> > It is maybe worth to add start() and stop() to the 
> > RemoteNotificationClientHandler interface, and invoke them directly 
> > like your fix suggested.
> 
> This was why I put it into the AbstractRNCH.  Because
> the threading is localized to that class, why should
> the interface care about it?  

I see it as a semantic call. From the connector I tell to the RNCH to start fetching notification:

notificationHandler.start();

And I tell it to stop when I close the connector.

> Now if you are telling
> me that all classes that implement 
> RemoteNotificationClientHandler are not going to implement it 
> directly, but instead are going to extend AbstractRNCH - then 
> it might as well be carried in the 
> RemoteNotificationClientHandler interface.  Or the internal 
> type references should be of type AbstractRNCH.

No, right now the actual implementation class is hardcoded in the connector, but nothing prevents us to write a factory for that, configurable via system properties.
So I prefer to keep the references to the interface instead of some implementation class.
We may sort out a totally different implementation that uses less threads or more threads, but still the semantic of start()/stop() is clear (at least to me) whatever implementation you choose.

> I was confused by the use of the 
> RemoteNotificationClientHandler interface as the type of the 
> references in the RMIConnector and SOAPConnector.  

A best practice I use frequently.

> Each of these has its own implementation of 
> RemoteNotificationClientHandler that they require to operate 
> properly - why not just type the references as such?

Because there is the possibility to make it pluggable. Maybe we'll never use it, but the effort to prepare the code is exactly zero, so I don't mind.
I left TODOs in the places where I have hardcoded pluggable stuff.

> I don't know that burdening the RNCH interface with
> start/stop makes sense.  This certainly needs more
> discussion.

As I've explained, it seems semantic to me, but I'll be glad to hear your interpretation.

BTW, the fetcher thread is a strange beast. From my first experiments, it turn out that you cannot stop an RMI call that is pending on the server side (as it is the fetchNotification() call: it is waiting a timeout on the server).
Calling Thread.interrupt() does not throw an InterruptedIOException, but I have to investigate more.

Cheers

Simon


-------------------------------------------------------
SF.Net is sponsored by: Speed Start Your Linux Apps Now.
Build and deploy apps & Web services for Linux with
a free DVD software kit from IBM. Click Now!
http://ads.osdn.com/?ad_id56&alloc_id438&op=click