Skip to content
This repository has been archived by the owner on Jun 20, 2023. It is now read-only.

Simplify Fragments view unbinding code #927

Closed
3 tasks done
henriquenfaria opened this issue Jul 25, 2020 · 4 comments · Fixed by #928
Closed
3 tasks done

Simplify Fragments view unbinding code #927

henriquenfaria opened this issue Jul 25, 2020 · 4 comments · Fixed by #928
Assignees
Labels
enhancement Improvement of an existing feature mirrored-to-jira This item is also tracked internally in JIRA
Milestone

Comments

@henriquenfaria
Copy link
Contributor

henriquenfaria commented Jul 25, 2020

Avoid duplicates

  • This enhancement request has not already been raised before
  • Enhancement request is specific for Android only, for general issues / questions that apply to iOS and Android please raise them in CWA-Wishlist
  • If you are proposing a new feature, please do so in CWA-Wishlist

Current Implementation

The View Binding usage inside the app's Fragments is a bit complicated and repetitive.

  1. duplicated binding properties to be able to get rid of the null checks:
private var _binding: FragmentMainBinding? = null
private val binding: FragmentMainBinding get() = _binding!!
  1. duplicated code, every Fragment override onDestroyView in order to unbind the view:
override fun onDestroyView() {	
     super.onDestroyView()	
    _binding = null	
}

Suggested Enhancement

My suggestion is to create a lifecycle-aware delegated property that automatically unbinds the view during onDestroyView. As a nice side-effect, we would be able to remove the _binding property.

The implementation would be similar to https://proandroiddev.com/make-android-view-binding-great-with-kotlin-b71dd9c87719

Expected Benefits

  • The code would become simpler and cleaner.
  • Less duplicated code.
  • Less chance of Fragment memory leaks caused by missing _binding = null statements.

Internal Tracking ID: EXPOSUREAPP-1926

@henriquenfaria henriquenfaria added the enhancement Improvement of an existing feature label Jul 25, 2020
@henriquenfaria
Copy link
Contributor Author

Pull Request: #928

@henriquenfaria
Copy link
Contributor Author

Hi @pwoessner
Any updates from your side regarding this issue/pr?

@svengabr svengabr assigned harambasicluka and unassigned pwoessner Aug 31, 2020
@svengabr
Copy link
Member

Hey @henriquenfaria sorry for the late response here. We are currently working hard on finalizing the next release. I have re-assigned the issue to make sure it gets in the hand of the right people soon.

Best regards,
SG

Corona-Warn-App Open Source Team

@henriquenfaria
Copy link
Contributor Author

No problem @svengabr :)
Let me know if you need my help to rebase/update my PR.

d4rken added a commit that referenced this issue Sep 14, 2020
closes #927) (#928)

* Create delegated property to automatically handle Fragment view unbinding

* Add new file at the end of file

* Remove empty line

* Update error message

* Add missing imports for other flavors.

Co-authored-by: darken <darken@darken.eu>
Co-authored-by: Matthias Urhahn <matthias.urhahn@sap.com>
@d4rken d4rken changed the title Simplify Fragments view unbinding code Simplify Fragments view unbinding code (EXPOSUREAPP-1926) Sep 14, 2020
@d4rken d4rken added the 1.4.0 label Sep 14, 2020
@d4rken d4rken changed the title Simplify Fragments view unbinding code (EXPOSUREAPP-1926) Simplify Fragments view unbinding code Sep 14, 2020
@d4rken d4rken added this to the 1.4.0 milestone Oct 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement Improvement of an existing feature mirrored-to-jira This item is also tracked internally in JIRA
Projects
None yet
6 participants