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/20 21:37:19 UTC

[Collections] Release status?

Hi Michael et al.

I've lost track.  Is there anything else pending, or can I retag and create
a new Release Candidate?

- 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] Release status?

Posted by "Michael A. Smith" <ma...@apache.org>.
On Wed, 20 Mar 2002, Morgan Delagrange wrote:
> Sure, we can wait until tomorrow.  Just be sure to get it all on the table
> at once, so we don't spread out the discussion unnecessarily.

Yeah, that's what I meant to do before, but I somehow managed to forget
what I sat down to do and ended up just fixing BinaryHeap et al.  Might
have something to do with the food poisoning I was still recovering from
at the time...

regards,
michael


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


Re: [Collections] non-compatible changes

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>
Sent: Friday, March 22, 2002 5:58 PM
Subject: Re: [Collections] non-compatible changes


> On Fri, 22 Mar 2002, James Strachan wrote:
> > > It's not ideal, but it's a little late in the game to change it.  I've
> > > updated the documentation to point out this behaviour.  (Also, the
fact
> > that
> > > changes to the keySet() are reflected by the Map is in keeping with
the
> > > general contract for Map.)
> > >
> > > I would be willing to retract my -1 if you got buy-in from James and
there
> > > were no other objections.  However, if we're going to fix it, let's
make
> > it
> > > agree with the Map contract.  Personally, changing it at this point
makes
> > me
> > > uncomfortable, and I'd rather not do it.
> >
> > I kinda agree with you both ;-)
> >
> > I think ultimately BeanMap should be fixed to truly implement the Map
> > interface if we can. Though the Collections package is built around the
idea
> > that not all operations need to be supported, so using unmodifiable
> > collections for keySet() seems to make sense; especially as keys cannot
be
> > removed or added - only values can be updated.
> >
> > So I'd prefer keySet() to be unmodifiable and values() to be modifiable
and
> > reflected in the Map - if it can't be reflected easily (or until it gets
> > implemented) then I'd prefer it to be unmodifiable as well for now. Its
> > better for a feature to not be available than for it to be wrong.
>
> Morgan,
>
> Is this considered buy-in from James?  If so, I'll get to work
> implementing this stuff and getting the changes in as quickly as possible.
>
> regards,
> michael

Works for me, as long as nobody else objects.

- 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] non-compatible changes

Posted by "Michael A. Smith" <ma...@apache.org>.
On Fri, 22 Mar 2002, James Strachan wrote:
> > It's not ideal, but it's a little late in the game to change it.  I've
> > updated the documentation to point out this behaviour.  (Also, the fact
> that
> > changes to the keySet() are reflected by the Map is in keeping with the
> > general contract for Map.)
> >
> > I would be willing to retract my -1 if you got buy-in from James and there
> > were no other objections.  However, if we're going to fix it, let's make
> it
> > agree with the Map contract.  Personally, changing it at this point makes
> me
> > uncomfortable, and I'd rather not do it.
> 
> I kinda agree with you both ;-)
> 
> I think ultimately BeanMap should be fixed to truly implement the Map
> interface if we can. Though the Collections package is built around the idea
> that not all operations need to be supported, so using unmodifiable
> collections for keySet() seems to make sense; especially as keys cannot be
> removed or added - only values can be updated.
> 
> So I'd prefer keySet() to be unmodifiable and values() to be modifiable and
> reflected in the Map - if it can't be reflected easily (or until it gets
> implemented) then I'd prefer it to be unmodifiable as well for now. Its
> better for a feature to not be available than for it to be wrong.

Morgan,

Is this considered buy-in from James?  If so, I'll get to work
implementing this stuff and getting the changes in as quickly as possible.

regards,
michael


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


Re: [Collections] non-compatible changes

Posted by James Strachan <ja...@yahoo.co.uk>.
From: "Morgan Delagrange" <md...@yahoo.com>
> ----- Original Message -----
> From: "Michael A. Smith" <ma...@apache.org>
> > On Thu, 21 Mar 2002, Morgan Delagrange wrote:
> > > > 3. BeanMap.values() is modifiable, but modifications are not
reflected
> in
> > > > the BeanMap.  Probably should return an unmodifiable collection.
> > > >
> > > > 4. BeanMap.keySet() is modifiable.  Modifications to it (i.e.
removes)
> > > > will be reflected in the BeanMap, but that's probably not a good
idea.
> > > > Probably should return an unmodifiable collection.
> > >
> > > I'm -1 to these changes.  There may be clients to BeanMap that expect
> this
> > > behaviour, and I'm uncomfortable with changing it in the eleventh hour
> of
> > > the release.  I think that, for now, we should just document the
> behavior in
> > > the Javadocs.
> >
> > So, you'd rather leave the behavior such that changes to keySet() are
> > reflected in the map, entrySet() is unmodifiable, and changes to
values()
> > are not reflected in the map?  Having three different behaviors for
these
> > just seems wrong to me as well.
>
> It's not ideal, but it's a little late in the game to change it.  I've
> updated the documentation to point out this behaviour.  (Also, the fact
that
> changes to the keySet() are reflected by the Map is in keeping with the
> general contract for Map.)
>
> I would be willing to retract my -1 if you got buy-in from James and there
> were no other objections.  However, if we're going to fix it, let's make
it
> agree with the Map contract.  Personally, changing it at this point makes
me
> uncomfortable, and I'd rather not do it.

I kinda agree with you both ;-)

I think ultimately BeanMap should be fixed to truly implement the Map
interface if we can. Though the Collections package is built around the idea
that not all operations need to be supported, so using unmodifiable
collections for keySet() seems to make sense; especially as keys cannot be
removed or added - only values can be updated.

So I'd prefer keySet() to be unmodifiable and values() to be modifiable and
reflected in the Map - if it can't be reflected easily (or until it gets
implemented) then I'd prefer it to be unmodifiable as well for now. Its
better for a feature to not be available than for it to be wrong.

James


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


Re: [Collections] non-compatible changes

Posted by Morgan Delagrange <md...@yahoo.com>.
----- Original Message -----
From: "Michael A. Smith" <ma...@apache.org>
To: "Morgan Delagrange" <mo...@apache.org>
Cc: "Jakarta Commons Developers List" <co...@jakarta.apache.org>
Sent: Thursday, March 21, 2002 1:26 PM
Subject: Re: [Collections] non-compatible changes


> On Thu, 21 Mar 2002, Morgan Delagrange wrote:
> > > 3. BeanMap.values() is modifiable, but modifications are not reflected
in
> > > the BeanMap.  Probably should return an unmodifiable collection.
> > >
> > > 4. BeanMap.keySet() is modifiable.  Modifications to it (i.e. removes)
> > > will be reflected in the BeanMap, but that's probably not a good idea.
> > > Probably should return an unmodifiable collection.
> >
> > I'm -1 to these changes.  There may be clients to BeanMap that expect
this
> > behaviour, and I'm uncomfortable with changing it in the eleventh hour
of
> > the release.  I think that, for now, we should just document the
behavior in
> > the Javadocs.
>
> So, you'd rather leave the behavior such that changes to keySet() are
> reflected in the map, entrySet() is unmodifiable, and changes to values()
> are not reflected in the map?  Having three different behaviors for these
> just seems wrong to me as well.

It's not ideal, but it's a little late in the game to change it.  I've
updated the documentation to point out this behaviour.  (Also, the fact that
changes to the keySet() are reflected by the Map is in keeping with the
general contract for Map.)

I would be willing to retract my -1 if you got buy-in from James and there
were no other objections.  However, if we're going to fix it, let's make it
agree with the Map contract.  Personally, changing it at this point makes me
uncomfortable, and I'd rather not do it.

> > > 5. DoubleOrderedMap requires its keys and elements to be comparable.
I
> > > don't think the external API relies on Comparable (just the internal
API),
> > > so I don't think this must be fixed before a 2.0 release, but I didn't
> > > look quite that closely to know for sure.
> > >
> > > 6. The following classes have "protected" members and/or methods, but
> > > the ability to subclass without "corrupting" the internal state of
> > > the parent class is suspect: BeanMap, FastArrayList, FastHashMap,
> > > FastTreeMap.  Fixing these would probably involve making some fields
> > > private.
> >
> > -1.  We should minimize changes that affect Serialization, and in this
case
> > it doesn't seem worth the price.
>
> I don't know how either of these have effects on Serialization.
>
> DoubleOrderedMap isn't serializable, and even if it was, it doesn't have
> any Comparable fields that would be serialized (all the Comparable
> references are casts in order to do a comparison).

