Skip to content

The binding should not be passed around #57

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

Merged
merged 5 commits into from
Nov 6, 2019

Conversation

sierisimo
Copy link
Contributor

What type of PR is this? (check all applicable)

  • Refactor
  • Feature
  • Bug Fix
  • Documentation Update

Description

The binding generated by the MainActivity should not be passed around as it can generate memory leaks. This also helps to keep things where they belong instead of doing view logic everywhere.

Adding a single lambda instead as the way to communicate makes the class more generic and easy to manage.

Eventually this could help to write better unit tests.

[optional] What gif best describes this PR or how it makes you feel?

alt_text

The binding generated by the MainActivity should not be passed around as it can generate memory leaks. This also helps to keep things where they belong instead of doing view logic everywhere. Eventually this could help to write better unit tests.
joanb
joanb previously approved these changes Oct 18, 2019
…ty.kt

Co-Authored-By: Joan Barroso Garrido <joanbarrosogarrido@gmail.com>
Copy link
Contributor

@VarunBarad VarunBarad left a comment

Choose a reason for hiding this comment

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

Everything looks good, only a small change is required in one file to make it less error-prone and more readable.

@sierisimo
Copy link
Contributor Author

Any help merging @maestromac ?

Copy link
Contributor

@maestromac maestromac left a comment

Choose a reason for hiding this comment

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

Sorry for the long delay @sierisimo 🙇

LGTM!

@maestromac maestromac merged commit 26d91ec into forem:master Nov 6, 2019
@sierisimo sierisimo deleted the remove_dependency_on_binding branch November 7, 2019 05:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants