toby-thanathip / SurveyApp

0 stars 0 forks source link

Need a proper architecture implementation #17

Open sleepylee opened 6 years ago

sleepylee commented 6 years ago

Hi Tuubz, I'm Trung from Nimbl3 and I'll be in charge of giving the comments over this assignment.

For this first issue, I'd say it's pretty naive nowadays to start the implementation without a proper architecture (either MVP, MVVM, MVI...), putting logic controllers in Activity is a very bad practice for testing and decoupling components.

toby-thanathip commented 6 years ago

Thanks for the feedback, Trung!

toby-thanathip commented 6 years ago

putting logic controllers in Activity is a very bad practice for testing and decoupling component

Could your perhaps specify or give an example, please? Preferably one from my project :)

sleepylee commented 6 years ago

Hi @Tuubz, for example, in this Activity, preferably the network request execution, response handler (for success and error) should not be driven directly by the activity. Activity should be dumb as much as possible, it should only receive the signal whether it should show/hide an UI element. This will help us decoupling the responsibility, avoiding a hugee Activity class when the app grows, and enhance the testability on the logic/integration part after separated from the Android concrete classes.

Hope my explanation helps.

toby-thanathip commented 6 years ago

Thanks @sleepylee,

I've decided to move the success/error handling to my repository class. But I'm kinda struggling on how to make the relationship between the activity and the repository class, to update the view. Somehow I need a observer/listener in the activity, to let it know if the data that's been called (asynchronously) in the repository class has arrived or not, so it can update the view.

I was thinking of using EventBus, what do you think?

sleepylee commented 6 years ago

@Tuubz it's not a bad choice, we used it before too, but now we are moving toward RxJava as our main event driven force. You may want to check out this google sample on mvvm + rxjava (also check out the other branches as well, they have another different pattern implementation as well).