You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@commons.apache.org by Morgan Delagrange <md...@yahoo.com> on 2002/03/18 23:27:57 UTC

Re: [COLLECTIONS] [VOTE] Release Collections 2.0

----- Original Message -----
From: "Michael A. Smith" <ma...@apache.org>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>;
"Morgan Delagrange" <mo...@apache.org>
Sent: Sunday, March 17, 2002 11:00 PM
Subject: Re: [COLLECTIONS] [VOTE] Release Collections 2.0


> On Fri, 15 Mar 2002, Morgan Delagrange wrote:
> > The release candidate is ready and sitting on the server:
> >
> >
http://jakarta.apache.org/builds/jakarta-commons/release/commons-collections
> > /v2.0/
> >
> > Please state your vote for this candidate.  Also, please let me know if
you
> > discover errors in the distribution.
>
> -0 on the release.  I don't want to hold it up, but I feel there are still
> some issues that could be resolved before we actually release a 2.0.
>
> As I mentioned earlier, documentation definately needs to be improved, but
> that could be added in a 2.0.1 without too much problems.  Another thing
> that I think could be improved is argument checking.
>
> For example, ArrayEnumeration takes an Object as a parameter, but does not
> provide checking on that to ensure that it is actually an Array.  The
> checking would eventually be provided when the hasNext or next methods are
> called (in the form of an IllegalArgumentException), but I feel this
> should really be thrown from the constructor and and the setArray methods.
> That is, a more fail-fast behavior.

Dang nabbit, now you've made me think.

ArrayEnumeration actually takes Object[] as its parameter, I think you're
thinking of ArrayIterator.  But you are right, it would be nice if
ArrayIterator was fail-fast.  Unfortunately I can't think of a way to do it
that's not really hokey.  ArrayIterator can take an array of primitives, and
there is no specific operation I know of to determine if an Object is an
array of primatives. :P

ArrayIterator takes primitives and ArrayEnumeration doesn't.  :P  Should we
fix ArrayEnumeration?  Or maybe we should deprecate the class and encourage
users to wrap ArrayIterator with IteratorEnumeration instead.  I think the
latter would be my vote.  We shouldn't really need to have any Enumeration
classes around, since they can always be wrapped.

>
> Changing such argument checking may be construed as an incompatible
> change, so that may not be able to be fixed in a 2.0.1 or a 2.1.
>
> Another thing on I'd like to see is full generalization of some of the
> collections.  For example, the BinaryHeap is currently restricted to
> Comparable objects.  While you can't really have a heap without the
> ability to compare the objects, that doesn't mean the objects themselves
> have to be comparable.  It should be allowable to specify a Comparator and
> use that to compare objects that don't implement Comparable.  Such a
> change would alter the signatures of a couple of methods and would be an
> non-backwards compatible change.

That only sounds like more constructors, right?  You could probably do that
in a minor release, especially considering BinaryHeap is neither
Serializable nor subclass-able.

>
> regards,
> michael


_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [COLLECTIONS] [VOTE] Release Collections 2.0

Posted by James Strachan <ja...@yahoo.co.uk>.
----- Original Message -----
From: "Michael A. Smith" <ma...@apache.org>
> On Mon, 18 Mar 2002, Morgan Delagrange wrote:
> > > For example, ArrayEnumeration takes an Object as a parameter, but does
not
> > > provide checking on that to ensure that it is actually an Array.  The
> > > checking would eventually be provided when the hasNext or next methods
are
> > > called (in the form of an IllegalArgumentException), but I feel this
> > > should really be thrown from the constructor and and the setArray
methods.
> > > That is, a more fail-fast behavior.
> >
> > Dang nabbit, now you've made me think.
> >
> > ArrayEnumeration actually takes Object[] as its parameter, I think
you're
> > thinking of ArrayIterator.  But you are right, it would be nice if
> > ArrayIterator was fail-fast.  Unfortunately I can't think of a way to do
it
> > that's not really hokey.  ArrayIterator can take an array of primitives,
and
> > there is no specific operation I know of to determine if an Object is an
> > array of primatives. :P
>
> yeah, I think I did mean ArrayIterator.  The simple way of doing this
> would be to use Array.getLength(array).  That does the checking for you.
> It also lets you calculate the length once (in the constructor) so that
> multiple calls to hasNext does not require the overhead of
> Array.getLength(...).

Good idea. I was thinking of object.getClass().isArray() but you're idea is
better.

> > ArrayIterator takes primitives and ArrayEnumeration doesn't.  :P  Should
we
> > fix ArrayEnumeration?  Or maybe we should deprecate the class and
encourage
> > users to wrap ArrayIterator with IteratorEnumeration instead.  I think
the
> > latter would be my vote.  We shouldn't really need to have any
Enumeration
> > classes around, since they can always be wrapped.
>
> There's probably an argument for keeping it around, but it's not an
> argument that I'm going to make.  I'm with you on deprecating
> ArrayEnumeration.

+1.

James


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [COLLECTIONS] [VOTE] Release Collections 2.0

Posted by Morgan Delagrange <md...@yahoo.com>.
----- Original Message -----
From: "James Strachan" <ja...@yahoo.co.uk>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>
Sent: Monday, March 18, 2002 5:45 PM
Subject: Re: [COLLECTIONS] [VOTE] Release Collections 2.0


> ----- Original Message -----
> From: "Morgan Delagrange" <md...@yahoo.com>
> > > yeah, I think I did mean ArrayIterator.  The simple way of doing this
> > > would be to use Array.getLength(array).  That does the checking for
you.
> > > It also lets you calculate the length once (in the constructor) so
that
> > > multiple calls to hasNext does not require the overhead of
> > > Array.getLength(...).
> >
> > OK, sounds reasonable.  FWIW, it may make sense to optimize this class
> > someday, so we don't perform unnecessary reflection on Object[] arrays.
>
> Though I didn't think you could cast all arrays to Object[] so I'd prefer
to
> keep the reflection based implementation - I've certainly written code
> before that needs it. e.g.
>
> int[] foo = {1,2,3};
> Object[] bar = (Object[]) foo;
>
> The above is invalid.
>
> James
>

I was thinking more along the lines of:

  if (array instanceof Object[]) {
      // don't use reflections
  } else {
      // use reflection
  }

Reflection is cheaper then it used to be, but it's still a bit pricey.

- Morgan


_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [COLLECTIONS] [VOTE] Release Collections 2.0

Posted by Morgan Delagrange <md...@yahoo.com>.
----- Original Message -----
From: "James Strachan" <ja...@yahoo.co.uk>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>
Sent: Wednesday, March 20, 2002 12:56 AM
Subject: Re: [COLLECTIONS] [VOTE] Release Collections 2.0


> [snip]
>
> I just thought Morgan was thinking of removing the reflection code and
just
> using Object[] instead and just wanted to explain why the reflection was
> needed - though I think I got the wrong end of the stick and Morgan was
> really just talking about tuning the Object[] use case while still
> supporting reflection for non-Object[] types.
>
> Sorry for the confusion everyone...
>

No problem.  I already patched ArrayIterator.next(), but I decided not to
apply it:

        // don't use reflection for Object arrays
        if (array instanceof Object[]) {
            return ((Object[]) array)[index++];
        }

        // use reflection for primitive arrays
        return Array.get( array, index++ );

It works fine, but it seems like premature optimization now.  I can't tell
for sure (Array.get(Object,int) is a native method), but surely the
Collections method performs the same optimization internally.  Anything else
seems silly.

- Morgan



_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [COLLECTIONS] [VOTE] Release Collections 2.0

Posted by James Strachan <ja...@yahoo.co.uk>.
----- Original Message -----
From: "Craig R. McClanahan" <cr...@apache.org>
>
> On Mon, 18 Mar 2002, James Strachan wrote:
>
> > Date: Mon, 18 Mar 2002 23:45:21 -0000
> > From: James Strachan <ja...@yahoo.co.uk>
> > Reply-To: Jakarta Commons Developers List
<co...@jakarta.apache.org>
> > To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> > Subject: Re: [COLLECTIONS] [VOTE] Release Collections 2.0
> >
> > ----- Original Message -----
> > From: "Morgan Delagrange" <md...@yahoo.com>
> > > > yeah, I think I did mean ArrayIterator.  The simple way of doing
this
> > > > would be to use Array.getLength(array).  That does the checking for
you.
> > > > It also lets you calculate the length once (in the constructor) so
that
> > > > multiple calls to hasNext does not require the overhead of
> > > > Array.getLength(...).
> > >
> > > OK, sounds reasonable.  FWIW, it may make sense to optimize this class
> > > someday, so we don't perform unnecessary reflection on Object[]
arrays.
> >
> > Though I didn't think you could cast all arrays to Object[] so I'd
prefer to
> > keep the reflection based implementation - I've certainly written code
> > before that needs it. e.g.
> >
> > int[] foo = {1,2,3};
> > Object[] bar = (Object[]) foo;
> >
> > The above is invalid.
> >
>
> True, but you can say
>
>   Object bar = foo;
>
> and then use reflection underneath to figure out that bar is really an
> array (bar.getClass().isArray()) and what its component type is
> (bar.getClass().getComponentType()).  Also, you can use
> java.lang.reflect.Array to extract the elements for you -- for arrays of
> primitives you get the corresponding wrapper objects, but the concept of
> iterating over it still works.

I know - I wrote the first ArrayIterator that way ;-)

I just thought Morgan was thinking of removing the reflection code and just
using Object[] instead and just wanted to explain why the reflection was
needed - though I think I got the wrong end of the stick and Morgan was
really just talking about tuning the Object[] use case while still
supporting reflection for non-Object[] types.

Sorry for the confusion everyone...

James


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [COLLECTIONS] [VOTE] Release Collections 2.0

Posted by "Craig R. McClanahan" <cr...@apache.org>.

On Mon, 18 Mar 2002, James Strachan wrote:

> Date: Mon, 18 Mar 2002 23:45:21 -0000
> From: James Strachan <ja...@yahoo.co.uk>
> Reply-To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> To: Jakarta Commons Developers List <co...@jakarta.apache.org>
> Subject: Re: [COLLECTIONS] [VOTE] Release Collections 2.0
>
> ----- Original Message -----
> From: "Morgan Delagrange" <md...@yahoo.com>
> > > yeah, I think I did mean ArrayIterator.  The simple way of doing this
> > > would be to use Array.getLength(array).  That does the checking for you.
> > > It also lets you calculate the length once (in the constructor) so that
> > > multiple calls to hasNext does not require the overhead of
> > > Array.getLength(...).
> >
> > OK, sounds reasonable.  FWIW, it may make sense to optimize this class
> > someday, so we don't perform unnecessary reflection on Object[] arrays.
>
> Though I didn't think you could cast all arrays to Object[] so I'd prefer to
> keep the reflection based implementation - I've certainly written code
> before that needs it. e.g.
>
> int[] foo = {1,2,3};
> Object[] bar = (Object[]) foo;
>
> The above is invalid.
>

True, but you can say

  Object bar = foo;

and then use reflection underneath to figure out that bar is really an
array (bar.getClass().isArray()) and what its component type is
(bar.getClass().getComponentType()).  Also, you can use
java.lang.reflect.Array to extract the elements for you -- for arrays of
primitives you get the corresponding wrapper objects, but the concept of
iterating over it still works.

> James
>

Craig


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [COLLECTIONS] [VOTE] Release Collections 2.0

Posted by "Michael A. Smith" <ma...@apache.org>.
On Mon, 18 Mar 2002, James Strachan wrote:
> ----- Original Message -----
> From: "Morgan Delagrange" <md...@yahoo.com>
> > > yeah, I think I did mean ArrayIterator.  The simple way of doing this
> > > would be to use Array.getLength(array).  That does the checking for you.
> > > It also lets you calculate the length once (in the constructor) so that
> > > multiple calls to hasNext does not require the overhead of
> > > Array.getLength(...).
> >
> > OK, sounds reasonable.  FWIW, it may make sense to optimize this class
> > someday, so we don't perform unnecessary reflection on Object[] arrays.
> 
> Though I didn't think you could cast all arrays to Object[] so I'd prefer to
> keep the reflection based implementation - I've certainly written code
> before that needs it. e.g.
> 
> int[] foo = {1,2,3};
> Object[] bar = (Object[]) foo;
> 
> The above is invalid.

