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

Remove list comprehension from str.join function calls #7433

Closed
Solomon1732 opened this issue May 22, 2023 · 2 comments · Fixed by #7440
Closed

Remove list comprehension from str.join function calls #7433

Solomon1732 opened this issue May 22, 2023 · 2 comments · Fixed by #7440

Comments

@Solomon1732
Copy link
Contributor

Describe the bug
Thanks to generator expressions and other lazy evaluation functions (such as map), there's no need to build a list when calling str.join. This saves memory during runtime since no list is built. Below each relevant line I wrote a substitute I offer.

return "".join([filter_character(char) for char in name])

                    return "".join(map(filter_character, name))

return "".join([filter_character(char) for char in name])

                            return "".join(map(filter_character, name))

                " ".join(getattr(self.payload, attr) for attr in ("title", "tags", "text") if hasattr(self.payload, attr))

return ''.join([chr(c) for c in ext_peer_info])

        return ''.join(map(chr, ext_peer_info))
  • Tribler's version [main branch]
@drew2a
Copy link
Contributor

drew2a commented May 23, 2023

@Solomon1732 thank you for your contributions to improving Tribler's code quality. If it's not too much trouble, could you kindly convert this issue into a pull request? This way, we could avoid duplication of work, as it would save us the step of implementing the string replacements that you've described. Plus, we could run tests on the modified code to ensure its correctness.
Thank you in advance for your consideration.

@Solomon1732
Copy link
Contributor Author

No problem. I would be glad to 😄

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

Successfully merging a pull request may close this issue.

2 participants