patternpythonMinor
Python script to test music sight reading
Viewed 0 times
scriptreadingtestpythonmusicsight
Problem
I decided to write a program to test music theory, and, while it would've been much easier for me to make it elegant and perfect in Java, I thought I'd take the opportunity to get more familiar with Python.
Below is the full script. It simply prints a staff in treble of bass clef with a note somewhere and asks the user to identify it.
Things I'm particularly interested in getting feedback about:
Of course, any and all feedback is welcome, especially including anything which breaks from conventional Python style. I'd like to be as Pythonic as possible.
```
import random
import time
class Clef:
def __init__(self, name, notes):
self.name = name
self.notes = n
Below is the full script. It simply prints a staff in treble of bass clef with a note somewhere and asks the user to identify it.
Things I'm particularly interested in getting feedback about:
- The
@staticmethodin theStaffclass. I tried this both as an instance function (is it even appropriate to call it this in Python?) and as the static version below, but both seemed kind of clunky to me. It doesn't make much sense to me that I have to refer to the class's fields with the class name when I'm already in the class (e.g.,Staff.linesinstead of justlines, orself.linesif it's an instance function). It makes me think I'm doing something wrong.
- I've heard multiple times that having constants isn't very Pythonic. Is there a better way for me to handle the
TREBLE_CLEF,BASS_CLEF,TREBLE_CLEF_NOTES, andBASS_CLEF_NOTESfields?
- I'm interested in a solution which allows the user to input a command like
exitinto the prompt in order to leave the program. I know I could've put this in thetest_notefunction, but that would break the principle that each function should do only one thing and do it well (i.e., thetest_notefunction test's the user's knowledge of the staff -- it shouldn't be responsible at all for controlling the application or its state). I couldn't think of an elegant way to do this and still preserve that principle, so I just bypassed the issue for now, haha.
- Am I using classes appropriately? Any way I can use them better?
Of course, any and all feedback is welcome, especially including anything which breaks from conventional Python style. I'd like to be as Pythonic as possible.
```
import random
import time
class Clef:
def __init__(self, name, notes):
self.name = name
self.notes = n
Solution
Warning for other reviewers : input and input are different depending on the version of Python you are using. This is about Python 3.
Instead of defining the notes of a clef as a dictionnary mapping consecutive integers from 0 to n to names, you could use a list.
You'll have the much more natural :
and you just need to change :
The way you are playing with string slicing to display a note or a line doesn't need to be that complicated. Also, defining values as class members was a nice touch but the fact that you hardcode indices in the logic with the string slicing kind of removes the all point. As for me, I'll keep things simple and not so well organised for the time being as these values are unlikely to be useful for anything else than displaying.
Here's my suggestion :
I have to go, I'll try to add more details later. In the meantime, if you are interested in Python and music theory, you might be interested by this github project.
Instead of defining the notes of a clef as a dictionnary mapping consecutive integers from 0 to n to names, you could use a list.
You'll have the much more natural :
TREBLE_CLEF_NOTES = [ 'F', 'E', 'D', 'C', 'B', 'A', 'G', 'F', 'E' ]
BASS_CLEF_NOTES = [ 'A', 'G', 'F', 'E', 'D', 'C', 'B', 'A', 'G' ]and you just need to change :
def random_note(self):
return random.choice(range(len(self.notes)))add_note = i == note_index does not seem useful : get rid of it.The way you are playing with string slicing to display a note or a line doesn't need to be that complicated. Also, defining values as class members was a nice touch but the fact that you hardcode indices in the logic with the string slicing kind of removes the all point. As for me, I'll keep things simple and not so well organised for the time being as these values are unlikely to be useful for anything else than displaying.
Here's my suggestion :
def display(note_index):
for i in range(9):
line_element = '-' if i%2 else ' '
print(line_element * 4 + ('O' if i == note_index else line_element) + line_element*5)I have to go, I'll try to add more details later. In the meantime, if you are interested in Python and music theory, you might be interested by this github project.
Code Snippets
TREBLE_CLEF_NOTES = [ 'F', 'E', 'D', 'C', 'B', 'A', 'G', 'F', 'E' ]
BASS_CLEF_NOTES = [ 'A', 'G', 'F', 'E', 'D', 'C', 'B', 'A', 'G' ]def random_note(self):
return random.choice(range(len(self.notes)))def display(note_index):
for i in range(9):
line_element = '-' if i%2 else ' '
print(line_element * 4 + ('O' if i == note_index else line_element) + line_element*5)Context
StackExchange Code Review Q#46665, answer score: 8
Revisions (0)
No revisions yet.