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

Using method reference for better memorization. #739

Merged

Conversation

eneim
Copy link
Contributor

@eneim eneim commented Sep 24, 2022

Issue

  • Attempt to close 526, but I think this PR is not enough.

Overview (Required)

  • AS-IS: callback using viewModel.method are not effectively remembered, because viewModel is not a stable type.
  • TO-BE: using method reference instead because method reference capture the viewModel reference when it (method referenc) is created, so the value of viewModel doesn't affect the stabability of method reference. In another word, it is stable.

Links

Screenshot

Before After
droidkaigi_drawer_session_lambda.1080p.mov
droidkaigi_drawer_session_method_reference.1080p.mov

@github-actions github-actions bot temporarily deployed to deploygate-distribution September 24, 2022 14:30 Inactive
onClickDrawerItem = { drawerItem ->
kaigiAppScaffoldState.navigate(drawerItem)
}
onClickDrawerItem = kaigiAppScaffoldState::navigate,
Copy link
Member

Choose a reason for hiding this comment

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

May I ask if we can fix the problem by adding Immutable to KaigiAppScaffoldState? 👀

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Since KaigiAppScaffoldState as a DrawerState which is Stable, should we use Stable for the KaigiAppScaffoldState instead of Immutable?

Copy link
Member

Choose a reason for hiding this comment

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

I think so too!
But I think we should have both of codes, method reference and stable. So now, I merge this PR. Thanks

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should have both of codes, method reference and stable

@takahirom Agree. Thanks for merging. If you need any follow-up please assign me to a ticket.

@takahirom takahirom merged commit 22c5d28 into DroidKaigi:main Sep 25, 2022
@eneim eneim deleted the drawer_callback_as_method_reference_fix_526 branch September 27, 2022 02:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants