Community
Participate
Working Groups
Both providers have several methods in common, thus it makes sense to factor out a common superclass. Cheers Markus
Created attachment 79064 [details] patch implementing this enhancement
Applied patch to HEAD and committed. Please review and comment on this bug.
Created attachment 80137 [details] patch makes members protected and adds missing method
+1 for FIXED
Hi Markus. Thanks for the additional patch. I've applied the member protection changes, but not the addition of getConnectedID, as I would like to leave the implementation of getConnectedID up to subclasses, as they also have to implement connect()/disconnect (and the meaning of getConnectedID() is really dependent upon connect()). Anyway, thanks again for the patch. IP log also updated. Resolving as fixed, as I think this works.
But why not leave getConnectedID { return config.getID(); } as the default impl. It isn't final so subclasses can overwrite.
Because subclasses that implement connect may then forget to implement getConnectedID.
But isn't this always the problem with providing an implementation in the superclass? But can you elaborate on the semantical meaning of getConnectID(), so I better understand why config.getID() might not be sufficient in a couple of scenarios.
(In reply to comment #8) > But isn't this always the problem with providing an implementation in the > superclass? Yes, absolutely...to differing degrees. As an example, it's relatively clear that getID() can/should be implemented by the abstract superclass, because subclasses (typically) don't implement anything that would affect the getID() behavior. OTOH, forcing subclasses to implement IContainer.connect() implies that they can/should? also implement getConnectedID() (since getConnectedID() is dependent upon connect). >But can you elaborate on the semantical meaning of getConnectID(), > so I better understand why config.getID() might not be sufficient in a couple > of scenarios. getConnectedID returns the targetID (or null if not connected at all) of the remote container that the given container is connected to (e.g. after call to connect(ID targetID). Discovery API is somewhat unusual (compared to other container adapters...e.g. presence, etc), in that it doesn't have a clear notion of 'connection' to a particular identified (except to perhaps the/a network itself...e.g. to a particular network interface eth0, eth1, etc).
Created attachment 80854 [details] protected collection/map in superclass should be thread safe
reopened because of previous comment
Added synchronization to relevant places in class rather than creating synchronized collection. Marking as fixed.
Hi Scott, this way subclasses will have to synchronize "simple" access to the collections as well which wouldn't be necessary if the collections would be instantiated as synchronized ones.
(In reply to comment #13) > Hi Scott, > > this way subclasses will have to synchronize "simple" access to the collections > as well which wouldn't be necessary if the collections would be instantiated as > synchronized ones. > Added creation of synchronized map/set and documentation for member variables.
closing.
