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

fixed anti-pattern, formating and typo #81

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Joonsey
Copy link

@Joonsey Joonsey commented Sep 12, 2023

tripletex.create_session_token should not have to be null-checked when behaviour will result in an Exception, this is an anti-pattern. Rather raise Exception explicitly.

Fixed parameter typo in tripletex.map.

Formated empty space characters.
and PEP-8 method-spacing formating.

tripletex.create_session_token should not have to be null-checked when behaviour
will result in an Exception, this is an anti-pattern. Rather raise Exception explicitly.

Fixed parameter typo in tripletex.map.

Formated empty space characters.
and PEP-8 method-spacing formating.
@tlxtellef tlxtellef self-requested a review September 12, 2023 07:07
else:
print(r.status_code, r.text, r.reason)

raise Exception(r.status_code, r.text, r.reason)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since this is getting converted in to throwing an exception the exception type it throws should be as accurate as practically possible.
If none of the types here fit then either a new custom exception type should be made by extending Exception or use RuntimeError.

There is a very informative answer on StackOverflow regarding this here

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

Successfully merging this pull request may close these issues.

2 participants