Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
uliska opened this issue May 4, 2018 · 3 comments
Open

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

uliska opened this issue May 4, 2018 · 3 comments
Assignees

Comments

@uliska
Copy link
Collaborator

uliska commented May 4, 2018

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
Copy link
Collaborator Author

uliska commented May 4, 2018

@uliska
Copy link
Collaborator Author

uliska commented May 4, 2018

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.

uliska added a commit that referenced this issue May 4, 2018
This is a suggestion for an approach to handle the situation described
in #117.
@PeterBjuhr
Copy link
Collaborator

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants