saebekassebil / teoria

Javascript taught Music Theory
http://saebekassebil.github.io/teoria
MIT License
1.31k stars 114 forks source link

Expose the list of known scales as part of the Scale class #95

Closed emanchado closed 8 years ago

emanchado commented 8 years ago

Proposal for #94. It would maybe be nice to provide "human names" for the scales, too, esp. for cases like "doubleharmonic", "majorpentatonic" and such, but having the scale ids that can be passed to the Scale constructor is a good step forward.

Ideas?

saebekassebil commented 8 years ago

Seems to me a scale should be represented as an object with a set of minimal properties, maybe something like this:

var scale = { 
  id: 'aeolian', 
  name: 'Aeolian', 
  altNames: ['Minor'], 
  intervals: ['P1', 'M2', 'm3', 'P4', 'P5', 'm6', 'm7']
}

then it'd be possible to implement different ways of obtaining such an object, by id, by human name, or whatever?

emanchado commented 8 years ago

Sounds good, that way the "human names" problem would be fixed. A couple of comments:

1) The problem I'm trying to solve is getting a list of all scales so I can show them in some kind of list/dropdown, so it wouldn't be enough with just having a way to look up by id or human name, but there should be an API call or similar to actually return a list.

2) There are currently some scale aliases, like major/ionian. Should there be part of the object? Otherwise, there will be two elements in the list that point to the exact same scale (including that the "name" and "altNames" in both would be the same, instead of each one having a different name).

3) Should be intervals be exposed to the outside, or treated as an implementation detail? It might be useful from the outside, but it feels a bit like an implementation detail.

saebekassebil commented 8 years ago

1) Certainly, we'll need to expose that. Also, the user should be able to implement her own scales, and also to fetch them from different resources.

2) I think major should be an alias of ionian and so forth?

3) Definitely exposed - a scale is basically just an array of intervals decorated with some metadata - so the "raw" data should be accessible

emanchado commented 8 years ago

1) Ah, good idea. Out of curiosity, how would the API be? Simply passing the data in the constructor, or add something like Scale.register({...})?

2) What I meant is that, if one naively converts the current object-based data structure into some array-based new structure, one could end with something like:

[...,
 { 
   id: 'aeolian', 
   name: 'Aeolian', 
   altNames: ['Minor'], 
   intervals: ['P1', 'M2', 'm3', 'P4', 'P5', 'm6', 'm7']
 },
 ...
 { 
   id: 'minor', 
   name: 'Minor', 
   altNames: ['Aeolian'], 
   intervals: ['P1', 'M2', 'm3', 'P4', 'P5', 'm6', 'm7']
 },
 ...
]

Which will end up with duplicates when shown in eg. a dropdown. So what I meant was that, in the same way that you have "altNames" for the human names, there should probably be a list of aliases for the ids. Say, that each scale would be something like:

{ 
  id: 'aeolian',
  aliases: ['minor'],
  name: 'Aeolian',
  altNames: ['Minor'],
  intervals: ['P1', 'M2', 'm3', 'P4', 'P5', 'm6', 'm7']
}

3) Ok :-)

saebekassebil commented 8 years ago

Maybe only .aliases and then it's up to the user to fetch alternative names by iterating through the aliases and fetching the alternative names by its id?

emanchado commented 8 years ago

That could work, but then someone trying to fetch a full list of scales would get duplicates (they would have to manually filter the aliases out). Also, I guess one would have to make sure that all entries for aliases had the corresponding alias back to the "canonical" scale. Or maybe not, I'm not sure.

What about having entries like:

{ id: 'aeolian', name: 'Aeolian', aliases: [{id: 'minor', name: 'Minor'}], intervals: ['P1', 'M2', 'm3', 'P4', 'P5', 'm6', 'm7'] }

and NOT have a separate entry for minor? Then, the function that fetches a scale by id would have to fetch that entry when looking for "minor".

emanchado commented 8 years ago

Actually, I've been thinking about it, and I now think that duplicates are not that bad. In fact, at least Aeolian and Ionian should appear twice I think.

I'll try to make up with a good proposal.

emanchado commented 8 years ago

Can you have a look at the new branch https://github.com/emanchado/teoria/tree/known-scales2? I haven't tested much so I don't know if everything works, but at least the unit tests pass :-)

At least it will illustrate what I have in mind: allowing duplicates when one would like the different names to appear on a list (at least IMO), but using aliases for cases in which we barely want several ways to write the id of a scale.

The KNOWN_SCALES variable is not exported or anything, I just wanted your feedback before I worked too much on it.

emanchado commented 8 years ago

If you want to see the proposal in context, check http://music-explorer.org/. I think it works fairly well, actually: you have the scales you would expect, by the names you would expect, and it's not really annoying to have eg. both "Minor" and "Aeolian".

saebekassebil commented 8 years ago

Hey Esteban, just want to let you know, that I've seen this - and I'll take a look at it as soon as possible :)

emanchado commented 8 years ago

Ping! :-)

saebekassebil commented 8 years ago

Sorry about the waiting time - and thanks for your patience! - I've been so busy and I've barely coded in the holidays.

Merging now - let's discuss further if there are better ways of implementing scales. Thanks for the addition :)

saebekassebil commented 8 years ago

Merged per a12ffa841faf4db9f5b2b729cc77942b14f7e89

emanchado commented 8 years ago

Nice, thanks!

emanchado commented 8 years ago

Ah, I just realised that you merged the initial proposal, simply exposing the scale ids, without human names. That's a pity, as it will probably make it harder in the future to add the human names :-(

But anyway, cool that it's finally merged, let's see how we can improve it!

saebekassebil commented 8 years ago

Woops, you're right. Well I think adding a metadata property "label" or something similar should be straight-forward. Let's start with that and see if we can find a better solution later.

// Jakob

Den 6. jan. 2016 kl. 19.32 skrev Esteban Manchado Velázquez notifications@github.com:

Ah, I just realised that you merged the initial proposal, simply exposing the scale ids, without human names. That's a pity, as it will probably make it harder in the future to add the human names :-(

But anyway, cool that it's finally merged, let's see how we can improve it!

— Reply to this email directly or view it on GitHub.