Open michaelbukachi opened 4 years ago
I can work on this
Feel free to work on it
@wangerekaharun is it ok if I move all the Firebase Remote Config code from EventsFragment into a new repository. This will make it a lot easier to test the fragment
Yes its okay to do so. Do the same for all Remote Config code.. We were supposed to do so but you can take the task.
Alright
Also @wangerekaharun can I ask what the reason is for having the getter methods for the livedata in the viewmodels. Are they really necessary?
More of separation of concerns. Having one method doing the fetch and the other one doing the get response from the livedata. This also helps in orientation changes. Assuming you only have one method for fetching and updating the livedata, it will be called every time we have orientation change meaning livedata will be updated again. In the current approach, since the fetch methods are called when needed, the livedata will still have their values even on orientation change
If instead of using the livedata getters, we accessed the livedata directly (like this: myViewModel.myLiveData
) would it make a difference.
Why would you want it to be so?
I feel like the getters are not necessary. The livedata properties could just be made public and accessed directly.
@michaelbukachi what's your take on this?
I usually make live data public. It's just easier since it reduces the amount of code written.
@johnGachihi you can go ahead and make the changes
@johnGachihi you can go ahead and make the changes
@wangerekaharun I've found that it's good practice to use MutableLiveData
in viewmodels but expose the immutable LiveData
to Activities and Fragments like you had done.
But I feel like this example is more Kotlin-like than using getter methods
Yes we already had agreed on this as @michaelbukachi had pointed out
This is a sub issue for #53