3
\$\begingroup\$

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
                })
            }
        }
    }
```
\$\endgroup\$
  • 1
    \$\begingroup\$ This is not actual code from the app, but an example - then it risks being off-topic. Please read codereview.stackexchange.com/help/how-to-ask \$\endgroup\$ – Reinderien Jul 6 at 18:09
  • 1
    \$\begingroup\$ Code Review requires concrete code from a project, with enough code and / or context for reviewers to understand how that code is used. Pseudocode, stub code, hypothetical code, obfuscated code, and generic best practices are outside the scope of this site. \$\endgroup\$ – Reinderien Jul 6 at 18:17
  • 1
    \$\begingroup\$ Its a working bit of code that I would like reviewed, so I can improve upon it and understand what I am doing wrong, if anything. Although it is not the exact class name and properties, it is literally how I am currently structuring the flow. How does that not constitute? @Reinderien \$\endgroup\$ – BHogan Jul 6 at 18:43
  • \$\begingroup\$ When we understand what the code is supposed to do, it makes it easier to review the code and give a good code review. When the object and function names have been changed and we don't have a clear idea of what the code is supposed to do, such as why you want to share the json between the 2 classes, then giving a good review is very difficult. \$\endgroup\$ – pacmaninbw Jul 6 at 19:32
  • \$\begingroup\$ I'm not sure how my example isn't clear enough. Its passing an About object from the database to the fragment. It goes from a data layer to the UI. I have each step of the flow in the main question, in order of flow. \$\endgroup\$ – BHogan Jul 6 at 20:08
1
\$\begingroup\$

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.

| improve this answer | |
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.