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

Minor: exception defined inside internal method #2750

Closed
raph-luc opened this issue Apr 30, 2024 · 1 comment · Fixed by #2752
Closed

Minor: exception defined inside internal method #2750

raph-luc opened this issue Apr 30, 2024 · 1 comment · Fixed by #2752
Assignees
Labels
question Further information is requested refactoring

Comments

@raph-luc
Copy link
Member

Quick question:

def _new_command_for_task(task, session):
class NewCommandError(Exception):
"""Raised on an attempt to create command for a given task."""
def __init__(self, task_name):
super().__init__(f"Could not create command for task {task_name}")
task_cmd_name = task.CommandName()

Isn't this definition for a custom Exception nested inside a non-public method kind of useless, in the sense that users won't easily be able to import and catch this NewCommandError? In which case this 5 lines definition could be replaced with just Exception(f"Could not create command for task {task_name}") as it would be caught using except Exception: regardless?

Otherwise I think this custom exception definition should be moved outside of the non-public method, so that it can more easily be imported if an user wants to catch it.

@raph-luc raph-luc added question Further information is requested refactoring labels Apr 30, 2024
@raph-luc
Copy link
Member Author

As discussed in the meeting earlier today, I will work on this

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested refactoring
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant