On Mon, 2005-09-26 at 09:48 -0600, Tom Tromey wrote:
> I think this is looking great.  I think it is OK to go in.

Thanks.  I'll do that.

> I think @author should have your full name, like:
> 
>     @author Anthony Green ([EMAIL PROTECTED])
> 
> Each class' javadoc should say '@since 1.3'.

Ok, I've fixed these.

> At least ShortMessage.clone() uses 'new ShortMessage(...)'.
> This doesn't work if the class is extended.  You must write:
> 
>   try
>     {
>       ShortMessage dup = (ShortMessage) super.clone();
>       .. set fields
>     }
>   catch (CloneNotSupportedException _)
>     {
>       .. I forget what we decided here
>       .. look for other examples
>     }
> 
> I didn't look to see if this occurs elsewhere.

This doesn't quite work because ShortMessage's parent declares an
abstract clone(), so you can't super.clone() it.  Perhaps they did this
to force the implementation method I chose.

> There are one or two places where the code goes past column 79.
> (I'm not super concerned about this.  I think we need a reformatting
> flag day anyway.)

I'll clean these up in a subsequent commit.

Thanks,

AG




_______________________________________________
Classpath-patches mailing list
Classpath-patches@gnu.org
http://lists.gnu.org/mailman/listinfo/classpath-patches

Reply via email to