teerox / Nimble-iOS-Challenge

0 stars 0 forks source link

Architecture issues #13

Open minhnimble opened 2 years ago

minhnimble commented 2 years ago

I noticed that inside every completion block of the viewModel's method which is implemented in the ViewControllers, there is an addition of the DispatchQueue.main.async block usage for sending the tasks in the completion block on to the main thread. As a result, the DispatchQueue.main.async block usage will be repeated each time the application wants to call a new API in the future which creates scalability and redundant code problems. Do you know if there is any better way to refactor this code and avoid the duplications here? https://github.com/teerox/Nimble-iOS-Challenge/blob/d566fa26227d81d93b858f0374fc762696b33886/Nimble-iOS-Challenge/Views/HomeView/SingleHomeView/SingleHomeComponentViewController.swift#L46-L59

There are a mixed of logic and UI setup in your ViewControllers. For instance, you define an if else check for email and password validation logic directly inside LoginViewController to decide whether to call the login API or show an error popup. What is your opinion on moving these logics into the ViewModel layer? And how would you extract these logic to ViewModel if it is recommended? https://github.com/teerox/Nimble-iOS-Challenge/blob/d566fa26227d81d93b858f0374fc762696b33886/Nimble-iOS-Challenge/Views/Authentication/LoginView/LoginViewController.swift#L31-L59

For the data layer, I noticed that you are using Codable and parse JSON API formatted responses manually instead of using a 3rd-party library for handling 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? https://github.com/teerox/Nimble-iOS-Challenge/blob/2d1bdb4ffd5172a76b189e6dbcbfca87761b05d0/Nimble-iOS-Challenge/Models/HomeModel.swift#L35-L69

This one is more of a nitpick, but I don't see you apply Dependency Injection in the project at all and you are generating the ViewModel objects and their dependencies directly in the ViewController class. Do you think it would be better to apply Dependency Injection for this project? If yes, how and where are the most suitable places you think it would be worth applying?