Skip to main content
Code Review

Return to Answer

review of original code
Source Link
dfhwze
dfhwze
  • 14.2k
  • 3
  • 40
  • 101

Review

(1) Algorithm

Since the octave and the note name are completely independant, you could mitigate boiler-plate concatenation and optimize reusability by using a class that stores the information for you. Additional complexity is required to determine the accidentals of a note given its degree and pitch. I have added the algorithm for this in my alternative solution.

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

class Note {
    constructor(degree, pitch) {
        this.degree = degree;
        this.pitch = pitch;
    }
    get Octave() {
        return this.modulo(this.pitch);
    }
    set Octave(n) {
        this.pitch = n * 12 + this.modulo(this.pitch);
    }
    modulo(n) {
        return (n % 12 + 12) % 12;
    }
    ..
}

(2) Algorithm

I like the idea of using interval patterns to identify scales.

majorSemiTones = [2,2,1,2,2,2];
minorSemiTones = [2,1,2,2,1,2];

However, when hardcoding them in the algorithm, there is not much room for extensibility and modularity.

if(scaleName.indexOf("m") > -1){
            minor = true;
            intervals = this.minorSemiTones;
        }

Alternative

Review

(1) Algorithm

Since the octave and the note name are completely independant, you could mitigate boiler-plate concatenation and optimize reusability by using a class that stores the information for you. Additional complexity is required to determine the accidentals of a note given its degree and pitch. I have added the algorithm for this in my alternative solution.

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

class Note {
    constructor(degree, pitch) {
        this.degree = degree;
        this.pitch = pitch;
    }
    get Octave() {
        return this.modulo(this.pitch);
    }
    set Octave(n) {
        this.pitch = n * 12 + this.modulo(this.pitch);
    }
    modulo(n) {
        return (n % 12 + 12) % 12;
    }
    ..
}

(2) Algorithm

I like the idea of using interval patterns to identify scales.

majorSemiTones = [2,2,1,2,2,2];
minorSemiTones = [2,1,2,2,1,2];

However, when hardcoding them in the algorithm, there is not much room for extensibility and modularity.

if(scaleName.indexOf("m") > -1){
            minor = true;
            intervals = this.minorSemiTones;
        }

Alternative

added 804 characters in body
Source Link
dfhwze
dfhwze
  • 14.2k
  • 3
  • 40
  • 101

Fiddle that outputs to the browser consoleFiddle that outputs to the browser console

function onLoad() {
  "use strict";
  
  // C# major scale
  console.log(getScaleByIntervalPattern('C#', [2, 2, 1, 2, 2, 2, 1]));
  // Db major scale
  console.log(getScaleByIntervalPattern('Db', [2, 2, 1, 2, 2, 2, 1]));
  // C# minor scale
  console.log(getScaleByIntervalPattern('C#', [2, 1, 2, 2, 1, 2, 2]));
  // Db minor scale
  console.log(getScaleByIntervalPattern('Db', [2, 1, 2, 2, 1, 2, 2]));
}

results

by scale function