I think what Morgan was referring to was possibly optimizing when this 
*is* possible.  E.g. have a second constructor that takes an Object[].  
That way, if the object past in *is* castable as Object[], then a 
non-reflection based impl could be used.  I'm not sure how the two impls 
could be mixed though, so I'm not sure how much of an optimization it 
really is.  

I think we both recognize that taking an int[] should be allowed.

regards,
michael


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [COLLECTIONS] [VOTE] Release Collections 2.0

Posted by James Strachan <ja...@yahoo.co.uk>.
----- Original Message -----
From: "Morgan Delagrange" <md...@yahoo.com>
> > yeah, I think I did mean ArrayIterator.  The simple way of doing this
> > would be to use Array.getLength(array).  That does the checking for you.
> > It also lets you calculate the length once (in the constructor) so that
> > multiple calls to hasNext does not require the overhead of
> > Array.getLength(...).
>
> OK, sounds reasonable.  FWIW, it may make sense to optimize this class
> someday, so we don't perform unnecessary reflection on Object[] arrays.

Though I didn't think you could cast all arrays to Object[] so I'd prefer to
keep the reflection based implementation - I've certainly written code
before that needs it. e.g.

int[] foo = {1,2,3};
Object[] bar = (Object[]) foo;

The above is invalid.

James


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [COLLECTIONS] [VOTE] Release Collections 2.0

Posted by Morgan Delagrange <md...@yahoo.com>.
----- Original Message -----
From: "Michael A. Smith" <ma...@apache.org>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>;
"Morgan Delagrange" <mo...@apache.org>
Sent: Monday, March 18, 2002 5:09 PM
Subject: Re: [COLLECTIONS] [VOTE] Release Collections 2.0


> On Mon, 18 Mar 2002, Morgan Delagrange wrote:
> > > For example, ArrayEnumeration takes an Object as a parameter, but does
not
> > > provide checking on that to ensure that it is actually an Array.  The
> > > checking would eventually be provided when the hasNext or next methods
are
> > > called (in the form of an IllegalArgumentException), but I feel this
> > > should really be thrown from the constructor and and the setArray
methods.
> > > That is, a more fail-fast behavior.
> >
> > Dang nabbit, now you've made me think.
> >
> > ArrayEnumeration actually takes Object[] as its parameter, I think
you're
> > thinking of ArrayIterator.  But you are right, it would be nice if
> > ArrayIterator was fail-fast.  Unfortunately I can't think of a way to do
it
> > that's not really hokey.  ArrayIterator can take an array of primitives,
and
> > there is no specific operation I know of to determine if an Object is an
> > array of primatives. :P
>
> yeah, I think I did mean ArrayIterator.  The simple way of doing this
> would be to use Array.getLength(array).  That does the checking for you.
> It also lets you calculate the length once (in the constructor) so that
> multiple calls to hasNext does not require the overhead of
> Array.getLength(...).

OK, sounds reasonable.  FWIW, it may make sense to optimize this class
someday, so we don't perform unnecessary reflection on Object[] arrays.

> > ArrayIterator takes primitives and ArrayEnumeration doesn't.  :P  Should
we
> > fix ArrayEnumeration?  Or maybe we should deprecate the class and
encourage
> > users to wrap ArrayIterator with IteratorEnumeration instead.  I think
the
> > latter would be my vote.  We shouldn't really need to have any
Enumeration
> > classes around, since they can always be wrapped.
>
> There's probably an argument for keeping it around, but it's not an
> argument that I'm going to make.  I'm with you on deprecating
> ArrayEnumeration.

Cool.  That's what I'm going to do, then, unless there are complaints.