I didn't mean to refer to #5, since you aren't proposing any changes.

> As for changing protected members to private or otherwise adjusting to
> ensure subclasses can't corrupt internal structures, that doesn't change
> serialization either.  If anything, it just requires the addition of a
> serialVersionUID to say "yes, this class really does have the same
> fields".

If you take care to preserve Serialization, that would be a plus.  (I'm far
from the world's expert on Serialization  :)  But what about existing
subclasses?  I cannot say with certainty that such subclasses do not exist
and do not rely on methods we currently expose.  We can insert warnings in
the Javadocs, but changing the interface seems premature.

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


_________________________________________________________
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] non-compatible changes

Posted by "Michael A. Smith" <ma...@apache.org>.
On Thu, 21 Mar 2002, Morgan Delagrange wrote:
> > 3. BeanMap.values() is modifiable, but modifications are not reflected in
> > the BeanMap.  Probably should return an unmodifiable collection.
> >
> > 4. BeanMap.keySet() is modifiable.  Modifications to it (i.e. removes)
> > will be reflected in the BeanMap, but that's probably not a good idea.
> > Probably should return an unmodifiable collection.
> 
> I'm -1 to these changes.  There may be clients to BeanMap that expect this
> behaviour, and I'm uncomfortable with changing it in the eleventh hour of
> the release.  I think that, for now, we should just document the behavior in
> the Javadocs.

So, you'd rather leave the behavior such that changes to keySet() are 
reflected in the map, entrySet() is unmodifiable, and changes to values() 
are not reflected in the map?  Having three different behaviors for these 
just seems wrong to me as well. 

> > 5. DoubleOrderedMap requires its keys and elements to be comparable.  I
> > don't think the external API relies on Comparable (just the internal API),
> > so I don't think this must be fixed before a 2.0 release, but I didn't
> > look quite that closely to know for sure.
> >
> > 6. The following classes have "protected" members and/or methods, but
> > the ability to subclass without "corrupting" the internal state of
> > the parent class is suspect: BeanMap, FastArrayList, FastHashMap,
> > FastTreeMap.  Fixing these would probably involve making some fields
> > private.
> 
> -1.  We should minimize changes that affect Serialization, and in this case
> it doesn't seem worth the price.

I don't know how either of these have effects on Serialization.  

DoubleOrderedMap isn't serializable, and even if it was, it doesn't have 
any Comparable fields that would be serialized (all the Comparable 
references are casts in order to do a comparison).  

As for changing protected members to private or otherwise adjusting to 
ensure subclasses can't corrupt internal structures, that doesn't change 
serialization either.  If anything, it just requires the addition of a 
serialVersionUID to say "yes, this class really does have the same 
fields".  


regards,
michael



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


Re: [Collections] non-compatible changes

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: Thursday, March 21, 2002 12:07 AM
Subject: [Collections] non-compatible changes


> On Wed, 20 Mar 2002, Morgan Delagrange wrote:
> > Sure, we can wait until tomorrow.  Just be sure to get it all on the
table
> > at once, so we don't spread out the discussion unnecessarily.
>
> Okay, I've finished my (somewhat hurried and tiring) review for possible
> non-backwards compatible changes.  I'm willing to do the necessary work
> for all of these.
>
> Here's the list of issues and improvements I have found in collections
> which require non-backwards compatible changes (and thus probably should
> go in before a 2.0 release if they are to go in at all):
>
> 1. AbstractBag seems to be more of a "default map based implementation"
> and differs in design from the AbstractSet, AbstractMap classes which do
> not make assumptions about how they might be implemented.  To be
> consistent with JDK AbstractX collections, this should just be providing
> default implementations that could be used regardless of underlying
> storage mechanism.  For example, the add(Object) method would call the
> abstract add(Object,int) method passing the object and 1.  I think
> iterator() would need to be the only other abstract method.
>
> There's a few ways to fix this.  First, reimplement so that only the
> minimal interfaces need to be implemented, thus matching the style of the
> JDK AbstractX collections.  Second rename to DefaultMapBag or similar so
> that there isn't a difference in style.  Third, a combination of both.
> The minimum necessary for a 2.0 release is just the rename.
>
> If you're confused about what I mean here, I can probably put together a
> patch this weekend for the third option (a new AbstractBag and a
> DefaultMapBag).

Sounds fine.

> 2. ArrayEnumeration is not fail-fast when a (Object[])null value is passed
> to the constructor.

Performing maintenance on a deprecated class seems unnecessary.  -0.

> 3. BeanMap.values() is modifiable, but modifications are not reflected in
> the BeanMap.  Probably should return an unmodifiable collection.
>
> 4. BeanMap.keySet() is modifiable.  Modifications to it (i.e. removes)
> will be reflected in the BeanMap, but that's probably not a good idea.
> Probably should return an unmodifiable collection.

I'm -1 to these changes.  There may be clients to BeanMap that expect this
behaviour, and I'm uncomfortable with changing it in the eleventh hour of
the release.  I think that, for now, we should just document the behavior in
the Javadocs.

> 5. DoubleOrderedMap requires its keys and elements to be comparable.  I
> don't think the external API relies on Comparable (just the internal API),
> so I don't think this must be fixed before a 2.0 release, but I didn't
> look quite that closely to know for sure.
>
> 6. The following classes have "protected" members and/or methods, but
> the ability to subclass without "corrupting" the internal state of
> the parent class is suspect: BeanMap, FastArrayList, FastHashMap,
> FastTreeMap.  Fixing these would probably involve making some fields
> private.

-1.  We should minimize changes that affect Serialization, and in this case
it doesn't seem worth the price.

> My brain power started dying, so I didn't really review the following
> classes (unfortunate since these are pretty sophisticated classes):
> CollectionUtils, CursorableLinkedList, ExtendedProperties, SoftRefHashMap
>
> There's also a lot of places where I disagree with the implementation of
> things, but I think it'd probably be too much for me to complain about all
> of them as well (e.g. MapUtils.getXXX() methods should throw exceptions or
> return null for things that aren't XXX rather than "massaging" them into
> XXX or using System.out.println)
>
> One other note:  package.html probably should be updated before 2.0
> release.

Will do.

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


[Collections] non-compatible changes

Posted by "Michael A. Smith" <ma...@apache.org>.
On Wed, 20 Mar 2002, Morgan Delagrange wrote:
> Sure, we can wait until tomorrow.  Just be sure to get it all on the table
> at once, so we don't spread out the discussion unnecessarily.

Okay, I've finished my (somewhat hurried and tiring) review for possible 
non-backwards compatible changes.  I'm willing to do the necessary work 
for all of these.

Here's the list of issues and improvements I have found in collections
which require non-backwards compatible changes (and thus probably should
go in before a 2.0 release if they are to go in at all):

1. AbstractBag seems to be more of a "default map based implementation"  
and differs in design from the AbstractSet, AbstractMap classes which do
not make assumptions about how they might be implemented.  To be
consistent with JDK AbstractX collections, this should just be providing
default implementations that could be used regardless of underlying
storage mechanism.  For example, the add(Object) method would call the
abstract add(Object,int) method passing the object and 1.  I think
iterator() would need to be the only other abstract method.

There's a few ways to fix this.  First, reimplement so that only the
minimal interfaces need to be implemented, thus matching the style of the
JDK AbstractX collections.  Second rename to DefaultMapBag or similar so
that there isn't a difference in style.  Third, a combination of both.  
The minimum necessary for a 2.0 release is just the rename.

If you're confused about what I mean here, I can probably put together a 
patch this weekend for the third option (a new AbstractBag and a 
DefaultMapBag).

2. ArrayEnumeration is not fail-fast when a (Object[])null value is passed
to the constructor.  

3. BeanMap.values() is modifiable, but modifications are not reflected in 
the BeanMap.  Probably should return an unmodifiable collection.

4. BeanMap.keySet() is modifiable.  Modifications to it (i.e. removes) 
will be reflected in the BeanMap, but that's probably not a good idea.  
Probably should return an unmodifiable collection.

5. DoubleOrderedMap requires its keys and elements to be comparable.  I 
don't think the external API relies on Comparable (just the internal API), 
so I don't think this must be fixed before a 2.0 release, but I didn't 
look quite that closely to know for sure.

6. The following classes have "protected" members and/or methods, but 
the ability to subclass without "corrupting" the internal state of 
the parent class is suspect: BeanMap, FastArrayList, FastHashMap, 
FastTreeMap.  Fixing these would probably involve making some fields 
private. 