- (7) ["C#5", "D#5", "E#5", "F#5", "G#5", "A#5", "B#6"]
- (7) ["Db5", "Eb5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["C#5", "D#5", "E5", "F#5", "G#5", "A5", "B5"]
- (7) ["Db5", "Eb5", "Fb5", "Gb5", "Ab5", "Bbb5", "Cb5"]

enharmonic

- (7) ["C#5", "D#5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["Db5", "Eb5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["C#5", "D#5", "E5", "F#5", "G#5", "A5", "B5"]
- (7) ["Db5", "Eb5", "E5", "F#5", "G#5", "A5", "B5"]

descending enharmonic (how you'd prefer)

- (7) ["Db5", "Eb5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["Db5", "Eb5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["Db5", "Eb5", "E5", "Gb5", "Ab5", "A5", "B5"]
- (7) ["Db5", "Eb5", "E5", "Gb5", "Ab5", "A5", "B5"]

ascending enharmonic

- (7) ["C#5", "D#5", "F5", "F#5", "G#5", "A#5", "C6"]
- (7) ["C#5", "D#5", "F5", "F#5", "G#5", "A#5", "C6"]
- (7) ["C#5", "D#5", "E5", "F#5", "G#5", "A5", "B5"]
- (7) ["C#5", "D#5", "E5", "F#5", "G#5", "A5", "B5"]

Fiddle that outputs to the browser console

function onLoad() {
  "use strict";
  
  console.log(getScaleByIntervalPattern('C#', [2, 2, 1, 2, 2, 2, 1]));
  console.log(getScaleByIntervalPattern('Db', [2, 2, 1, 2, 2, 2, 1]));
  console.log(getScaleByIntervalPattern('C#', [2, 1, 2, 2, 1, 2, 2]));
  console.log(getScaleByIntervalPattern('Db', [2, 1, 2, 2, 1, 2, 2]));
}

results

- (7) ["C#5", "D#5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["Db5", "Eb5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["C#5", "D#5", "E5", "F#5", "G#5", "A5", "B5"]
- (7) ["Db5", "Eb5", "E5", "F#5", "G#5", "A5", "B5"]

Fiddle that outputs to the browser console

function onLoad() {
  "use strict";
  
  // C# major scale
  console.log(getScaleByIntervalPattern('C#', [2, 2, 1, 2, 2, 2, 1]));
  // Db major scale
  console.log(getScaleByIntervalPattern('Db', [2, 2, 1, 2, 2, 2, 1]));
  // C# minor scale
  console.log(getScaleByIntervalPattern('C#', [2, 1, 2, 2, 1, 2, 2]));
  // Db minor scale
  console.log(getScaleByIntervalPattern('Db', [2, 1, 2, 2, 1, 2, 2]));
}

results

by scale function

- (7) ["C#5", "D#5", "E#5", "F#5", "G#5", "A#5", "B#6"]
- (7) ["Db5", "Eb5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["C#5", "D#5", "E5", "F#5", "G#5", "A5", "B5"]
- (7) ["Db5", "Eb5", "Fb5", "Gb5", "Ab5", "Bbb5", "Cb5"]

enharmonic

- (7) ["C#5", "D#5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["Db5", "Eb5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["C#5", "D#5", "E5", "F#5", "G#5", "A5", "B5"]
- (7) ["Db5", "Eb5", "E5", "F#5", "G#5", "A5", "B5"]

descending enharmonic (how you'd prefer)

- (7) ["Db5", "Eb5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["Db5", "Eb5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["Db5", "Eb5", "E5", "Gb5", "Ab5", "A5", "B5"]
- (7) ["Db5", "Eb5", "E5", "Gb5", "Ab5", "A5", "B5"]

ascending enharmonic

- (7) ["C#5", "D#5", "F5", "F#5", "G#5", "A#5", "C6"]
- (7) ["C#5", "D#5", "F5", "F#5", "G#5", "A#5", "C6"]
- (7) ["C#5", "D#5", "E5", "F#5", "G#5", "A5", "B5"]
- (7) ["C#5", "D#5", "E5", "F#5", "G#5", "A5", "B5"]
Source Link
dfhwze
dfhwze
  • 14.2k
  • 3
  • 40
  • 101

Given your expected outcome:

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"]

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

I have created a little API to get you the scale given the input key and scale interval pattern.

Fiddle that outputs to the browser console

function getScaleByIntervalPattern(key, intervalPattern) {
  "use strict";

  const keyNote = Note.FromName(key);
  var currentNote = keyNote;

  var result = intervalPattern.reduce(function (scale, scaleStep, degree) {
    scale.push(currentNote.enharmonic().Name);
    currentNote = currentNote.clone().transpose(new Interval(1, scaleStep));
    return scale;
  }, []);
  
  return result;
}

This results in the following scales, using enharmonic note names. If you don't want to use the enharmonic names, you should substitute currentNote.enharmonic().Name with currentNote.Name. Or - in your case - if you want only flattened altered notes, use currentNote.descendingEnharmonic().Name.

function onLoad() {
  "use strict";
  
  console.log(getScaleByIntervalPattern('C#', [2, 2, 1, 2, 2, 2, 1]));
  console.log(getScaleByIntervalPattern('Db', [2, 2, 1, 2, 2, 2, 1]));
  console.log(getScaleByIntervalPattern('C#', [2, 1, 2, 2, 1, 2, 2]));
  console.log(getScaleByIntervalPattern('Db', [2, 1, 2, 2, 1, 2, 2]));
}

results

- (7) ["C#5", "D#5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["Db5", "Eb5", "F5", "Gb5", "Ab5", "Bb5", "C6"]
- (7) ["C#5", "D#5", "E5", "F#5", "G#5", "A5", "B5"]
- (7) ["Db5", "Eb5", "E5", "F#5", "G#5", "A5", "B5"]