[Bleeding] Added sharp() and natural() factory functions to mirror the flat() one, sharped() and flattened() functions to get a note from an existing note, a toString(), and more tests. Addresses BUKKIT-861

2 files changed, 106 insertions, 5 deletions

diff --git a/src/main/java/org/bukkit/Note.java b/src/main/java/org/bukkit/Note.java
index 8e053fd7..e1102e49 100644
--- a/src/main/java/org/bukkit/Note.java
+++ b/src/main/java/org/bukkit/Note.java
@@ -131,7 +131,7 @@ public class Note {
      */
     public Note(int octave, Tone tone, boolean sharped) {
         if (sharped && !tone.isSharpable()) {
-            tone = tone == Tone.F ? Tone.G : Tone.values()[tone.ordinal() + 1];
+            tone = Tone.values()[tone.ordinal() + 1];
             sharped = false;
         }
         if (octave < 0 || octave > 2 || (octave == 2 && !(tone == Tone.F && sharped))) {
@@ -144,16 +144,56 @@ public class Note {
     /**
      * Creates a new note for a flat tone, such as A-flat.
      *
-     * @param octave The octave where the note is in. Has to be 0 - 2.
-     * @param tone The tone within the octave. If the octave is 2 the note has to be F#.
+     * @param octave The octave where the note is in. Has to be 0 - 1.
+     * @param tone The tone within the octave.
      * @return The new note.
      */
     public static Note flat(int octave, Tone tone) {
+        Validate.isTrue(octave != 2, "Octave cannot be 2 for flats");
         tone = tone == Tone.G ? Tone.F : Tone.values()[tone.ordinal() - 1];
         return new Note(octave, tone, tone.isSharpable());
     }
 
     /**
+     * Creates a new note for a sharp tone, such as A-sharp.
+     *
+     * @param octave The octave where the note is in. Has to be 0 - 2.
+     * @param tone The tone within the octave. If the octave is 2 the note has to be F#.
+     * @return The new note.
+     */
+    public static Note sharp(int octave, Tone tone) {
+        return new Note(octave, tone, true);
+    }
+
+    /**
+     * Creates a new note for a natural tone, such as A-natural.
+     *
+     * @param octave The octave where the note is in. Has to be 0 - 1.
+     * @param tone The tone within the octave.
+     * @return The new note.
+     */
+    public static Note natural(int octave, Tone tone) {
+        Validate.isTrue(octave != 2, "Octave cannot be 2 for naturals");
+        return new Note(octave, tone, false);
+    }
+
+    /**
+     * @return The note a semitone above this one.
+     */
+    public Note sharped() {
+        Validate.isTrue(note < 24, "This note cannot be sharped because it is the highest known note!");
+        return new Note(note + 1);
+    }
+
+    /**
+     * @return The note a semitone below this one.
+     */
+    public Note flattened() {
+        Validate.isTrue(note > 0, "This note cannot be flattened because it is the lowest known note!");
+        return new Note(note - 1);
+    }
+
+    /**
      * Returns the internal id of this note.
      *
      * @return the internal id of this note.
@@ -215,4 +255,9 @@ public class Note {
             return false;
         return true;
     }
+
+    @Override
+    public String toString() {
+        return "Note{" + getTone().toString() + (isSharped() ? "#" : "") + "}";
+    }
 }
diff --git a/src/test/java/org/bukkit/NoteTest.java b/src/test/java/org/bukkit/NoteTest.java
index b76b0b6b..4759041a 100644
--- a/src/test/java/org/bukkit/NoteTest.java
+++ b/src/test/java/org/bukkit/NoteTest.java
@@ -2,6 +2,7 @@ package org.bukkit;
 
 import static org.hamcrest.CoreMatchers.is;
 import static org.hamcrest.CoreMatchers.nullValue;
+import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertThat;
@@ -79,9 +80,64 @@ public class NoteTest {
         new Note((byte) 3, Note.Tone.A, true);
     }
 
-    @Test(expected = IllegalArgumentException.class)
+    @Test
     public void createNoteOctaveNonSharpable() {
-        new Note((byte) 0, Note.Tone.B, true);
+        Note note = new Note((byte) 0, Note.Tone.B, true);
+        assertFalse(note.isSharped());
+        assertThat(note.getTone(), is(Note.Tone.C));
+    }
+
+    @Test
+    public void createNoteFlat() {
+        Note note = Note.flat(0, Note.Tone.D);
+        assertTrue(note.isSharped());
+        assertThat(note.getTone(), is(Note.Tone.C));
+    }
+
+    @Test
+    public void createNoteFlatNonFlattenable() {
+        Note note = Note.flat(0, Note.Tone.C);
+        assertFalse(note.isSharped());
+        assertThat(note.getTone(), is(Note.Tone.B));
+    }
+
+    @Test
+    public void testFlatWrapping() {
+        Note note = Note.flat(1, Note.Tone.G);
+        assertTrue(note.isSharped());
+        assertThat(note.getTone(), is(Note.Tone.F));
+    }
+
+    @Test
+    public void testFlatWrapping2() {
+        Note note = new Note(1, Note.Tone.G, false).flattened();
+        assertTrue(note.isSharped());
+        assertThat(note.getTone(), is(Note.Tone.F));
+    }
+
+    @Test
+    public void testSharpWrapping() {
+        Note note = new Note(1, Note.Tone.F, false).sharped();
+        assertTrue(note.isSharped());
+        assertThat(note.getTone(), is(Note.Tone.F));
+        assertEquals(note.getOctave(), 2);
+    }
+
+    @Test(expected=IllegalArgumentException.class)
+    public void testSharpWrapping2() {
+        new Note(2, Note.Tone.F, true).sharped();
+    }
+
+    @Test
+    public void testHighest() {
+        Note note = new Note(2, Note.Tone.F, true);
+        assertEquals(note.getId(), (byte)24);
+    }
+
+    @Test
+    public void testLowest() {
+        Note note = new Note(0, Note.Tone.F, true);
+        assertEquals(note.getId(), (byte)0);
     }
 
     @Test