3
\$\begingroup\$

I have written a dummy code to check if an app was launched for the time using DataStore. It is working however i want to know if this can be optimised and i also observed a small glitch initially like black screen for like a few seconds only for the first installation. I would like to know why thats being caused as well.

MainActivity


class MainActivity : ComponentActivity() {
    private lateinit var dataStore: FirstTimeLaunch
    private lateinit var viewModel : FirstTimeViewModel

    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        viewModel = ViewModelProvider(this)[FirstTimeViewModel::class.java]
        dataStore = FirstTimeLaunch(this)
        setContent { 
            MyApp(viewModel = viewModel)
        }

    }

    @Composable
    private fun MyApp(viewModel: FirstTimeViewModel) {
        val isFirstTime by viewModel.getFirstTime.observeAsState(initial = true)

        Surface(color = MaterialTheme.colorScheme.onPrimary) {
            Column(
                modifier = Modifier.fillMaxSize(),
                horizontalAlignment = Alignment.CenterHorizontally,
                verticalArrangement = Arrangement.Center
            ) {
                if (isFirstTime) {
                    FirstTimeContent(viewModel)
                } else {
                    NonFirstTimeContent()
                }
            }
        }
    }

    @Composable
    private fun FirstTimeContent(viewModel: FirstTimeViewModel) {
        Column(
            horizontalAlignment = Alignment.CenterHorizontally
        ) {
            Text(text = "First Time Launch")
            Button(onClick = { viewModel.setFirstTime(false) }) {
                Text(text = "Mark as Not First Time")
            }
        }
    }

    @Composable
    private fun NonFirstTimeContent() {
        Text(text = "Non First Time Launch")
    }

FirstTimeViewModel

class FirstTimeViewModel(application: Application) : AndroidViewModel(application) {

    val dataStore = FirstTimeLaunch(application)
    val getFirstTime = dataStore.getFirstTimeLaunch().asLiveData(Dispatchers.IO)

    fun setFirstTime(isFirstTime: Boolean){
        viewModelScope.launch {
            dataStore.setFirstTime(isFirstTime)
        }
    }
}

FirstTimeLaunch

class FirstTimeLaunch (context : Context){

    private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(name = "FirstTimeLaunch")
    private val dataStore = context.dataStore

    companion object{
        val IS_FIRST_TIME_LAUNCH_KEY = booleanPreferencesKey("isFirsTime")
    }

    suspend fun setFirstTime(isFirstTime : Boolean){
        dataStore.edit {
            preferences -> preferences[IS_FIRST_TIME_LAUNCH_KEY] = isFirstTime
        }
    }

    fun getFirstTimeLaunch(): Flow<Boolean> {
        return dataStore.data.map {
            pref -> pref[IS_FIRST_TIME_LAUNCH_KEY]?: true
        }
    }

}

