Music note class

A class for notes calculation.

#!/usr/bin/env python3

from string import ascii_uppercase

class Notes:
notes = ascii_uppercase[:7]
notes_number = range(7)
notes_dict = dict(zip(notes, notes_number))
numbers_dict = dict(zip(notes_number, notes))

def __init__(self, _note):
if not _note in Notes.notes:
raise Exception("Not a valid note")
self.note = _note
self.note_number = Notes.notes_dict[self.note]

def add(self, n):
if not n in range(8):
raise Exception("Not a valid number")

old_number = Notes.notes_dict[self.note]
new_number = (old_number + n) % 7
return Notes.numbers_dict[new_number]

def minus(self, n):
if not n in range(8):
raise Exception("Not a valid number")

old_number = Notes.notes_dict[self.note]
new_number = (old_number - n) % 7
return Notes.numbers_dict[new_number]


Test:

x = Notes("G")
x.add(7)

• What about sharps/flats and different octaves? Commented Dec 7, 2014 at 11:50

3 Answers

• __init__ assigns self.note_number, but the other functions don't use it, instead they look up the number from the dict. Choose one approach and eliminate the other.
• Checking the range of n and raising the exception in add and minus is not necessary, because the computation has a valid result for all numbers.
• add and minus are almost the same. One function that allows a negative argument would suffice. If you want to keep minus implement it simply as return self.add(-n)

I recommend to rename the class from Notes to Note, as plural suggests a collection of notes, when in fact your implementation is for individual notes.

PEP8 suggests to use x not in items instead of not x in items, so instead of this:

if not n in range(8):


This is the recommended way:

if n not in range(8):


notes and notes_number are not great names, and the second could be derived from the first, like this:

class Note:
valid_note_letters = ascii_uppercase[:7]
valid_note_numbers = range(len(valid_note_letters))


This is not efficient:

    if not n in range(8):


In the worst case, it will iterate over all 8 elements of the range. It would be better this way:

if not 0 <= n < 8:


This is a simple range check with precisely 2 comparisons.

A custom __str__ implementation would be user-friendly:

def __str__(self):
return self.note

• Python 3 doesn't need object; all classes are new-style by default Commented Dec 6, 2014 at 15:10
• Wow, thanks @jonrsharpe for that, good to know Commented Dec 6, 2014 at 15:12

Your code is full of 7 and 8, numbers like that are called magic numbers and should be avoided. Instead use a constant.

• Named constants are certainly appropriate for things which could be changed without rendering a program meaningless. To my eye, however, they are not always appropriate in cases where they aren't just used as numbers but also form part of the structure of a program or its algorithms. Within the context of musical note processing, it would take less thinking for me to recognize the numerical literals 7 and 12 as being more recognizable the number of letter names per octave and half-steps per octave, respectively, than for me to recognize LetterNamesPerOctave and HalfStepsPerOctave. Commented Dec 6, 2014 at 22:11
• A concern I would have using a named constant for LetterNamesPerOctave is that such usage might imply that code could be easily adapted to something like German note names (German uses letters A-H, but doesn't use eight note letters per octave; a C major scale in German would be C-D-E-F-G-A-H-C, while F major would be F-G-A-B-C-D-E-F). If the relationship between letter names and chromatic note numbers is baked into the program structure, numeric literals would simply be a part of that. Commented Dec 6, 2014 at 22:17