Skip to content
This repository has been archived by the owner on Feb 5, 2022. It is now read-only.

Raise an exception instead of asserting if a bad argument is given #17

Closed
jmp opened this issue Apr 25, 2020 · 1 comment
Closed

Raise an exception instead of asserting if a bad argument is given #17

jmp opened this issue Apr 25, 2020 · 1 comment
Assignees
Labels
code Code issues

Comments

@jmp
Copy link
Contributor

jmp commented Apr 25, 2020

Currently Settings.__init__ uses assert to make sure value or type_hint are not given as arguments:

def __init__(self, **kwargs):
    assert (
        'value' not in kwargs
    ), '"value" argument should not be passed to Settings.__init__()'
    assert (
        'type_hint' not in kwargs
    ), '"type_hint" argument should not be passed to Settings.__init__()'

I think it would be better to explicitly raise an exception (either ValueError or better, some more specific one) if illegal arguments are passed.

I like how assert is used generously in the library as a defensive programming technique, but it's better not to use it in the public API. Assertions are stripped when running with higher optimization levels (e.g. -O or -OO), so, if the user passes some illegal arguments, no exception may be thrown at the point where the check was supposed to be made, making it harder to debug what went wrong.

@BasicWolf
Copy link
Owner

@jmp thank you very much for reporting the issue. Yes, the initial idea was that assertions are there for development time only. I think you are right, it should always be there, no matter the optimization level and should be an explicit exception.

@BasicWolf BasicWolf self-assigned this May 3, 2020
@BasicWolf BasicWolf added the code Code issues label May 3, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
code Code issues
Projects
None yet
Development

No branches or pull requests

2 participants