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

Use of lambda functions when connecting to Qt signals #131

Open
gabrieljreed opened this issue May 4, 2024 · 2 comments
Open

Use of lambda functions when connecting to Qt signals #131

gabrieljreed opened this issue May 4, 2024 · 2 comments
Labels
enhancement New feature or request implementation suggestion A suggestion for how a feature could be implemented refactor Code that needs to be restructured or cleaned up

Comments

@gabrieljreed
Copy link
Contributor

gabrieljreed commented May 4, 2024

I've noticed that across the codebase, signals are connected using lambda functions. While they certainly work, and can seem like a good solution for properly connecting signals to functions (especially with arguments), they do have some downsides.

Memory safety

Lambda functions can cause QWidgets to not be properly garbage collected. (See this forum post and this StackOverflow thread). This can lead to some unexpected behavior and/or the application consuming more resources than expected.

Code cleanliness/readability

While it's true that

self.my_button.clicked.connect(lambda: self.my_button_pressed())

does work,

self.my_button.clicked.connect(self.my_button_pressed)

is functionally equivalent, along with being cleaner and easier to read. Additionally, while lambda functions can be extremely useful in the right situations, they can over-complicate what would otherwise be a simple operation.

Fix

Simple cases can just drop the lambda and actual function call in favor of a reference to the function. This is actually the intended way to use signals (see this example from the Qt for Python docs).

Example:

# Original
open_library_action.triggered.connect(lambda: self.open_library_from_dialog())

# Updated
open_library_action.triggered.connect(self.open_library_from_dialog)

More complicated cases (ex: a function needs to be called with specific arguments) can be replaced with functools.partial, which is part of the Python standard library. partial creates a "partial" function call, with pre-filled arguments.

Example:

# Original
self.edit_modal.saved.connect(lambda: self.edit_tag_callback(btp))

# Updated
from functools import partial

self.edit_modal.saved.connect(partial(self.edit_tag_callback, btp))
@gabrieljreed
Copy link
Contributor Author

I can start updating this, I just wanted to do my due diligence and submit an issue first.

@Loran425 Loran425 added enhancement New feature or request implementation suggestion A suggestion for how a feature could be implemented refactor Code that needs to be restructured or cleaned up labels May 4, 2024
@Loran425
Copy link
Collaborator

Loran425 commented May 4, 2024

This seems like a solid improvement.
Great write up to explain the issue and the fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request implementation suggestion A suggestion for how a feature could be implemented refactor Code that needs to be restructured or cleaned up
Projects
None yet
Development

No branches or pull requests

2 participants