This repository was archived by the owner on Jun 20, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 487
Introduce ViewModel Injection #1151
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
# Conflicts: # Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/TestRiskLevelCalculationFragment.kt
# Conflicts: # Corona-Warn-App/src/test/java/de/rki/coronawarnapp/transaction/RetrieveDiagnosisKeysTransactionTest.kt
…o feature/viewmodel-injection
# Conflicts: # Corona-Warn-App/build.gradle # Corona-Warn-App/src/deviceForTesters/java/de/rki/coronawarnapp/test/TestRiskLevelCalculationFragment.kt
Contributor
jakobmoellerdev
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor Remarks in the code. Overall I think we should evaluate whether the gain of using pure constructor-based DI is worth the overhead instead of using lateinit vars.
Waiting for more reviews to give approval 👍
Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/ui/LiveDataExtensions.kt
Show resolved
Hide resolved
Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/viewmodel/CWAViewModelSource.kt
Outdated
Show resolved
Hide resolved
Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/viewmodel/CWAViewModelSource.kt
Show resolved
Hide resolved
Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/ui/SingleLiveEvent.kt
Show resolved
Hide resolved
Corona-Warn-App/src/main/java/de/rki/coronawarnapp/util/di/AppInjector.kt
Show resolved
Hide resolved
.../java/de/rki/coronawarnapp/test/risklevel/ui/TestRiskLevelCalculationFragmentCWAViewModel.kt
Show resolved
Hide resolved
|
Kudos, SonarCloud Quality Gate passed!
|
jakobmoellerdev
approved these changes
Sep 17, 2020
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Labels
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds the boilerplate necessary for combining ViewModels and dependency injection. As guide/demo the
TestRiskLevelCalculationFragmenthas been refactored.Our injected "ViewModels" are called
VDC(View Data Controller/Component) because "ViewModel" is a confusing name and overloading the existingviewModelsextensions would lead to confusion.Using a
VDCrequires 3 things:In a dagger
Modulewe need to add aVDC.Factoryto a lookup map:In the
Fragmentwe need to instantiate itvdcSourceis a factory that has access to the lookup map we created in step one.vdcsAssistedis a little indirection that allows us to have an injected class with manual arguments, meaning we can have both injected parameters from our DI graph, and manual parameters, such as fragment/nav graph arguments.Finally we need the
VDC(ViewModel) itself:AssistedInject,andAssisted. Further details: https://github.com/square/AssistedInjectSavedStateHandleis a framework class that offers behavior similar toonSavedInstance.Now use the vdc/viewmodel in your fragment by moving logic into it, and keeping UI updates out of it, i.e.
How to test