Overview feedback

[dbof10]

Overall, it looks really great. However, there are some issues we could improve on

1. Better way to dispose .

   if (disposable != null) {
     disposable.dispose();
   }

   is repeated all over the place. Using https://github.com/uber/AutoDispose to reduce.

2. LoginPresenter. java A lot of bindings between Views and Presenter happening in the constructor looks like MVVM on iOS. It seems pretty hard to test!!!! Am I right? Need more unit test examples.

3. MainActivity.java line:39 I don't know which activity will be started. It's very subtle. I got it after navigating the project. Need a better name for this like Intent.makeInjectionActivity().name()...

4. Could you compare your dagger to google-architecture project? My only concern on your dagger is that you're using reflection in onCreate. it may slow down start-up process.

5. I really like CollectionView. It solves most of LCE problem.

6. I still prefer MVVM, Redux, MVI to MVP. How do you survive config change in MVP because the the view is detachedonDestroy? How about state management? for example if the button is liked, press to dislike and vice versa?

7. Navigation? References iOS: https://medium.com/@giovannyorozco24/mvvm-and-coordinator-pattern-together-8920fc0f1f55 https://github.com/AndreyPanov/ApplicationCoordinator https://will.townsend.io/2016/an-ios-coordinator-pattern

[talenguyen]

First of all, thanks for your attention and very detail feedback. I suggest we will discuss each item in a separated comment.

[talenguyen]

The disposable.dispose() is implemented in a base class then we don't need to repeat it over time. So I think we don't need to use a 3rd library in this case because it's over-engineering. By the way, +1 for your suggestion.

[talenguyen]

• It does not matter whether to binding in the constructor or somewhere else as long as you stop your binding in an appropriate lifecycle callback. In this case, destroy is the counterpart to constructor.
• To test the presenter you just need to mock the UserModel then you can do your testing. It's absolutely very easy to test.

[talenguyen]

Issue 3: thanks for your suggestion. I'll consider your suggestion for the better naming convention.

[talenguyen]

Issue 4:

• I'm not expert at the new dagger (2.1x) so I can't give a fair comparison. I just post my minimal configuration of my daggers here. So, you can compare yourself.
• I agree with you that daggers uses reflection to do its logic. But it's a trade-off to have the convenience and I think it's acceptable. FYI, butter-knife used the same kind of reflection to do its binding.