Closed minhnimble closed 7 months ago
These days migrating to jetpack compose is a popular approach and it has advantages over using activity/fragment such as app size, build time. But it is still new and constantly developing solution and not all libraries and components are not compatible with compose.
On the other hand, I am still learning stage for the compose and I only build some small apps for testing so for the test I want to make it perfect. That is another reason I didn’t use compose for this code challenge.
But if I were requested to implement the application using compose, I am confident to deal with the request because I have beginner knowledge about the jetpack compose and through my development experience, I have to handle many changes in technology so I will be fine with this change, too. And also these days, I am in learning phrase so I can prioritize my time in learning jetcompose to be ready to apply.
For the error handling, I am sorry that I left it to do later and forgot about it in the previous submission. But now, in pull request #23 I handle the errors and the app will display a Toast message with appropriate information to the user when the APIs throw an exception.
Yes, wrapping const with Object is old-fashioned with Java-style. We can safely drop both class, object and companion object and use top-level vals in Kotlin. But I used an Object because I want to separate all the api routes and keep those together in one place.
I used Moshi because it is a small and light library compared to most of the other libraries. I manually handle the attribute because sometimes we want to change the variable name according to how we used it so I don’t want to be concerned with naming so I handle it like this.
Thank you for your answers, just one small additional concern on the JSON:API parsing topic as follow before we wrap up this ticket 🙏
I manually handle the attribute because sometimes we want to change the variable name according to how we used it so I don’t want to be concerned with naming so I handle it like this.
Have you ever had the chance to work with JSON:API formatted responses before in your previous projects? Do you know any fixed attributes that won't change the name in this format?
I mostly used moshi and gson converter library in my previous project. I don't have experience using JSON:API library over those. I only heard those libraries and have little knowledge about that topic for now.
It is great to see that you are working with standardized architectural practices from Google to create the application (UI and Data layers, MVVM, DI with Hilt, etc.) At the same time, I do have a few small clarifications on the following parts that I would like to hear more from you:
You are implementing the UI layer of the application with Activity/Fragments. Thus, I am wondering if you have any experience with Jetpack Compose? And if you are requested to implement the application with it, how confident are you to deal with this request?
You are not handling the errors from the API at all. Right now, when there is an error, you are just throwing
RuntimeException
and thus the application will crash. It would be best if all the errors are handled with proper messages displayed to the user. https://github.com/hsu-hsu/nimble-survey-app/blob/81c798e104ee3a1fa125d72e060540d0ac316a86/app/src/main/java/com/nimble/android/features/home/HomeViewModel.kt#L37 https://github.com/hsu-hsu/nimble-survey-app/blob/81c798e104ee3a1fa125d72e060540d0ac316a86/app/src/main/java/com/nimble/android/features/login/LoginViewModel.kt#L46Wrapping
const
withObject
is old-fashioned with Java-style, thus do you happen to know any better approach that Kotlin allows us to do? https://github.com/hsu-hsu/nimble-survey-app/blob/81c798e104ee3a1fa125d72e060540d0ac316a86/app/src/main/java/com/nimble/android/utils/Constants.kt#L3-L12For the data layer, I noticed that you use Moshi and parse JSON:API formatted responses manually instead of using a 3rd-party library to handle this task. Why do you think parsing manually like this would be a better choice? And do you think using a JSON:API parser library would add more benefits?