commons-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Arun Thomas" <arun.tho...@paybytouch.com>
Subject RE: [collections] [PATCH] CompositeCollection class for Commons-Collections
Date Wed, 05 Nov 2003 17:07:39 GMT
>> The last comment suggests another possibly useful method: toList(), 
>> returning an aggregated collection consisting of all of the objects 
>> in the composite collections.

In this case, will there be a clever way to return an aggregation of this sort that isn't
simply a new object with copies of all elements of the contained objects?  If that's the case,
then is seems more reasonable to leave the creation of this type of object to the user - create
a new object passing in the composite collection in the constructor, or use addAll(Collection
c).   

If there's a clever way to handle provide the implementation of List/Set/Collection using
the contained collections as a base, that might be interesting, but otherwise....  

Cheers, 
-AMT 

-----Original Message-----
From: Phil Steitz [mailto:phil@steitz.com] 
Sent: Wednesday, November 05, 2003 8:31 AM
To: Jakarta Commons Developers List
Subject: Re: [collections] [PATCH] CompositeCollection class for Commons-Collections


Brian McCallister wrote:
> 
> 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.

Note the similarity to the API doc for add:
"Ensures that this collection contains the specified element (optional 
operation). Returns true if this collection changed as a result of the 
call. (Returns false if this collection does not permit duplicates and 
already contains the specified element.)"

My point is that "kill first" in a composite collection is no more 
"natural" than "add last".  I would be OK with both being defaulted but 
modifiable via Mutator.  Since "the collection" could mean either the 
aggregate or *each* of the summands in each case, I see both add and 
remove as ambiguous (hence the need for strategies).  This is a small point.

> 
> +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?

What I meant was that without assuming an ordering on the aggregate (so 
binary search would be possible), is there a faster way to do the "*all" 
methods. I assume that if there is a clever way to do this, that is what 
the JDK methods do.

> 
>> 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.

What do you mean by "whichever subclass"?  The aggregated collections 
could be of multiple different types. That makes an interesting problem. 
  I suppose that toCollection() could return an instance of the one 
common type if the summands are "homogeneous" (all the same type), 
otherwise default to a (Array?)List or (Hash?)Set.  I agree that toSet() 
would also be natural. Need to think about these things some more.  It 
might be better to just have the API take the aggregation target as an 
actual parameter -- i.e. toCollection(collection), effectively punting 
the issue of return types.

Anyone have any objections to committing this to the decorators subpackage?

Phil

> 
> 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
> 
> 



---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org


Mime
View raw message