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)); }