-
Notifications
You must be signed in to change notification settings - Fork 82
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
Shorter, possibly more efficient code #41
Conversation
adbar
commented
Oct 15, 2021
- Unnecessary lines removed
- Possibly faster code through pre-compiled regex and list comprehension
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I added some notes. Please take a look :)
justext/utils.py
Outdated
if "\n" in text or "\r" in text: | ||
return "\n" | ||
else: | ||
return " " | ||
return " " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I know this may be seen as unnecessary, but I like to see it visually clear these are 2 distinctive branches rather than some filtering by early return followed by an algorithm. Maybe a single line would be better.
if "\n" in text or "\r" in text: | |
return "\n" | |
else: | |
return " " | |
return " " | |
return "\n" if "\n" in text or "\r" in text else " " |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's done ✔️
Co-authored-by: Mišo Belica <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you :)