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

Factor out menu choice #154

Open
toonarmycaptain opened this issue Feb 21, 2019 · 2 comments
Open

Factor out menu choice #154

toonarmycaptain opened this issue Feb 21, 2019 · 2 comments
Labels
enhancement New feature or request good first issue Good for newcomers size/M

Comments

@toonarmycaptain
Copy link
Owner

The code for taking user input to choose and run items from menus in main_menu.py and settings_menu.py deploy similar functionality which can be factored out.

take_settings_menu_input was recently refactored here and here not to use a flag to break the loop, matching the implementation in main_menu.py:

possible_options = {
'1': call_set_default_chart_save_location,
# '0': return_to_main_menu,
}
while True:
chosen_option = input('>>> ')
if chosen_option in possible_options:
possible_options[chosen_option]()
break # Exit loop when chosen action finishes. Returns None.
if chosen_option == '0': # User selects to return to main menu.
return True
# else:
print("Invalid input.")
)

To generalise the function I propose reintroducing a flag, returned by the functions corresponding to the options in a menu, that would indicate a return to a higher menu/scope.
This also reintroduces using a function for breaking the loop/menu without action, as originally implemented in settings_menu.py (albeit with stdout user feedback).

This would allow any function to break the take/run menu selection loop with a flag returned to the calling menu logic, which may have a variety of uses - for example returning False if the action was unsuccessful could trigger feedback to the user before displaying the menu again eg Action unsuccessful. Please select another option: {menu options}. These implementations make assumptions about what the called functions return:

Assume only True is a relevant return value:

while True:
    chosen_option = input('>>> ')

    if chosen_option in possible_options:
        if possible_options[chosen_option]():  # or more explicitly: if possible_options[chosen_option]() is True:
            return True
        break  # Exit loop when chosen action finishes. Returns None.
    # else:
    print("Invalid input.")

Or to be able to return True or False flags:
Assumes True and False are relevant return values.

while True:
    chosen_option = input('>>> ')

    if chosen_option in possible_options:
        return_flag = possible_options[chosen_option]()
        if return_flag is not None:
            return return_flag
        break  # Exit loop when chosen action finishes. Returns None.
    # else:
    print("Invalid input.")

Alternatively, assuming functions will return None, True or False - or equivalent values :
A weakness of this approach is that there is a risk of bugs creeping in where functions happen to return an object (eg str, dict) that evaluates to True/False/None that the caller will then interpret as those values - which may be useful, or a source of bugs.

while True:
    chosen_option = input('>>> ')

    if chosen_option in possible_options:
        return possible_options[chosen_option]()
    # else:
    print("Invalid input.")

While the above is very elegant/simple, a more explicit implementation might be preferable:

while True:
    chosen_option = input('>>> ')

    if chosen_option in possible_options:
        return_flag = possible_options[chosen_option]()
        if return_flag is True or return_flag is  False or return_flag is  None:
            return return_flag
        break  # Exit loop when chosen action finishes. Returns None.
    # else:
    print("Invalid input.")
@toonarmycaptain toonarmycaptain added enhancement New feature or request good first issue Good for newcomers Codebase maintenance Refactoring, lifestyle, reorganisation. labels Feb 21, 2019
@toonarmycaptain
Copy link
Owner Author

GitMate.io thinks a possibly related issue is #122 (Factor out UI elements).

@toonarmycaptain toonarmycaptain added size/L size/M good first issue Good for newcomers and removed Codebase maintenance Refactoring, lifestyle, reorganisation. good first issue Good for newcomers size/L labels Feb 21, 2019
@toonarmycaptain
Copy link
Owner Author

In retrospect probably better to leave the menus as is. Use settings_menu as a pattern for other menus, move that general function to UI_functions, and have all menus, aside from the main menu, use a '0' entry to return to upper scope/menu.
Alternatively, run quit_app as a function from main_menu. Have quit_app also run after run_main_menu if you press '0' (although that option is not listed) and break out of main_menu, with an "are you sure you want to quit" wrapper.
Probably fine to call run_main_menu again on 'no I didn't really want to quit' if calling after main_menu and didn't really want to quit. Unless user is particularly silly and does this a bunch of times, there won't be an issue with too many nested calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers size/M
Projects
None yet
Development

No branches or pull requests

1 participant