classpath-patches
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [cp-patches] Patch: javax.sound.midi


From: Anthony Green
Subject: Re: [cp-patches] Patch: javax.sound.midi
Date: Mon, 26 Sep 2005 09:31:36 -0700

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 (address@hidden)
> 
> 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






reply via email to

[Prev in Thread] Current Thread [Next in Thread]