search

steve1rm / BusbySurvey

Fetches and conducts a user suvery
MIT License
0 stars 0 forks source link

Some issues regarding the ViewModel logic and flow #19

Open luongvo opened 1 month ago

luongvo commented 1 month ago
steve1rm commented 1 month ago
  • All API executions are in the default empty CoroutineContext with viewModelScope.launch. Should we execute them with the io context?

The presentation should be main safe. It should be the method that actually makes the call to the endpoint or cache that should use a Dispatcher. In room, retrofit, or ktor that would be handled internally by their libraries. So in most cases you might not need to switch dispatchers.

steve1rm commented 1 month ago
  • Why do we fetch the "survey list" in the MainViewModel of the MainActivity? It's being fetched and handled in the HomeViewModel of HomeScreen already

I thought that was feature to fetch the survey when the app was starting i.e. splash screen so it uses the same usecase to fetch the survey list. However, only for logged in users. The reason I have added it to the HomeViewModel as that uses the same fetching usecase to fetch the survey, when landing on the homepage after logging in. However, if the user has logged in then it just passes this survey list to the homepage without making a second request. This could also be used for pull to refresh.

steve1rm commented 1 month ago
  • Kotlin Flow is now a popular asynchronous data stream in Kotlin. Can I know why you haven't used Flow yet? All the data flow in the project is being executed and streamed by suspended fun with a lot of boilerplate code. Kotlin Flow and its operation like map, flatMap, merge, catch, etc, is so powerful to optimize the code.

The calls to fetch use suspend for the use case. But handling the UI updates from the viewmodel uses compose state. I have in the passed used StateFlow and I find it easier to use the savedStateHandle with them. However, you can create a Flow from compose state using the snapshotFlow if you want to use those operators. For this project I decided to go with compose state.

steve1rm commented 1 month ago
  • Why do we mix Compose components (UI) in the ViewModel layer (mutableStateOf -> Compose's State)? Do you think it's good for unit testing for ViewModel classes?

If I understand you correctly. You want to avoid having any compose related state free from the viewmodels? However, the unit testing can be done with this implementation as I have written unit test for this viewmodel. However, I used to use StateFlow for my projects. But for simple projects I might prefer to use compose state. However, this could be debatable based on the business requires and developer preference. Some developers might prefer compose state for developing and updating composables. I am not sure if this is the recommended best practice is from google