akavel / up

Ultimate Plumber is a tool for writing Linux pipes with instant live preview
Apache License 2.0
8.39k stars 129 forks source link

refactor: replace goto with error handling #29

Closed rhnvrm closed 6 years ago

rhnvrm commented 6 years ago

This makes the code more readable and deterministic as well as clearly will show the priority of execution. We can also add a pflag to set shell.

akavel commented 6 years ago

What do you mean by "deterministic" and "priority of execution" here? I don't understand; can you please try to explain?

rhnvrm commented 6 years ago

Sorry for the vague description and poor choice of words. I meant to outline that since we are not going to be utilizing the label elsewhere it would be difficult to maintain the goto as one would have to search the whole file to find if there is a goto to the label making it a bit more easier to read and determine what the flow is. With the errors we are able to express that we want to prioritize the execution/selection of SHELL and are providing others as a fallback. Tomorrow if we change the order of execution for this selection we have to follow the same pattern.

akavel commented 6 years ago

Thanks for explaining! Your concerns were valid, though after thinking some more, I decided to take a different approach, based on your ideas and comments. I'm not a big fan of increasing indentation depth; exploring some other refactorings, I believe I managed to improve code symmetry somewhat in the end. Thanks again!

rhnvrm commented 6 years ago

Yes, it looks and reads better now. Thanks :+1: