search

kiwix / kiwix-xulrunner

[ARCHIVED] Legacy Kiwix desktop solution for Windows/macOS/Linux
https://download.kiwix.org/release/kiwix-xulrunner/
GNU General Public License v3.0
112 stars 28 forks source link

MVP pattern for the app? #420

Closed rashiq closed 7 years ago

rashiq commented 7 years ago

Hey! So I've been wanting to do this for quite some time now, but I'm not sure, if I should do it.

I want to refactor the app to follow the MVP (Model View Presenter) design pattern, in order to structure the app better, to enforce separation of concern better and especially to separate business logic from view logic.

It takes some time to wrap your head around this pattern, but once you get it it's really awesome and will make our code so much better. We will finally get rid of our gigantic classes. This will also make it possible for us to start writing actual tests. I've been refactoring our main activity for the past two weeks now, and this would be my next logical step.

Reading material: http://fernandocejas.com/2014/09/03/architecting-android-the-clean-way/ Examples of implementations: http://fernandocejas.com/2014/09/03/architecting-android-the-clean-way/, https://github.com/rashiq/bitcoin

(Look at how the activity in this example only has view logic: https://github.com/rashiq/bitcoin/blob/master/app/src/main/java/com/linechart/presentation/view/mainview/MainActivity.java and all the business logic is in the presenter: https://github.com/rashiq/bitcoin/blob/master/app/src/main/java/com/linechart/presentation/view/mainview/MainViewPresenter.java)

Let me know what you think of this @kelson42 @mhutti1 @EladKeyshawn

kelson42 commented 7 years ago

@rashiq I support this, but you mostly do have to agree with @mhutti1 and @EladKeyshawn. I also insist on the fact that this move should be done step-by-step and by implementing bug fixes/features. I want to avoid a many months long release blackout.

EladKeyshawn commented 7 years ago

@kelson42 @rashiq I agree as well, the code is really difficult to maintain as of now, there are many classes unnecessarily nested withing classes. This would be a great step.

rashiq commented 7 years ago

great ❤️ I will do it in steps as @kelson42 suggested, to not block new features

mhutti1 commented 7 years ago

@rashiq I also support this as the code does need to be cleaned up. Perhaps if you could start with an activity that is less complicated and considerably more compact i.e the Bookmark activity we could then see the application of this style in our project and it would certainly help me to get to grips with it.

rashiq commented 7 years ago

That's a really good idea @mhutti1. I will do that!

kelson42 commented 7 years ago

This issue was moved to kiwix/kiwix-android#29