```
\$\endgroup\$
2

1 Answer 1

2
\$\begingroup\$

This already looks quite good.

Apart from some minor formatting issues I think the following things can be improved:

  1. In the MainActivity the property dataStore can be removed. It isn't used anywhere.

  2. When you only pass the app context to the view model's constructor you don't need to call ViewModelProvider, just use viewModel() from your compose code to obtain an instance like this:

    MyApp(viewModel = viewModel()) 
    
  3. The data store is like a small database and can store any number of key/value pairs. You should rename it to something more generic like SettingsDataStoreManager. It may only provide a single setting right now, but its nature is to potentially provide many settings.

  4. The creation of the dataStore object should not be part of the class definition itself, it should be moved to the file level as described in the documentation. You should also rename the data store name from FirstTimeLaunch to settings.

  5. Upper case is reserved for (enums and) constants that are actually declared with const. IS_FIRST_TIME_LAUNCH_KEY should therefore be renamed to isFirstTimeLaunchKey. Also make it private, it should only be accessible by the datastore itself. In addition, the companion object where you defined the key should be the last member of the class as defined in the Kotlin Coding Conventions.

  6. There must only be one instance of any given the data store. Currently only FirstTimeViewModel uses it, but this may change in the future, especially if you add more settings. The easiest way to make sure there is only one instance is to annotate it with @Singleton and let a dependency injection framework like Hilt handle the rest. You won't create objects yourself anymore, your classes will have everything they need automatically passed by their constructor. This also makes view models with multiple parameters much more easy to use.

  7. In Kotlin you usually don't use method names starting with get or set just to indicate the kind of property access. getFirstTimeLaunch in your data store should be renamed to isFirstTimeLaunch which corresponds better to its return value.

  8. The dataStore property in the view model should be made private, it shoudn't be accessible from outside of the class.

  9. The data store already returns a flow for getFirstTimeLaunch, but you decided to switch it to a LiveData in your view model. LiveData shouldn't be used anymore when you use Compose for your UI. Everything should either be a Flow or a MutableState. View models in general should just transform flows when the content needs to be updated, but when exposed (as in "made public" by a property) they should be converted into a StateFlow. Your view model's getFirstTime property (now also isFirstTime) should be defined like this:

    val isFirstTime: StateFlow<Boolean> = dataStore.isFirstTimeLaunch()
        .stateIn(
            scope = viewModelScope,
            started = SharingStarted.WhileSubscribed(5_000),
            initialValue = true,
        )
    

    To access this in your composable, you would now replace this:

    val isFirstTime by viewModel.getFirstTime.observeAsState(initial = true)
    

    by this:

    val isFirstTime by viewModel.isFirstTime.collectAsStateWithLifecycle()
    

    You need the gradle dependency androidx.lifecycle:lifecycle-runtime-compose for this.

  10. As you can see the initial value true moved to the view model. This default value will be used until the data store provides its first value (which might take a moment because it needs to access the file system). So even when the button was clicked and isFirstTime is set to false, on app startup you will for a very brief moment still see the FirstTimeContent instead of NonFirstTimeContent. If this isn't what you want you can change the default value to whatever you want so the compose code can decide what to display. You can even set it to null and, in compose, display a CircularProgressIndicator() or something else for this case to indicate a loading state.

  11. The view model provides the possibility with setFirstTime to set the value to true and false. This probably should only allow it to be set to false, though, because when the first time passed it will never come back. The method should then be changed to this:

    fun onFirstTime() {
        viewModelScope.launch {
            dataStore.setFirstTime(false)
        }
    }
    
  12. In your composables you shouldn't pass view model instances around. Only pass values and callbacks to a composable (also no Flows or States, which you don't do, but just to be clear; only their values should be passed). FirstTimeContent should look like this:

    private fun FirstTimeContent(onFirstTime: () -> Unit) {
        // ...
        Button(onClick = onFirstTime) {
        // ...
    }
    

    MyApp can then call it like this:

    FirstTimeContent(viewModel::onFirstTime) 
    

Put together (without the dependency injection recommendation in 6.) it will look like this:

MainActivity.kt

// ...

class MainActivity : ComponentActivity() {
    override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        setContent {
            MyApp(viewModel = viewModel())
        }
    }
}

@Composable
private fun MyApp(viewModel: FirstTimeViewModel) {
    val isFirstTime by viewModel.isFirstTime.collectAsStateWithLifecycle()

    Surface(color = MaterialTheme.colorScheme.onPrimary) {
        Column(
            modifier = Modifier.fillMaxSize(),
            horizontalAlignment = Alignment.CenterHorizontally,
            verticalArrangement = Arrangement.Center,
        ) {
            if (isFirstTime) {
                FirstTimeContent(viewModel::onFirstTime)
            } else {
                NonFirstTimeContent()
            }
        }
    }
}

@Composable
private fun FirstTimeContent(onFirstTime: () -> Unit) {
    Column(
        horizontalAlignment = Alignment.CenterHorizontally,
    ) {
        Text(text = "First Time Launch")
        Button(onClick = onFirstTime) {
            Text(text = "Mark as Not First Time")
        }
    }
}

@Composable
private fun NonFirstTimeContent() {
    Text(text = "Non First Time Launch")
}

FirstTimeViewModel

class FirstTimeViewModel(application: Application) : AndroidViewModel(application) {
    private val dataStore = SettingsDataStoreManager(application)

    val isFirstTime: StateFlow<Boolean> = dataStore.isFirstTimeLaunch()
        .stateIn(
            scope = viewModelScope,
            started = SharingStarted.WhileSubscribed(5_000),
            initialValue = true,
        )

    fun onFirstTime() {
        viewModelScope.launch {
            dataStore.setFirstTime(false)
        }
    }
}

SettingsDataStoreManager.kt

// ...

private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(name = "settings")

class SettingsDataStoreManager(context: Context) {
    private val dataStore = context.dataStore

    suspend fun setFirstTime(isFirstTime: Boolean) {
        dataStore.edit { preferences ->
            preferences[isFirstTimeLaunchKey] = isFirstTime
        }
    }

    fun isFirstTimeLaunch(): Flow<Boolean> =
        dataStore.data.map { pref ->
            pref[isFirstTimeLaunchKey] ?: true
        }

    companion object {
        private val isFirstTimeLaunchKey = booleanPreferencesKey("isFirstTime")
    }
}
\$\endgroup\$
6
  • \$\begingroup\$ Yikes i missed it sorry , but can you please give me a code example of point 10. And that initial glitch while i install the app (a black screen for like 1 second still exist.. idk why ) \$\endgroup\$
    – dev4Life
    Commented Apr 28 at 15:57
  • \$\begingroup\$ you mean like this in mainactivity for point 10 when { isFirstTime == null -> { // Loading state CircularProgressIndicator() } isFirstTime == true -> { // First time launch FirstTimeContent(viewModel::onFirstTime) } else -> { // Non-first time launch NonFirstTimeContent() } } \$\endgroup\$
    – dev4Life
    Commented Apr 28 at 16:06
  • \$\begingroup\$ I'm not sure what is still unclear. The solution I suggested revolved around replacing the default value true with null. Go from there, fix all compile errors that pop up because the flow's content is now nullable and display a CircularProgressIndicator() in compose for this third state (with true and false being the other two). If you still have problems with that you can ask another question on StackOverflow, this site only focuses on code reviews, not how additional features can be implemented. \$\endgroup\$
    – Leviathan
    Commented Apr 28 at 16:08
  • \$\begingroup\$ Yes, this is pretty much how the compose code would look like when you changed the default value of the StateFlow. \$\endgroup\$
    – Leviathan
    Commented Apr 28 at 16:09
  • \$\begingroup\$ As for the black screen: I cannot reproduce that, and it wouldn't even be on-topic on this site, the code you provide here is expected to work as intended. You should ask another question on StackOverflow for that (be sure to reduce your code to a minimal reproducible example, don't use this code since it doesn't reproduce the problem and contains way too much irrelevant code). \$\endgroup\$
    – Leviathan
    Commented Apr 28 at 16:10

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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