search

frescobaldi / python-ly

A Python package and commandline tool to manipulate LilyPond files
https://pypi.org/project/python-ly
135 stars 33 forks source link

Implementation if cascaded conditionals in musicxml/lymus2musxml.py #117

Open uliska opened 6 years ago

uliska commented 6 years ago

I was just starting to look into Felippe's pull requests (noticing that there are still a number open), and when following the GIthub interface to look into merge conflicts I stumbled over the following:

In (for example) musicxml.lymus2musxml.ParseSource.Command() supported commands are checked in a long if ... elif ... conditional. And any new supported command will have to add one elif level to that list, which seems somewhat inefficient to me.

Wouldn't it be better and more Pythonic to maintain the supported commands in a dictionary with functions as their values? So handling a command would use something like supported_commands.get(input, fallback_function)?

There are a number of such methods, and it seems useful to me to generalize this a bit. I would volunteer to do that (also as a means of finally getting my feet wet with python-ly) but I'd like to have some feedback first.

uliska commented 6 years ago

https://github.com/wbsoft/python-ly/blob/master/ly/musicxml/lymus2musxml.py#L482

uliska commented 6 years ago

Oh, I just realized that I did a major embarrassement to myself by not having pulled the repository - so it still ran on the state before GSoC.

So my comments about the pull requests are wrong, and instead we should go through eoma's Pull Request to see which of them may seem still relevant. But the question about the implementation should still be valid.

PeterBjuhr commented 6 years ago

I agree that a dictionary of functions can sometimes be a more elegant solution than an extensive compilation of elif statements. And these commands can very well be an example of that.