Code Review Asked by BHogan on December 10, 2021
I’m stepping back into Android after being away from it for about a year. Trying to get an out of date app of mine back on its feet, and continuing work on it.
The app was written using Java, MVP, Realm, RxJava, and Dagger. I am trying to update it to use Kotlin, MVVM, Realm, Coroutines and ideally dropping Dagger, as I find it more complicated than I need.
I put together a gist of the flow that I have so far, and would love some feedback on how I can improve, what I could change, or what I am doing wrong. Ideally with examples or direct changes to my code.
It works as is, I am just not sure if I am using coroutines correctly or efficiently, and if there is a better way to structure the DAO’s so that Realm can be injected for better testability. Someone has already mentioned changing the DAO to extend the LiveData<>, and using onActive() and onInactive() for posting the object. Is that a good idea?
// About Model is the model used by Realm. These models contains realm specific types, like RealmList
open class AboutModel(
var name: String = "",
@PrimaryKey
var version: String = ""
): RealmObject() {
/**
* Conversion function, to convert the view model layer object to the data layer object
*/
companion object {
fun from(about: About): AboutModel = AboutModel(about.name, about.version)
}
fun toObject(): About =
About(
this.name,
this.version
)
}
// About class used everywhere outside of the data/realm layer.
// Lines up with the AboutModel class, but free of realm or any other database specific types.
// This way, realm objects are not being referenced anywhere else. In case I ever need to
// replace realm for something else.
class About (val name: String = "Test", val version: String = "1.0.0") {
override fun toString(): String {
return "author is : $name, version is: $version"
}
}
// Couldn't inject the realm instance because its thread would not match with a suspend function.
// Even if both where background threads. Would be better if I could inject it, but couldn't get
// that to work.
class AboutDao() {
private val _about = MutableLiveData<About>()
init {
val realm = Realm.getDefaultInstance()
val aboutModel = realm.where(AboutModel::class.java).findFirst()
_about.postValue(aboutModel?.toObject() ?: About())
realm.close()
}
suspend fun setAbout(about: About) = withContext(Dispatchers.IO) {
val realm: Realm = Realm.getDefaultInstance()
realm.executeTransaction {
realm.copyToRealmOrUpdate(AboutModel.from(about))
_about.postValue(about)
}
realm.close()
}
fun getAbout() = _about as LiveData<About>
}
// Database is a singleton instance, so there is only ever one instance of the DAO classes
class Database private constructor() {
var aboutDao = AboutDao()
private set
companion object {
// @Volatile - Writes to this property are immediately visible to other threads
@Volatile private var instance: Database? = null
suspend fun getInstance() = withContext(Dispatchers.IO) {
return@withContext instance ?: synchronized(this) {
instance ?: Database().also { instance = it }
}
}
}
}
// Repo maintains the dao access. Is also setup to run as a singleton
class AboutRepo private constructor(private val aboutDao: AboutDao){
// This may seem redundant.
// Imagine a code which also updates and checks the backend.
suspend fun set(about: About) {
aboutDao.setAbout(about)
}
suspend fun getAbout() = aboutDao.getAbout()
companion object {
// Singleton instantiation you already know and love
@Volatile private var instance: AboutRepo? = null
fun getInstance(aboutDao: AboutDao) =
instance ?: synchronized(this) {
instance ?: AboutRepo(aboutDao).also { instance = it }
}
}
}
// Injector is used to help keep the injection in a single place for the fragments and activities.
object Injector {
// This will be called from About Fragment
suspend fun provideAboutViewModelFactory(): AboutViewModelFactory = withContext(Dispatchers.Default) {
AboutViewModelFactory(getAboutRepo())
}
private suspend fun getAboutRepo() = withContext(Dispatchers.IO) {
AboutRepo.getInstance(Database.getInstance().aboutDao)
}
}
// AboutViewModel's Factory. I found this code online, as a helper for injecting into the viewModel's factory.
class AboutViewModelFactory (private val aboutRepo: AboutRepo)
: ViewModelProvider.NewInstanceFactory() {
@Suppress("UNCHECKED_CAST")
override fun <T : ViewModel?> create(modelClass: Class<T>): T {
return AboutViewModel(aboutRepo) as T
}
}
// About Fragments ViewModel
class AboutViewModel(private val aboutRepo: AboutRepo) : ViewModel() {
suspend fun getAbout() = aboutRepo.getAbout()
suspend fun setAbout(about: About) = aboutRepo.set(about)
}
// Fragment's onActivityCreated, I set the viewModel and observe the model from the view model for changes
override fun onActivityCreated(savedInstanceState: Bundle?) {
super.onActivityCreated(savedInstanceState)
lifecycleScope.launch {
viewModel = ViewModelProviders.of(
this@AboutFragment,
Injector.provideAboutViewModelFactory()
).get(AboutViewModel::class.java)
withContext(Dispatchers.Main) {
viewModel.getAbout().observe(viewLifecycleOwner, Observer { about ->
version_number.text = about?.version
})
}
}
}
```
I like your architecture decomposition. Especially separation of model and realm object. From what I see it is correct and quite clean.
One thing to consider is to keep using RxXX
to complement LiveData
, where LiveData is lifecycle-aware container of data and Rx
component is something with nice transformation api that you actually subscribe to.
I don't see Dao implementing LiveData as a good idea. Keep it as it is, it's methods returning LiveData.
Answered by K.H. on December 10, 2021
Get help from others!
Recent Questions
Recent Answers
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP