You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by "Mark R. Diggory" <md...@latte.harvard.edu> on 2003/11/02 18:05:32 UTC
[math] Date Error in Site News
Looks like theres a date error in the news for the math project, it
should be November not October.
http://jakarta.apache.org/site/news.html#20031030.1
--
Mark Diggory
Software Developer
Harvard MIT Data Center
http://www.hmdc.harvard.edu
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Phil Steitz <ph...@steitz.com>.
Stephen Colebourne wrote:
> Sounds like a bug in IteratorChain. hasNext() should never throw UOE. (b) is
> the best solution for your particuular problem however.
> Stephen
I have now committed the code. I made the following changes from
Brian's last post:
1. Modified iterator to return an empty iterator when the
CompositeCollection is empty
2. Modified toArray(array) to comply with the null padding requirement
of the Collection spec (turned up by the test framework :-)
3. Reformatted code to match collections style.
4. Modified TestCompositeCollection to extend AbstractTestCollection,
removing a few redundant tests, modifying to work on (sic) Junit 3.7 and
adding some more mutator tests.
Phil
>
> From: "Phil Steitz" <ph...@steitz.com>
>
>>Stephen Colebourne wrote:
>>
>>>From: "Phil Steitz" <ph...@steitz.com>
>>>
>>>>Anyone have any objections to committing this to the decorators
>>>
>>>subpackage?
>>>
>>>
>>>>Phil
>>>
>>>+1. The test needs work as its not a collections-testframework test.
>>
>>I have reworked the test class to extend AbstractTestCollection and have
>>run into a problem. The CompositeCollection interator method returns an
>>IteratorChain.In the test class, I implemented makeCollection to return
>>an empty CompositeCollection. The verifies are failing because
>>IteratorChain.hasNext() throws an UnsupportedOperationException when the
>>chain is empty.
>>
>>So the question is, what (if anything?) should change:
>>a) IteratorChain.hasNext() (return false when the chain is empty)
>>b) CompositeCollection.iterator() (return an emtpy non-chained iterator
>>when the CompsiteCollection is empty)
>>c) No change
>>
>>Thoughts?
>>
>>Phil
>>
>>
>>>Stephen
>>>
>>>
>>>
>>>---------------------------------------------------------------------
>>>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
>>
>
>
>
> ---------------------------------------------------------------------
> 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
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Stephen Colebourne <sc...@btopenworld.com>.
Sounds like a bug in IteratorChain. hasNext() should never throw UOE. (b) is
the best solution for your particuular problem however.
Stephen
From: "Phil Steitz" <ph...@steitz.com>
> Stephen Colebourne wrote:
> > From: "Phil Steitz" <ph...@steitz.com>
> >
> >>Anyone have any objections to committing this to the decorators
> >
> > subpackage?
> >
> >>Phil
> >
> > +1. The test needs work as its not a collections-testframework test.
>
> I have reworked the test class to extend AbstractTestCollection and have
> run into a problem. The CompositeCollection interator method returns an
> IteratorChain.In the test class, I implemented makeCollection to return
> an empty CompositeCollection. The verifies are failing because
> IteratorChain.hasNext() throws an UnsupportedOperationException when the
> chain is empty.
>
> So the question is, what (if anything?) should change:
> a) IteratorChain.hasNext() (return false when the chain is empty)
> b) CompositeCollection.iterator() (return an emtpy non-chained iterator
> when the CompsiteCollection is empty)
> c) No change
>
> Thoughts?
>
> Phil
>
> >
> > Stephen
> >
> >
> >
> > ---------------------------------------------------------------------
> > 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
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Phil Steitz <ph...@steitz.com>.
Stephen Colebourne wrote:
> From: "Phil Steitz" <ph...@steitz.com>
>
>>Anyone have any objections to committing this to the decorators
>
> subpackage?
>
>>Phil
>
> +1. The test needs work as its not a collections-testframework test.
I have reworked the test class to extend AbstractTestCollection and have
run into a problem. The CompositeCollection interator method returns an
IteratorChain.In the test class, I implemented makeCollection to return
an empty CompositeCollection. The verifies are failing because
IteratorChain.hasNext() throws an UnsupportedOperationException when the
chain is empty.
So the question is, what (if anything?) should change:
a) IteratorChain.hasNext() (return false when the chain is empty)
b) CompositeCollection.iterator() (return an emtpy non-chained iterator
when the CompsiteCollection is empty)
c) No change
Thoughts?
Phil
>
> Stephen
>
>
>
> ---------------------------------------------------------------------
> 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
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Stephen Colebourne <sc...@btopenworld.com>.
From: "Phil Steitz" <ph...@steitz.com>
> Anyone have any objections to committing this to the decorators
subpackage?
>
> Phil
+1. The test needs work as its not a collections-testframework test.
Stephen
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Brian McCallister <mc...@forthillcompany.com>.
On Wednesday, November 5, 2003, at 11:31 AM, Phil Steitz wrote:
> 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.
>
Point made.
> 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.
Thinking about it. Without
> 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.
>
CompositeCollection returns a Collection
CompositeList returns a List (it requires summands (good word) to be
List)
CompositeSet returns a Set (it requires summands to be Set)
> Anyone have any objections to committing this to the decorators
> subpackage?
>
If so, I have attached a version which puts remove() into
CollectionMutator
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Phil Steitz <ph...@steitz.com>.
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
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Brian McCallister <mc...@forthillcompany.com>.
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
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Phil Steitz <ph...@steitz.com>.
Stephen Colebourne wrote:
> I've updated the class in line with commons standards/documentation etc. It
> will probably end up in the decorators subpackage, as it decorates other
> collections.
>
> I've also changed the way the Mutator works - see what you think, it seems
> cleaner/quicker I think.
>
> The test case still needs looking at as it doesn't extend the collections
> testframework.
>
> Any other [collections] committers care to comment on the
> idea/implementation????
>
> Stephen
>
Looks good and useful to me. Here are some specific comments:
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.
The containsAll javadoc says "This is O(n^2) at the moment, be careful
using it.". I am not sure that this is correct; but in any case it does
have to linearly search the composite collection for each element in the
collection passed in. I am curious about how much faster this can be
done without an ordering. Does anyone know how the JDK containsAll does
it for, say, ArrayLists?
The last comment suggests another possibly useful method:
toList(), returning an aggregated collection consisting of all of the
objects in the composite collections.
Phil
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Brian McCallister <br...@apache.org>.
On Monday, November 3, 2003, at 10:47 PM, __matthewHawthorne wrote:
> I took a quick glance, and I think it looks good. I can't really
> imagine a use case for this class, but I'm sure somebody else can.
The exact one I created it for was where I have a class with a set of
things, a parent with a set of things, its parent with a set of things
etc. I needed to access them all as a single set from the lowest leaf
as if all the things were in the set on that leaf -- and for
performance reasons needed to support lazy loading of the collections
(this is all being pulled from a database, using a materialization
process that allows collection proxying until it is actually used then
loading the contents), access/remove them via iteration, etc. Calling
Foo.removeThing(thing) to modify a typical unmodifiable collection
would require searching for said thing across all and sundry -- ick.
-Brian
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by __matthewHawthorne <ma...@phreaker.net>.
I took a quick glance, and I think it looks good. I can't really
imagine a use case for this class, but I'm sure somebody else can.
Stephen Colebourne wrote:
> I've updated the class in line with commons standards/documentation etc. It
> will probably end up in the decorators subpackage, as it decorates other
> collections.
>
> I've also changed the way the Mutator works - see what you think, it seems
> cleaner/quicker I think.
>
> The test case still needs looking at as it doesn't extend the collections
> testframework.
>
> Any other [collections] committers care to comment on the
> idea/implementation????
>
> Stephen
>
> ----- Original Message -----
> From: "Brian McCallister" <mc...@forthillcompany.com>
>
>>On Sunday, November 2, 2003, at 07:27 PM, Stephen Colebourne wrote:
>>
>>>I haven't tested it, but I suspect the performance gain to be
>>>noticable, and
>>>[collections] has to choose the fastest implementation if it has a
>>>choice.
>>>Would you consider the alternative implementation I suggest?
>>>
>>>Stephen
>>
>>Performance optimized version attached. I think this actually reads
>>more clearly anyway. Must try to stop thinking in Ruby when writing
>>Java =) I also cleaned up the spec breaking toArray(Object[] array)
>>implementation.
>>
>>The highly unoptimized part is adding and removing composited
>>collections. I let ArrayList handle the array resizing for me.
>>
>>-Brian
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Gareth Andrew <ga...@ntlworld.com>.
Stephen,
Have you any tests to prove there is actually a performance gain?
Surely either (or both) the java compiler or the JVM should be
responsible for optimizations such as this.
Gareth.
Stephen Colebourne wrote:
>From: "Brian McCallister" <mc...@forthillcompany.com>
>On Monday, November 3, 2003, at 07:48 PM, Stephen Colebourne wrote:
>
>
>>>I've updated the class in line with commons standards/documentation
>>>etc. It
>>>will probably end up in the decorators subpackage, as it decorates
>>>other
>>>collections.
>>>
>>>
>>A few style questions about this...
>>
>>You have changed the array traversal to use the form:
>>
>> public int size() {
>> int size = 0;
>> for (int i = this.all.length - 1; i >= 0 ; i--) {
>> size += this.all[i].size();
>> }
>> return size;
>> }
>>
>>
>
>
>
>>Do you really see a performance gain from not dereferencing
>>this.all.length on each cycle? If so, cool stuff!
>>
>>
>
>Yes, you gain from not having to check the variable all and its length. You
>also gain from the comparision to 0 (i>=0) which is quicker than a non-zero
>comparison (i < n). Some methods required to keep the order, so I have been
>selective.
>
>Stephen
>
>
>
>
>---------------------------------------------------------------------
>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
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Stephen Colebourne <sc...@btopenworld.com>.
From: "Brian McCallister" <mc...@forthillcompany.com>
On Monday, November 3, 2003, at 07:48 PM, Stephen Colebourne wrote:
> > I've updated the class in line with commons standards/documentation
> > etc. It
> > will probably end up in the decorators subpackage, as it decorates
> > other
> > collections.
>
> A few style questions about this...
>
> You have changed the array traversal to use the form:
>
> public int size() {
> int size = 0;
> for (int i = this.all.length - 1; i >= 0 ; i--) {
> size += this.all[i].size();
> }
> return size;
> }
> Do you really see a performance gain from not dereferencing
> this.all.length on each cycle? If so, cool stuff!
Yes, you gain from not having to check the variable all and its length. You
also gain from the comparision to 0 (i>=0) which is quicker than a non-zero
comparison (i < n). Some methods required to keep the order, so I have been
selective.
Stephen
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Brian McCallister <mc...@forthillcompany.com>.
On Monday, November 3, 2003, at 07:48 PM, Stephen Colebourne wrote:
> I've updated the class in line with commons standards/documentation
> etc. It
> will probably end up in the decorators subpackage, as it decorates
> other
> collections.
>
A few style questions about this...
You have changed the array traversal to use the form:
public int size() {
int size = 0;
for (int i = this.all.length - 1; i >= 0 ; i--) {
size += this.all[i].size();
}
return size;
}
Do you really see a performance gain from not dereferencing
this.all.length on each cycle? If so, cool stuff!
-Brian
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Brian McCallister <mc...@forthillcompany.com>.
On Tuesday, November 4, 2003, at 04:00 PM, Stephen Colebourne wrote:
> The main aim of the Mutator change was to make the implementation
> obvious
> and typesafe. IMO it does fit better with the collections API.
Okay, I don't feel strongly about it, and if it stays as an inner class
it will work fine. I will propose changing the name to
CollectionMutator though in order to allow ListMutator and SetMutator
in the subclasses.
-Brian
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Stephen Colebourne <sc...@btopenworld.com>.
----- Original Message -----
From: "Brian McCallister" <br...@apache.org>
> On Monday, November 3, 2003, at 07:48 PM, Stephen Colebourne wrote:
> > I've also changed the way the Mutator works - see what you think, it
> > seems
> > cleaner/quicker I think.
>
> Also a lot less flexible. Mutator is also used in the CompositeSet and
> CompositeList (as yet unposted as they are broken by the changes to
> Collection and I haven't had a chance to update them(and they are not
> yet complete implementations -- only the methods I have needed)) and
> will be in CompositeMap (am looking forward to figuring a nice way to
> do conflict resolution on the Map impl for duplicate keys -- default
> will probably be to throw an exception when a get() is made on a dup
> but will be able to plug it so you can handle however you want)).
>
> I prefer the more fine-grained control of setting them per method. Your
> change it a wee bit quicker to type (probably only four lines for the
> implementor) but significantly reduces the reusability of the Mutator
> interface.
The main aim of the Mutator change was to make the implementation obvious
and typesafe. IMO it does fit better with the collections API. The
reusability of the interface is actually very unclear, as each method will
expect different input parameters and return different result. Under the
original design there was no way to determine which method on the Collection
had acually been called.
I can understand how this might affect the List and Set subclasses, but I
would suggest adapting them to have their own interfaces too.
Stephen
> > The test case still needs looking at as it doesn't extend the
> > collections
> > testframework.
> >
>
> No problem -- It is set up as just a plain JUnit test.
>
> Your thoughts, time, and commentary is much appreciated!
>
> -Brian
>
>
> ---------------------------------------------------------------------
> 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
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Brian McCallister <br...@apache.org>.
On Monday, November 3, 2003, at 07:48 PM, Stephen Colebourne wrote:
> I've updated the class in line with commons standards/documentation
> etc.
Much appreciated!
> I've also changed the way the Mutator works - see what you think, it
> seems
> cleaner/quicker I think.
Also a lot less flexible. Mutator is also used in the CompositeSet and
CompositeList (as yet unposted as they are broken by the changes to
Collection and I haven't had a chance to update them(and they are not
yet complete implementations -- only the methods I have needed)) and
will be in CompositeMap (am looking forward to figuring a nice way to
do conflict resolution on the Map impl for duplicate keys -- default
will probably be to throw an exception when a get() is made on a dup
but will be able to plug it so you can handle however you want)).
I prefer the more fine-grained control of setting them per method. Your
change it a wee bit quicker to type (probably only four lines for the
implementor) but significantly reduces the reusability of the Mutator
interface.
> The test case still needs looking at as it doesn't extend the
> collections
> testframework.
>
No problem -- It is set up as just a plain JUnit test.
Your thoughts, time, and commentary is much appreciated!
-Brian
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Stephen Colebourne <sc...@btopenworld.com>.
I've updated the class in line with commons standards/documentation etc. It
will probably end up in the decorators subpackage, as it decorates other
collections.
I've also changed the way the Mutator works - see what you think, it seems
cleaner/quicker I think.
The test case still needs looking at as it doesn't extend the collections
testframework.
Any other [collections] committers care to comment on the
idea/implementation????
Stephen
----- Original Message -----
From: "Brian McCallister" <mc...@forthillcompany.com>
> On Sunday, November 2, 2003, at 07:27 PM, Stephen Colebourne wrote:
> > I haven't tested it, but I suspect the performance gain to be
> > noticable, and
> > [collections] has to choose the fastest implementation if it has a
> > choice.
> > Would you consider the alternative implementation I suggest?
> >
> > Stephen
>
> Performance optimized version attached. I think this actually reads
> more clearly anyway. Must try to stop thinking in Ruby when writing
> Java =) I also cleaned up the spec breaking toArray(Object[] array)
> implementation.
>
> The highly unoptimized part is adding and removing composited
> collections. I let ArrayList handle the array resizing for me.
>
> -Brian
>
>
>
----------------------------------------------------------------------------
----
>
>
>
>
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Brian McCallister <mc...@forthillcompany.com>.
On Sunday, November 2, 2003, at 07:27 PM, Stephen Colebourne wrote:
> I haven't tested it, but I suspect the performance gain to be
> noticable, and
> [collections] has to choose the fastest implementation if it has a
> choice.
> Would you consider the alternative implementation I suggest?
>
> Stephen
Performance optimized version attached. I think this actually reads
more clearly anyway. Must try to stop thinking in Ruby when writing
Java =) I also cleaned up the spec breaking toArray(Object[] array)
implementation.
The highly unoptimized part is adding and removing composited
collections. I let ArrayList handle the array resizing for me.
-Brian
Re: [collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Stephen Colebourne <sc...@btopenworld.com>.
These strike me as a Good Idea with merit to be added to [collections]. I am
intrigued by your implementation however. The use of Closure requires two
new objects (the inner class Closure and the Holder) for each of the key
methods including size(). Perhaps you come from a Smalltalk background?
Whatever, I fear for performance.
I would have chosen to
- store the list of contained collections in an array Collection[] (faster
access as the list should be changed relatively infrequently relative to
using the combined list)
- loop around the array calling size() or isEmpty() or whatever directly.
For isEmpty() this can increase performance, as if the first collection
returns false you can break the loop.
I haven't tested it, but I suspect the performance gain to be noticable, and
[collections] has to choose the fastest implementation if it has a choice.
Would you consider the alternative implementation I suggest?
Stephen
----- Original Message -----
From: "Brian McCallister" <br...@apache.org>
> Attached is a CompositeCollection class which allows a single
> Collection instance to behave correctly as a composite for N
> collections, and unit tests for that class.
>
> All operations (including iterator removal operations) which are
> deterministic per the Collection spec are implemented directly.
> Nondeterministic operations (add, addAll) are pluggable and throw an
> UnsupportedOperationException if no behavior has been specified.
>
> Implementations of Set and List (which are dependent upon
> CompositeCollection) are forthcoming as soon as I have a chance.
>
> -Brian
>
>
>
----------------------------------------------------------------------------
----
>
>
>
---------------------------------------------------------------------
To unsubscribe, e-mail: commons-dev-unsubscribe@jakarta.apache.org
For additional commands, e-mail: commons-dev-help@jakarta.apache.org
[collections] [PATCH] CompositeCollection class for Commons-Collections
Posted by Brian McCallister <br...@apache.org>.
Attached is a CompositeCollection class which allows a single
Collection instance to behave correctly as a composite for N
collections, and unit tests for that class.
All operations (including iterator removal operations) which are
deterministic per the Collection spec are implemented directly.
Nondeterministic operations (add, addAll) are pluggable and throw an
UnsupportedOperationException if no behavior has been specified.
Implementations of Set and List (which are dependent upon
CompositeCollection) are forthcoming as soon as I have a chance.
-Brian