My brain power started dying, so I didn't really review the following 
classes (unfortunate since these are pretty sophisticated classes):
CollectionUtils, CursorableLinkedList, ExtendedProperties, SoftRefHashMap

There's also a lot of places where I disagree with the implementation of 
things, but I think it'd probably be too much for me to complain about all 
of them as well (e.g. MapUtils.getXXX() methods should throw exceptions or 
return null for things that aren't XXX rather than "massaging" them into 
XXX or using System.out.println)

One other note:  package.html probably should be updated before 2.0 
release. 

tired,
michael


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


Re: [Collections] Release status?

Posted by Morgan Delagrange <md...@yahoo.com>.
Sure, we can wait until tomorrow.  Just be sure to get it all on the table
at once, so we don't spread out the discussion unnecessarily.

----- Original Message -----
From: "Michael A. Smith" <ma...@apache.org>
To: "Jakarta Commons Developers List" <co...@jakarta.apache.org>;
"Morgan Delagrange" <mo...@apache.org>
Sent: Wednesday, March 20, 2002 3:03 PM
Subject: Re: [Collections] Release status?


> On Wed, 20 Mar 2002, Morgan Delagrange wrote:
> > Hi Michael et al.
> >
> > I've lost track.  Is there anything else pending, or can I retag and
create
> > a new Release Candidate?
>
> Well, I've fixed all the things on my todo list that can't be made in a
> 2.0.1 release, but I also haven't finished reviewing the collections code
> for other places that may need fixing up.  I was hoping to finish
> reviewing tonight.  Ok to wait until tomorrow?
>
> 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] Release status?

Posted by "Michael A. Smith" <ma...@apache.org>.
On Wed, 20 Mar 2002, Morgan Delagrange wrote:
> Hi Michael et al.
> 
> I've lost track.  Is there anything else pending, or can I retag and create
> a new Release Candidate?

Well, I've fixed all the things on my todo list that can't be made in a 
2.0.1 release, but I also haven't finished reviewing the collections code 
for other places that may need fixing up.  I was hoping to finish 
reviewing tonight.  Ok to wait until tomorrow?

regards,
michael


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


Re: [Collections] Release status?

Posted by Morgan Delagrange <md...@yahoo.com>.
Cool.  I may have one more addition to make to ComparatorChain, but expect
another release build by tomorrow at the latest.

----- 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 24, 2002 11:59 PM
Subject: Re: [Collections] Release status?


> On Wed, 20 Mar 2002, Morgan Delagrange wrote:
> > I've lost track.  Is there anything else pending, or can I retag and
create
> > a new Release Candidate?
>
> I'm ready now.  I still think there's tons of documentation that could be
> improved (most notably package.html), and tons more improvement that could
> be made to implementations, but it seems like there's some sort of rush to
> get a 2.0 release, and I'm not going to be the one to hold it up any
> longer.  :)
>
> michael
>
>
> --
> To unsubscribe, e-mail:
<ma...@jakarta.apache.org>
> For additional commands, e-mail:
<ma...@jakarta.apache.org>


_________________________________________________________
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] Release status?

Posted by Daniel Rall <dl...@finemaltcoding.com>.
"Michael A. Smith" <ma...@apache.org> writes:

> On Wed, 20 Mar 2002, Morgan Delagrange wrote:
> > I've lost track.  Is there anything else pending, or can I retag and create
>> a new Release Candidate?
>
> I'm ready now.  I still think there's tons of documentation that could be 
> improved (most notably package.html), and tons more improvement that could 
> be made to implementations, but it seems like there's some sort of rush to 
> get a 2.0 release, and I'm not going to be the one to hold it up any 
> longer.  :)

Including more documentation in a minor point release would be a
reasonable compromise.

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


Re: [Collections] Release status?

Posted by "Michael A. Smith" <ma...@apache.org>.
On Wed, 20 Mar 2002, Morgan Delagrange wrote:
> I've lost track.  Is there anything else pending, or can I retag and create
> a new Release Candidate?

I'm ready now.  I still think there's tons of documentation that could be 
improved (most notably package.html), and tons more improvement that could 
be made to implementations, but it seems like there's some sort of rush to 
get a 2.0 release, and I'm not going to be the one to hold it up any 
longer.  :)

michael


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