Networking For GameDetails Page and ViewModel#28
Conversation
zachseidner1
left a comment
There was a problem hiding this comment.
Emil, this is great work for your first networking PR. The logic makes sense and is easy to follow. This is a small PR too, so easier for me to review. I think the code inside the try catch could be a bit cleaner. Generally try catch blocks should be as short as possible. So if you clean it up with extension functions, this should be good!
| val sport: String?, | ||
| val state: String?, | ||
| val time: String?, | ||
| val scoreBreakdown: List<List<String?>?>?, |
There was a problem hiding this comment.
omg this type is heinous with all the nulls 😭
I think we should ask backend if it's possible to clean this up a little. Please tell them in the backend Slack that many of the fields are nullable, and ask if it is possible to be cleaned up. It might not be though.
There was a problem hiding this comment.
Nit: I always prefer full length variable names. We have auto complete so they don't slow us down when typing, and they're just more readable. So perhaps change this to currentGameFlow. Or better yet, gameByIdFlow.
There was a problem hiding this comment.
We don't need this comment here anymore, I think it's fine to create custom data classes like this because it creates a layer of abstraction between our Apollo types and the data that our app handles.
There was a problem hiding this comment.
Logically this makes sense but there's a way that you can clean it up with extension functions. Let's create a new file GameByIdQueryMappers that help us map Apollo types to our custom data classes. Then we can add each of these as extension functions. Here's an example:
fun GameByIdQuery.Game.toGameDetails(): GameDetailsGame {
TODO("Your mapping code here")
}You can also create one of these for the GameDetailsTeam and GameDetailsBoxScore. Let me know if that makes sense.
Then at the end in the repository your code could be simplified to something like:
val response = apolloClient.query(GameByIdQuery(id)).execute()
val game = response.data?.game?.toGameDetails()
if (game == null) {
// TODO handle null case and return
}
game?.let {
_currGameFlow.value = ApiResponse.Success(it)
}| val response = apolloClient.query(GameByIdQuery(id)).execute() | ||
| val game = response.data?.game | ||
|
|
||
| if (game != null) { |
There was a problem hiding this comment.
To know if there's an error, I think you should use the toResult function that I provided Amy. This will also make us more consistent. Ask her about this.
There was a problem hiding this comment.
I have asked Amy, and I am waiting for her response!
|
Merged into new PR |
I have added dataclasses for getting the information from the backed. I also added the flow and query to the repository.
I tested this with the dubugger. I basically just hardcoded some .getGameById and then looked at the flow through the debugger.