Conversation
Added a main view model that implements aspects of the movie service. I also added kotlinx dependencies to the gradle.
|
I went back and made some different changes. I added a main view model that implements your movie service. I also added some kotlinx dependencies to your gradle to hopefully fix a few error in other parts of your project. |
|
In all your project had a few dependency errors that came up with the latest commit. The original project a little before that commit worked well with no errors but lacked many of the recent additions. The code was easy to follow with a few good comments to help figure out what was happening. There was a lack of tests to test the different parts of the code. I think you guys are off to a good start though. |
|
You guys are also quick on your commits as one was made as I was working on this review today. |
| //Adding Kotlinx | ||
| testImplementation 'org.jetbrains.kotlinx:kotlinx-coroutines-test:1.6.0' | ||
| testImplementation "io.mockk:mockk:1.12.2" |
There was a problem hiding this comment.
This looks like an incomplete feature.
Are you using these anywhere? I don't see them used.
If not in use, remove them. Unused code is clutter, and bad documentation, which increases technical debt.
Only complete features, which meet the team's Definition of Done, should be merged to Main/Master.
| package com.example.mymovieinfo | ||
|
|
||
| import androidx.lifecycle.MutableLiveData | ||
| import androidx.lifecycle.ViewModel | ||
| import androidx.lifecycle.viewModelScope | ||
| import com.example.mymovieinfo.dto.Movie | ||
| import com.example.mymovieinfo.service.MovieService | ||
| import kotlinx.coroutines.launch | ||
|
|
||
| class MainViewModel : ViewModel() { | ||
| var movies : MutableLiveData<List<Movie>> = MutableLiveData<List<Movie>>() | ||
| var movieService : MovieService = MovieService() | ||
| fun fetchCountries() { | ||
| viewModelScope.launch { | ||
| var innerMovies = movieService.fetchMovies() | ||
| movies.postValue(innerMovies) | ||
| } | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
This looks like an incomplete feature.
Are you using these anywhere? I don't see them used.
If not in use, remove them. Unused code is clutter, and bad documentation, which increases technical debt.
Only complete features, which meet the team's Definition of Done, should be merged to Main/Master.
|
This branch introduces new, and possibly incomplete, functionality, instead of reducing technical debt of existing functionality. New functionality should be kept in a separate branch until it is complete and meets the team's Definition of Done. Thus, if the team would like these features, they can keep the branch open and continue to work on it, and then merge once it meets the Definition of Done. Otherwise, if they do not want these changes, they can abandon/delete this branch and not merge it. |
2 participants
Added a main view model that implements aspects of the movie service. I also added kotlinx dependencies to the gradle.