commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian McCallister <mccallis...@forthillcompany.com>
Subject Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Date Wed, 05 Nov 2003 14:28:32 GMT

On Wednesday, November 5, 2003, at 12:10 AM, Phil Steitz wrote:
> I think that that javadoc for remove is incorrect when it says:
> "This implementation calls <code>remove()</code> on each collection." 
> It stops when it finds the element to be removed.  The contract needs 
> to be made more explicit here.  It might actually be better to push 
> remove() into the Mutator, since one could argue that the current 
> "remove strategy" (kill the first one found) is no less arbitrary than 
> a default "add" strategy that just adds to the last collection.  Might 
> be better to make this pluggable like add.

To quote the Collection API doc:
<quote>
Removes a single instance of the specified element from this  
collection, if it is present (optional operation).  More formally,  
removes an element e such that (o==null ?  e==null :  o.equals(e)), if 
this collection contains one or more such  elements.
</quote>

I agree that this could be pluggable, but I think that providing a 
"remove first found" as a default is very reasonable in this case as it 
fits the idiomatic behavior people expect from extent collections.

+0 (non-binding) for putting this into the CollectionMutator but 
providing present behavior as default if no mutator set (rather than an 
exception as add/addAll do. This is internally inconsistent though so I 
would welcome better ideas.

>
> The containsAll javadoc says "This is O(n^2) at the moment, be careful 
> using it.".

It is not correct anymore. It was in the original version but the 
implementation has changed significantly already =)

> I am curious about how much faster this can be done without an 
> ordering.

Dropping ordering on what?

> The last comment suggests another possibly useful method:
> toList(), returning an aggregated collection consisting of all of the 
> objects in the composite collections.

That works for me, though I would make it a Collection and return the 
actual type of whichever subclass. I suspect Stephen will suggest that 
it be toCollection(), toList(), and toSet() respectively in order to 
allow greater specificity of the return type, which I am also okay with.

Hmm, would be nice if Java let you override a method to return a 
subclass of the return type of the method being overridden. It would 
satisfy the static typing still.

-Brian

PS: I have attached changes discussed


Mime
View raw message