> > > Another thing on I'd like to see is full generalization of some of the
> > > collections.  For example, the BinaryHeap is currently restricted to
> > > Comparable objects.  While you can't really have a heap without the
> > > ability to compare the objects, that doesn't mean the objects
themselves
> > > have to be comparable.  It should be allowable to specify a Comparator
and
> > > use that to compare objects that don't implement Comparable.  Such a
> > > change would alter the signatures of a couple of methods and would be
an
> > > non-backwards compatible change.
> >
> > That only sounds like more constructors, right?  You could probably do
that
> > in a minor release, especially considering BinaryHeap is neither
> > Serializable nor subclass-able.
>
> Nope.  peek() and pop() return Comparables and some of the protected
> methods take a Comparable as an argument.  Changing these from Comparable
> to Object would be an API change because the method signatures would
> change.
>
> If you can hold off on the release for a few days, I can fix the above
> mentioned problems.  Should be fairly simple to do.
>

I'll take care of the ArrayIterator and ArrayEnumeration changes right now.
If you want to tackle BinaryHeap, it's all yours.  Just let us know if it's
going to take more than a few days.

- Morgan


_________________________________________________________
Do You Yahoo!?
Get your free @yahoo.com address at http://mail.yahoo.com


--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>


Re: [COLLECTIONS] [VOTE] Release Collections 2.0

Posted by "Michael A. Smith" <ma...@apache.org>.
On Mon, 18 Mar 2002, Morgan Delagrange wrote:
> > For example, ArrayEnumeration takes an Object as a parameter, but does not
> > provide checking on that to ensure that it is actually an Array.  The
> > checking would eventually be provided when the hasNext or next methods are
> > called (in the form of an IllegalArgumentException), but I feel this
> > should really be thrown from the constructor and and the setArray methods.
> > That is, a more fail-fast behavior.
> 
> Dang nabbit, now you've made me think.
> 
> ArrayEnumeration actually takes Object[] as its parameter, I think you're
> thinking of ArrayIterator.  But you are right, it would be nice if
> ArrayIterator was fail-fast.  Unfortunately I can't think of a way to do it
> that's not really hokey.  ArrayIterator can take an array of primitives, and
> there is no specific operation I know of to determine if an Object is an
> array of primatives. :P

yeah, I think I did mean ArrayIterator.  The simple way of doing this
would be to use Array.getLength(array).  That does the checking for you.  
It also lets you calculate the length once (in the constructor) so that
multiple calls to hasNext does not require the overhead of
Array.getLength(...).

> ArrayIterator takes primitives and ArrayEnumeration doesn't.  :P  Should we
> fix ArrayEnumeration?  Or maybe we should deprecate the class and encourage
> users to wrap ArrayIterator with IteratorEnumeration instead.  I think the
> latter would be my vote.  We shouldn't really need to have any Enumeration
> classes around, since they can always be wrapped.

There's probably an argument for keeping it around, but it's not an 
argument that I'm going to make.  I'm with you on deprecating 
ArrayEnumeration.  

> > Another thing on I'd like to see is full generalization of some of the
> > collections.  For example, the BinaryHeap is currently restricted to
> > Comparable objects.  While you can't really have a heap without the
> > ability to compare the objects, that doesn't mean the objects themselves
> > have to be comparable.  It should be allowable to specify a Comparator and
> > use that to compare objects that don't implement Comparable.  Such a
> > change would alter the signatures of a couple of methods and would be an
> > non-backwards compatible change.
> 
> That only sounds like more constructors, right?  You could probably do that
> in a minor release, especially considering BinaryHeap is neither
> Serializable nor subclass-able.

Nope.  peek() and pop() return Comparables and some of the protected 
methods take a Comparable as an argument.  Changing these from Comparable 
to Object would be an API change because the method signatures would 
change.  

If you can hold off on the release for a few days, I can fix the above
mentioned problems.  Should be fairly simple to do.

regards,
michael



--
To unsubscribe, e-mail:   <ma...@jakarta.apache.org>
For additional commands, e-mail: <ma...@jakarta.apache.org>