patternjavascriptMinor
Piano, output all the keys and give a scale
Viewed 0 times
theallgiveoutputkeyspianoandscale
Problem
I've two snippets below, the first one output all the keys of a piano and the second one give me a scale when given a starting note. For those who don't know how a piano is structured here is an image of a piano.
Basically I want to put each notes, in an ordered array. As you can see the black notes have two names, one with
So I have to create this array (like the image):
Or check the console output from the jsfiddle below.
jsfiddle
The second algorithm returns a scale given a note. For those who don't know a scale starts on any note and is given by the algorithm below:
Major: 2 - 2 - 1 - 2 - 2 - 2 - 1
Minor : 2 - 1 - 2 - 2 - 1 - 2 - 2
We can distinguish between major and minor because an
Where 1 means go one note above, and 2 means go 2 notes above (the black keys are also notes).
Here is jsfiddle
Code :
```
majorSemiTones = [2,2,1,2,2,2];
minorSemiTones = [2,1,2,2,1,2];
KEYS = ["A", "Bb", "B", "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab"];
KEYS_NORMALIZED = ["C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab", "A", "Bb", "B"];
function getScale(scaleName){
let tonic = scaleName.replace("m", "");
let keys = [];
let minor = false;
let scaleIndex;
let intervals = this.majorSemiTones;
if(scaleName.indexOf("m") > -1){
minor = true;
intervals = this.minorSemiTones;
}
scaleIndex = KEYS.index
Basically I want to put each notes, in an ordered array. As you can see the black notes have two names, one with
b in it and one with #. Those two names are equivalent but I'm using the one with b.So I have to create this array (like the image):
[ "A0", "Bb0", "B0", "C1", "Db1", "D1", "Eb1", "E1", "F1", "Gb1", "G1", "Ab1", "A1", "Bb1", "B1".............., "B7", "C8"]Or check the console output from the jsfiddle below.
jsfiddle
const KEYS_NORMAL = ["C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab", "A", "Bb", "B"];
let keys = KEYS_NORMAL.slice(-3);
keys = keys.map( k => k + '0');
for(let octave = 1; octave console.log(k));
// I'm actually doing some work with those notes, So ultimately this is one more loop.The second algorithm returns a scale given a note. For those who don't know a scale starts on any note and is given by the algorithm below:
Major: 2 - 2 - 1 - 2 - 2 - 2 - 1
Minor : 2 - 1 - 2 - 2 - 1 - 2 - 2
We can distinguish between major and minor because an
m is appended to the name, eg: A is major whereas Am is minor.Where 1 means go one note above, and 2 means go 2 notes above (the black keys are also notes).
Here is jsfiddle
Code :
```
majorSemiTones = [2,2,1,2,2,2];
minorSemiTones = [2,1,2,2,1,2];
KEYS = ["A", "Bb", "B", "C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab"];
KEYS_NORMALIZED = ["C", "Db", "D", "Eb", "E", "F", "Gb", "G", "Ab", "A", "Bb", "B"];
function getScale(scaleName){
let tonic = scaleName.replace("m", "");
let keys = [];
let minor = false;
let scaleIndex;
let intervals = this.majorSemiTones;
if(scaleName.indexOf("m") > -1){
minor = true;
intervals = this.minorSemiTones;
}
scaleIndex = KEYS.index
Solution
Since you tagged this question with
Since there are different keyboard layouts, I wonder if it makes sense to generalize the functionality on creating a keyboard based on standard keyboard configurations?
Your current logic to build keys seems basically hard-coded for standard 88 key keyboard. There is an odd mix of work that is happening before the main loop where keys are populated (setting the octave 0 keys) and after that main loop (setting the octave 8 key). I would strive to set all key values within the loop so this logic isn't disjointed like it currently is.
In both pieces of logic, you might consider "rotating" the array of 12 notes to get to an appropriate starting position for working with the array based on the values input. This could make your array interaction logic much cleaner, by avoiding the need to look for boundary conditions on the array (i.e. when you need to go back to index 0);
You also have some fragility in your code around data input. You really don't have anything here that validates or handles edge cases in input data (for example what if values like
Code like this:
might be better to be made more specific like this:
as there is only one index value where
Alternately, I would consider a regex like
I don't understand the need for both
Some variable names seem confusing. For example:
Putting it all together might yield something like:
```
class Keyboard {
static readonly standardKeyboardConfig = {
49: {
startKey: 'C',
startOctave: 0,
octaveBoundary: 'C'
}
61: {
startKey: 'C',
startOctave: 0,
octaveBoundary: 'C'
},
76: {
startKey: 'E',
startOctave: 0,
octaveBoundary: 'C'
},
88: {
startKey: 'A',
startOctave: 0,
octaveBoundary: 'C'
}
}
static readonly defaultKeyCount = 88;
static readonly keyNotes = ["C", "Db", "D", "Eb", "E", "F",
"Gb", "G", "Ab", "A", "Bb", "B"];
readonly keys: string[];
constructor(keyCount: number) {
keyCount = keyCount || Keyboard.defaultKeyCount;
if (keyCount in Keyboard.standardKeyboardConfig === false) {
throw new RangeError('Invalid keyboard specified');
}
let config = Keyboard.standardKeyboardConfig[keyCount];
let startKey = config.startKey;
// first rotate the array of notes if needed
let keyNotes = Keyboard.keyNotes;
if(startKey !== keyNotes[0]) {
let index = keyNotes.indexOf(startKey);
keyNotes = keyNotes.slice(index, keyNotes.length)
.concat(keyNotes.slice(0, index));
}
// now build out keys with octave values
let octave = config.startOctave;
let octaveLength = keyNotes.length;
let octaveBoundary = config.octaveBoundary;
for (i = 0; i 0 && note === octaveBoundary) {
octave++;
}
this.keys.push(note + octave);
}
}
}
// usage for default keyboard (a piano)
let piano = new Keyboard();
let keys = piano.keys;
class Scale {
static readonly majorSemiToneIntervals = [2,2,1,2,2,2];
static readonly minorSemiToneIntervals = [2,1,2,2,1,2];
static readonly regex = /^([A-G]b?)(m?)$/;
static readonly allNotes = ["A", "Bb", "B", "C", "Db", "D", "Eb",
"E", "F", "Gb", "G", "Ab"];
readonly name: string;
readonly tonic: string;
readonly isMinor: boolean = false;
readonly notes: string[];
constructor (scaleName: string) {
// validate scale name and capture components
let match = Scale.regex.match(scaleName);
if(match === null) {
throw new RangeError('Invalid scale name.')
};
typescript, I would say you could benefit greatly from making your keyboard and scale representations into classes.Since there are different keyboard layouts, I wonder if it makes sense to generalize the functionality on creating a keyboard based on standard keyboard configurations?
Your current logic to build keys seems basically hard-coded for standard 88 key keyboard. There is an odd mix of work that is happening before the main loop where keys are populated (setting the octave 0 keys) and after that main loop (setting the octave 8 key). I would strive to set all key values within the loop so this logic isn't disjointed like it currently is.
In both pieces of logic, you might consider "rotating" the array of 12 notes to get to an appropriate starting position for working with the array based on the values input. This could make your array interaction logic much cleaner, by avoiding the need to look for boundary conditions on the array (i.e. when you need to go back to index 0);
You also have some fragility in your code around data input. You really don't have anything here that validates or handles edge cases in input data (for example what if values like
a, ABm, are passed to getScale()?). Code like this:
if(scaleName.indexOf("m") > -1){might be better to be made more specific like this:
if(scaleName.indexOf("m") === 1) {as there is only one index value where
m should ever be encountered.Alternately, I would consider a regex like
/^([A-G]b?)(m?)$/ to be able to validate the input string and capture the components of the string in one pass.I don't understand the need for both
KEYS and KEYS_NORMALIZED in the scale code. This is just the same array rotated, and considering you will have to rotate or transform the array given the specific input scale, why have two arrays that hold the same values?Some variable names seem confusing. For example:
- In the "piano" code, your
KEYSconstant might better be callednotesorkeyNotesor similar since this array is not the representation of the physical "keys" on the keyboard.
- In
getScale(),keysarray is not dealing with the key of the scale or keys on a keyboard, but rather the notes in the scale, so perhapsnotesmight be a better name.
- In
getScale()themajor/minorSemiTonesvariables might better be namedmajor/minroSemiToneIntervals.
Putting it all together might yield something like:
```
class Keyboard {
static readonly standardKeyboardConfig = {
49: {
startKey: 'C',
startOctave: 0,
octaveBoundary: 'C'
}
61: {
startKey: 'C',
startOctave: 0,
octaveBoundary: 'C'
},
76: {
startKey: 'E',
startOctave: 0,
octaveBoundary: 'C'
},
88: {
startKey: 'A',
startOctave: 0,
octaveBoundary: 'C'
}
}
static readonly defaultKeyCount = 88;
static readonly keyNotes = ["C", "Db", "D", "Eb", "E", "F",
"Gb", "G", "Ab", "A", "Bb", "B"];
readonly keys: string[];
constructor(keyCount: number) {
keyCount = keyCount || Keyboard.defaultKeyCount;
if (keyCount in Keyboard.standardKeyboardConfig === false) {
throw new RangeError('Invalid keyboard specified');
}
let config = Keyboard.standardKeyboardConfig[keyCount];
let startKey = config.startKey;
// first rotate the array of notes if needed
let keyNotes = Keyboard.keyNotes;
if(startKey !== keyNotes[0]) {
let index = keyNotes.indexOf(startKey);
keyNotes = keyNotes.slice(index, keyNotes.length)
.concat(keyNotes.slice(0, index));
}
// now build out keys with octave values
let octave = config.startOctave;
let octaveLength = keyNotes.length;
let octaveBoundary = config.octaveBoundary;
for (i = 0; i 0 && note === octaveBoundary) {
octave++;
}
this.keys.push(note + octave);
}
}
}
// usage for default keyboard (a piano)
let piano = new Keyboard();
let keys = piano.keys;
class Scale {
static readonly majorSemiToneIntervals = [2,2,1,2,2,2];
static readonly minorSemiToneIntervals = [2,1,2,2,1,2];
static readonly regex = /^([A-G]b?)(m?)$/;
static readonly allNotes = ["A", "Bb", "B", "C", "Db", "D", "Eb",
"E", "F", "Gb", "G", "Ab"];
readonly name: string;
readonly tonic: string;
readonly isMinor: boolean = false;
readonly notes: string[];
constructor (scaleName: string) {
// validate scale name and capture components
let match = Scale.regex.match(scaleName);
if(match === null) {
throw new RangeError('Invalid scale name.')
};
Code Snippets
if(scaleName.indexOf("m") > -1){if(scaleName.indexOf("m") === 1) {class Keyboard {
static readonly standardKeyboardConfig = {
49: {
startKey: 'C',
startOctave: 0,
octaveBoundary: 'C'
}
61: {
startKey: 'C',
startOctave: 0,
octaveBoundary: 'C'
},
76: {
startKey: 'E',
startOctave: 0,
octaveBoundary: 'C'
},
88: {
startKey: 'A',
startOctave: 0,
octaveBoundary: 'C'
}
}
static readonly defaultKeyCount = 88;
static readonly keyNotes = ["C", "Db", "D", "Eb", "E", "F",
"Gb", "G", "Ab", "A", "Bb", "B"];
readonly keys: string[];
constructor(keyCount: number) {
keyCount = keyCount || Keyboard.defaultKeyCount;
if (keyCount in Keyboard.standardKeyboardConfig === false) {
throw new RangeError('Invalid keyboard specified');
}
let config = Keyboard.standardKeyboardConfig[keyCount];
let startKey = config.startKey;
// first rotate the array of notes if needed
let keyNotes = Keyboard.keyNotes;
if(startKey !== keyNotes[0]) {
let index = keyNotes.indexOf(startKey);
keyNotes = keyNotes.slice(index, keyNotes.length)
.concat(keyNotes.slice(0, index));
}
// now build out keys with octave values
let octave = config.startOctave;
let octaveLength = keyNotes.length;
let octaveBoundary = config.octaveBoundary;
for (i = 0; i < keyCount; i++) {
let noteIndex = i % octaveLength;
let note = keyNotes[noteIndex];
if (i > 0 && note === octaveBoundary) {
octave++;
}
this.keys.push(note + octave);
}
}
}
// usage for default keyboard (a piano)
let piano = new Keyboard();
let keys = piano.keys;
class Scale {
static readonly majorSemiToneIntervals = [2,2,1,2,2,2];
static readonly minorSemiToneIntervals = [2,1,2,2,1,2];
static readonly regex = /^([A-G]b?)(m?)$/;
static readonly allNotes = ["A", "Bb", "B", "C", "Db", "D", "Eb",
"E", "F", "Gb", "G", "Ab"];
readonly name: string;
readonly tonic: string;
readonly isMinor: boolean = false;
readonly notes: string[];
constructor (scaleName: string) {
// validate scale name and capture components
let match = Scale.regex.match(scaleName);
if(match === null) {
throw new RangeError('Invalid scale name.')
};
this.name = match[0];
this.tonic = match[1];
let intervals = Scale.majorSemiToneIntervals;
if(match[2].length === 1) {
this.isMinor = true;
intervals = Scale.minorSemiToneIntervals;
}
// rotate the allNotes array
let notes = Scale.allNotes;
let index = notes.indexOf(this.toContext
StackExchange Code Review Q#152639, answer score: 3
Revisions (0)
No revisions yet.