ObserverWrapper -- unused fields/methods

[David Blevins <da...@visi.com>]

Looking at the ObserverWrapper it has three unused fields eventType,  
ifExist, and transObserverType.  I had originally concluded that since  
ifExist wasn't used that the Conditional Observer Method logic wasn't  
implemented, but then I found it in BeanObserverImpl.

None of the runtime code ever assigns a value to these fields -- aside  
from the default -- and none of the runtime code uses those fields or  
related getters.

Should these be deleted or is there a reason for keeping them around?

Seems to me that ObserverWrapper is really just an internal detail of  
the NotificationManager and exists more or less as a way to pair the  
Observer with the Event Qualifiers.  If you look at the java.util  
Collections classes, they'll typically use a private inner class for  
these "tuple" classes as they really have no meaning outside the class  
that uses it internally.  I tend to do the same.

Any objections to me making that change as part of the transaction  
related observer fix?


-David

Re: ObserverWrapper -- unused fields/methods

[Gurkan Erdogdu <cg...@gmail.com>]

It seems that no more usage of these fields. If tests are run successfully
after removing that is fine.

I actually wrote all of those event stuffs. Because, specification has
undergone lots of changes between releases, I may not able to reflect all of
those changed things. Actually it will be very neat if we can update our
event implementation according to the latest specification.

+1 for pushing change!

Thanks a lot ;

--Gurkan

2009/10/1 David Blevins <da...@visi.com>

> Looking at the ObserverWrapper it has three unused fields eventType,
> ifExist, and transObserverType.  I had originally concluded that since
> ifExist wasn't used that the Conditional Observer Method logic wasn't
> implemented, but then I found it in BeanObserverImpl.
>
> None of the runtime code ever assigns a value to these fields -- aside from
> the default -- and none of the runtime code uses those fields or related
> getters.
>
> Should these be deleted or is there a reason for keeping them around?
>
> Seems to me that ObserverWrapper is really just an internal detail of the
> NotificationManager and exists more or less as a way to pair the Observer
> with the Event Qualifiers.  If you look at the java.util Collections
> classes, they'll typically use a private inner class for these "tuple"
> classes as they really have no meaning outside the class that uses it
> internally.  I tend to do the same.
>
> Any objections to me making that change as part of the transaction related
> observer fix?
>
>
> -David
>
>


-- 
Gurkan Erdogdu
http://gurkanerdogdu.blogspot.com