diff --git a/Semantics.Music/Pitch.cs b/Semantics.Music/Pitch.cs
index 300d683..c2f2b5c 100644
--- a/Semantics.Music/Pitch.cs
+++ b/Semantics.Music/Pitch.cs
@@ -42,8 +42,16 @@ public static Pitch Create(int midi)
/// The octave number.
/// A new pitch.
/// Thrown when the resulting MIDI number is outside 0..127.
- public static Pitch Create(NoteLetter letter, Accidental accidental, int octave) =>
- Create(((octave + 1) * 12) + (int)letter + (int)accidental);
+ public static Pitch Create(NoteLetter letter, Accidental accidental, int octave)
+ {
+ long midi = MidiOf(octave, (int)letter, (int)accidental);
+ if (midi is < 0 or > 127)
+ {
+ throw new ArgumentOutOfRangeException(nameof(octave), octave, "The resulting MIDI note number must be in 0..127.");
+ }
+
+ return new() { Midi = (int)midi };
+ }
/// Parses a note name such as "C4", "F#3", or "Bb5".
/// A letter A-G, optional accidentals, then an octave integer.
@@ -78,21 +86,29 @@ public static bool TryParse(string? name, [System.Diagnostics.CodeAnalysis.NotNu
index++;
int accidental = Notation.ReadAccidentalOffset(name, ref index);
- if (!int.TryParse(name[index..], NumberStyles.Integer, CultureInfo.InvariantCulture, out int octave))
+ // The octave is a bare integer: a leading '-' is allowed for octave -1, but not '+' or
+ // whitespace, which would parse to a pitch that prints back differently.
+ if (index >= name.Length
+ || name[index] == '+'
+ || !int.TryParse(name[index..], NumberStyles.AllowLeadingSign, CultureInfo.InvariantCulture, out int octave))
{
return false;
}
- int midi = ((octave + 1) * 12) + (int)letter + accidental;
+ long midi = MidiOf(octave, (int)letter, accidental);
if (midi is < 0 or > 127)
{
return false;
}
- result = new() { Midi = midi };
+ result = new() { Midi = (int)midi };
return true;
}
+ // Computed in long so an extreme octave cannot wrap back into 0..127.
+ private static long MidiOf(int octave, int letter, int accidental) =>
+ ((octave + 1L) * 12) + letter + accidental;
+
/// Returns the canonical note name (e.g. "C4").
/// The note name.
public override string ToString() => Name;
diff --git a/Semantics.Test/Music/PitchTests.cs b/Semantics.Test/Music/PitchTests.cs
index 05391e9..6447240 100644
--- a/Semantics.Test/Music/PitchTests.cs
+++ b/Semantics.Test/Music/PitchTests.cs
@@ -57,4 +57,36 @@ public void TryParseFailsOnBadOctave()
Assert.IsFalse(Pitch.TryParse("Cx", out Pitch? result));
Assert.IsNull(result);
}
+
+ [TestMethod]
+ [DataRow("C1073741823")]
+ [DataRow("C-1073741825")]
+ [DataRow("C2147483647")]
+ [DataRow("C-2147483648")]
+ [DataRow("C 4")]
+ [DataRow("C 4 ")]
+ [DataRow("C4 ")]
+ [DataRow("C+4")]
+ [DataRow("C")]
+ [DataRow("C#")]
+ public void TryParse_RejectsWrappingOrNonCanonicalOctave(string name)
+ {
+ Assert.IsFalse(Pitch.TryParse(name, out Pitch? result));
+ Assert.IsNull(result);
+ }
+
+ [TestMethod]
+ public void TryParse_AcceptsNegativeOctave()
+ {
+ Assert.IsTrue(Pitch.TryParse("C-1", out Pitch? result));
+ Assert.AreEqual(0, result.Midi);
+ Assert.AreEqual("C-1", result.Name);
+ }
+
+ [TestMethod]
+ [DataRow(1073741823)]
+ [DataRow(int.MaxValue)]
+ [DataRow(int.MinValue)]
+ public void Create_FromLetter_RejectsOctaveThatWouldWrap(int octave) =>
+ Assert.ThrowsExactly(() => Pitch.Create(NoteLetter.C, Accidental.Natural, octave));
}