-
Notifications
You must be signed in to change notification settings - Fork 3.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Architecture #1160
Comments
Right now my thought is to use the Android Architecture Components to solve some of the problems. |
IMO, the app currently is overly reliant on inheritance, making code editing extremely hard. For example, some of the classesInstead we should rely on composition as it is a more scalable solution. I feel that once we adopt the mantra of "composition over inheritance", the separation of network and UI concerns should be easier to manage. I've looked into Android Architecture Components briefly, but have generally adopted an MVI approach in my own apps. However, it could be beneficial to have a standard library for the architecture to make it easier for people to contribute to the app. Ultimately, an adoption of any architecture would be beneficial. |
Well inheritance is a pain in some cases but for the most part I'm not seeing it as a huge problem right now. For architecture I've been having a look at:
It's mostly for inspiration but I'm leaning towards at least using the Domain, Data, Remote/Cache parts of it. The rest is kind of down to what ever we want do to in terms of presentation layer. |
Alright I've been looking mostly at the data end of this and still unsure about how I'd like to have it done. So I think we should probably start of with the UI parts. I think MVI is my prefered way to go. Rendering a single state object feels like a nice way to handle everything. So now it's probably just down to the how. @veyndan You have done some MVI earlier, do you have some nice way you generally approach it? |
MVI is definitely still an emerging structure in the Android community, but some things that I've found are useful is to make everything possible a stream. For example, the Android lifecycle, listening to any events (e.g. view events). A few libraries that should help with this is Navi and RxBinding. The use of Navi also means that we can remove the dependency on AutoDispose as Navi nicely handles unsubscription in a non "magical" way purely based on the way that Navi is used, compared to RxLifecycle and Autodispose. This is in relation to the discussion at #1149. Anko could also be potentially helpful but the other two libraries would definitely aid MVI. Regarding the overall design structure, this talk by Jake Wharton is a goldmine. I've implemented this in one of my other apps. Here is an example of how you can use events and models. Also it shows that side effects only occur in the subscribe of the Observable. All the methods referenced are pure functions and the stream doesn't modify the state except in the subscribe (e.g. there is no As converting an app of this size over to MVI will take a long time, I feel the best thing to do is make small changes before we start converting over to MVI. IMO the following should be done before such a change is made: fully convert the app over to Kotlin, remove unnecessary libraries, do some "light" refactoring, start using RxBinding and Navi (and Anko), be completely happy with the UI of the app (personally I found making architecture changes to an app while still making UI changes can be challenging). I probably left some stuff out, but this is the core of it. |
Regarding being happy with the UI that's not really gonna happen before moving I think. Also I'd prefer not pulling in to much extra stuff to be honest I'd prefer removing more libraries and only using the ones which are really needed. I'll take a look at the talk and your app over the week, will also be going away this weekend (+ mon-wed) so might be a bit unresponsive. Other than that I'll probably try to prototype something just to see how I'd like the handle the stuff. |
It's mostly with the overall structure of the app. Stuff like how list items look, colors, etc. don't really matter atm.
It's only really a net addition of one library as RxBinding and Navi can be added and can remove AutoDispose. It'll be hard to model an event stream with view clicks without having something like RxBinding so I feel that it is a necessity. Plus both of the libraries are very lightweight and very easy to understand.
Thanks for the heads up. If you want to discuss or talk about any ideas of MVI, I'm all ears. I'll probably continue to work on refactoring the code base, so hopefully MVI migration in the future will be less of a pain. |
As it stands the app mixes a lot of UI and network code. I makes it hard to understand the flow and to see where to UI changes are made. Other problems include huge files,
IssueFragment
is a good example.I'm open to suggestions to how we can fix this.
The text was updated successfully, but these errors were encountered: