1

While adding Room Database, it is suggested to use Singleton Design Pattern

Note: You should follow the singleton design pattern when instantiating an AppDatabase object, as each RoomDatabase instance is fairly expensive, and you rarely need access to multiple instances.

So, adding Room Database, following Google example which is written in Java, in will be as below

private var INSTANCE: AppDatabase? = null

fun getInstance(context: Context): AppDatabase? {
    if (INSTANCE == null){
        synchronized(AppDatabase::class){
            INSTANCE = Room.databaseBuilder(context.applicationContext,
                    AppDatabase::class.java, "app_database")
                    .build()
        }
    }
    return INSTANCE
}

When I call getInstance, compiler suggests that getInstance can be null. So my question is there any case that getInstance be null and do I have to check if it's null. If not, then how should I instantiate AppDatabase so that getInstance return AppDatabase not AppDatabase? and it fits documentation recommendation?

Jayson Minard
  • 84,842
  • 38
  • 184
  • 227
musooff
  • 6,412
  • 3
  • 36
  • 65

1 Answers1

1

Use something like this to make sure it is not null

object DatabaseSource {
    private lateinit var INSTANCE: AppDatabase

    fun getInstance(context: Context): AppDatabase {
        if (!::INSTANCE.isInitialized) {
            synchronized(AppDatabase::class) {
                if (!::INSTANCE.isInitialized) {
                    INSTANCE = Room.databaseBuilder(context.applicationContext,
                                                    AppDatabase::class.java,
                                                    "app_database").build()
                }
            }
        }
        return INSTANCE
    }
}

Call it with:

val db = DatabaseSource.getInstance(context)

And this will never be null so you no longer have that issue.

I added double locking to be safer for thread safety on the getInstance() call.

But really you should be using dependency injection with singletons to avoid this passed in dependency of context and this manual creation/locking. You'll have to have the context available everywhere which is a bad pattern.

This also acts as an answer to your other post which is apparently a duplicate of this one.

Jayson Minard
  • 84,842
  • 38
  • 184
  • 227
  • That indeed was what I am looking for. But how would I pass the `context` here? Since I am creating INSTANCE in my `companion object`, it's not a good practice to have `context` inside static fields. Looking [google sample](https://github.com/googlesamples/android-architecture-components/blob/master/BasicRxJavaSampleKotlin/app/src/main/java/com/example/android/observability/persistence/UsersDatabase.kt), I see that they use `also` which works perfectly. What do you say? – musooff Nov 20 '18 at 02:16
  • Are you using dependency injection? because this seems like it should be injected as a singleton, and the context should be made available for injection as well. – Jayson Minard Nov 20 '18 at 02:38
  • I went with another direction, to make your original sample more workable. – Jayson Minard Nov 20 '18 at 02:56
  • No I am not injecting as a singleton, but injecting when I call getInstance(context: Context) method. – musooff Nov 20 '18 at 02:57
  • I'm suggesting you can use the new version above but you should later move to dependency injection and not have code like this in your app. Inject context into the AppDatabase building which then is injected into your code as needed. Not this. But that is outside the scope of your question. – Jayson Minard Nov 20 '18 at 03:01
  • I see you point. Your new approach does follow the way I am doing and will also has double locking. However, I read [here] (https://stackoverflow.com/a/40437274/8412192) that I should not use `lateinit` here. – musooff Nov 20 '18 at 03:05
  • 1
    The comment about not using `lateinit` there is not universally a valid comment. It is fine in any context where you KNOW the lifecycle gaurantees it will be instantiated before use. Here the only way to access it is by a method that guarantees it is valid. So you are fine. The other comment is not relevant without this context. – Jayson Minard Nov 20 '18 at 03:07
  • 1
    I saw your proposed edit of volatile, but since this is only modified under lock, and we are checking if assigned which is I believe a bit field that would not be partially set. You can add it if you want, but not sure it is really what you want and will just add more contention on accessing it. – Jayson Minard Nov 20 '18 at 03:11
  • 1
    Let's not add things we see in other posts without reason. – Jayson Minard Nov 20 '18 at 03:12