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

2005-09-26 Thread Anthony Green
On Mon, 2005-09-26 at 09:48 -0600, Tom Tromey wrote:
> 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've cleaned most of these up in the attached patch.  This patch also
removes the declaration of some unchecked IllegalArgumentException
throws, as per the hacking guide.  I'm checking it in.

AG

2005-09-26  Anthony Green  <[EMAIL PROTECTED]>

* javax/sound/midi/Synthesizer.java (loadInstrument,
unloadInstrument, remapInstrument, loadAllInstruments,
unloadAllInstruments, unloadInstrument, loadInstrument): Don't
declare the unchecked IllegalArgumentException.
* javax/sound/midi/MidiSystem.java (getMidiDevice, write): Ditto.

* javax/sound/midi/ShortMessage.java: Fix 80-column formatting
problem.
* javax/sound/midi/Sequence.java: Ditto.
* javax/sound/midi/MidiMessage.java: Ditto.
* javax/sound/midi/MidiSystem.java: Ditto.
* javax/sound/midi/MidiFileFormat.java: Ditto.

Index: javax/sound//midi/MidiFileFormat.java
===
RCS file: /cvsroot/classpath/classpath/javax/sound/midi/MidiFileFormat.java,v
retrieving revision 1.1
diff -u -r1.1 MidiFileFormat.java
--- javax/sound//midi/MidiFileFormat.java   26 Sep 2005 16:35:00 -  
1.1
+++ javax/sound//midi/MidiFileFormat.java   26 Sep 2005 17:20:29 -
@@ -95,7 +95,8 @@
* @param bytes the MIDI file size in bytes
* @param microseconds the MIDI file length in microseconds
*/
-  public MidiFileFormat(int type, float divisionType, int resolution, int 
bytes, long microseconds)
+  public MidiFileFormat(int type, float divisionType, 
+   int resolution, int bytes, long microseconds)
   {
 this.type = type;
 this.divisionType = divisionType;
Index: javax/sound//midi/MidiMessage.java
===
RCS file: /cvsroot/classpath/classpath/javax/sound/midi/MidiMessage.java,v
retrieving revision 1.1
diff -u -r1.1 MidiMessage.java
--- javax/sound//midi/MidiMessage.java  26 Sep 2005 16:35:00 -  1.1
+++ javax/sound//midi/MidiMessage.java  26 Sep 2005 17:20:29 -
@@ -75,7 +75,8 @@
* @param length The length of the MIDI message.
* @throws InvalidMidiDataException Thrown when the MIDI message is invalid.
*/
-  protected void setMessage(byte[] data, int length) throws 
InvalidMidiDataException
+  protected void setMessage(byte[] data, int length) 
+throws InvalidMidiDataException
   {
 this.data = new byte[length];
 System.arraycopy(data, 0, this.data, 0, length);
Index: javax/sound//midi/MidiSystem.java
===
RCS file: /cvsroot/classpath/classpath/javax/sound/midi/MidiSystem.java,v
retrieving revision 1.1
diff -u -r1.1 MidiSystem.java
--- javax/sound//midi/MidiSystem.java   26 Sep 2005 16:35:00 -  1.1
+++ javax/sound//midi/MidiSystem.java   26 Sep 2005 17:20:29 -
@@ -71,7 +71,8 @@
*/
   public static MidiDevice.Info[] getMidiDeviceInfo()
   {
-Iterator deviceProviders = 
ServiceFactory.lookupProviders(MidiDeviceProvider.class);
+Iterator deviceProviders = 
+   ServiceFactory.lookupProviders(MidiDeviceProvider.class);
 List infoList = new ArrayList();
 
 while (deviceProviders.hasNext())
@@ -82,7 +83,8 @@
 infoList.add(infos[--i]);
 }
 
-return (MidiDevice.Info[]) infoList.toArray(new 
MidiDevice.Info[infoList.size()]);
+return (MidiDevice.Info[]) 
+   infoList.toArray(new MidiDevice.Info[infoList.size()]);
   }
   
   /**
@@ -94,9 +96,10 @@
* @throws IllegalArgumentException if the device described by info is not 
found
*/
   public static MidiDevice getMidiDevice(MidiDevice.Info info) 
-throws MidiUnavailableException, IllegalArgumentException
+throws MidiUnavailableException
   {
-Iterator deviceProviders = 
ServiceFactory.lookupProviders(MidiDeviceProvider.class);
+Iterator deviceProviders = 
+   ServiceFactory.lookupProviders(MidiDeviceProvider.class);
 
 if (! deviceProviders.hasNext())
   throw new MidiUnavailableException("No MIDI device providers 
available.");
@@ -109,7 +112,8 @@
 return provider.getDevice(info);
 } while (deviceProviders.hasNext());
 
-throw new IllegalArgumentException("MIDI device " + info + " not 
available.");
+throw new IllegalArgumentException("MIDI device " 
+  + info + " not available.");
   }
   
   /**
@@ -259,7 +263,8 @@
   if (sb != null)
 return sb;
 }
-throw new InvalidMidiDataException("Cannot read soundbank from file " + 
file);
+throw new InvalidMidiDataException("Cannot read soundbank from file " 
+  + file);
   } 
 
   /**
@@ -281,7 +286,7 @@
   i

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

2005-09-26 Thread Tom Tromey
Anthony> This doesn't quite work because ShortMessage's parent
Anthony> declares an abstract clone(), so you can't super.clone() it.
Anthony> Perhaps they did this to force the implementation method I
Anthony> chose.

Ok, thanks.  How amazingly lame.

Tom


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


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

2005-09-26 Thread Anthony Green
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


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

2005-09-26 Thread Tom Tromey
> "Anthony" == Anthony Green <[EMAIL PROTECTED]> writes:

Anthony> Here's a virtually complete javax.sound.midi implementation.
Anthony> No providers yet.
Anthony> Ok?

I took a quick look through this.

I think this is looking great.  I think it is OK to go in.

First though, some minor nits, one real bug:


I think @author should have your full name, like:

@author Anthony Green ([EMAIL PROTECTED])

Each class' javadoc should say '@since 1.3'.

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.

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.)

Tom


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