WorldHealthOrganization / app

COVID-19 App
Other
2.13k stars 509 forks source link

Apply Design Pattern to current Flutter code #374

Closed HuzefaGadi closed 4 years ago

HuzefaGadi commented 4 years ago

I went through the codebase and I think it is an initial draft, I would suggest we move it into more structured way with using design patterns like MVVM clubbed with Providers. If we start of with this practice early on it will be very easy to integrate things later on when the codebase becomes big.

let me know what you guys think

poojadev commented 4 years ago

@HuzefaGadi completely agree with you, need to move it into a more structured way..also I would like to suggest comment and document the code. so it will be easy to maintain the codebase for everyone who is working on it.

icostel commented 4 years ago

Think redux in the way to go in the long run: https://pub.dev/packages/redux and https://pub.dev/packages/flutter_redux with epics(optional). I've used in on several monolithic projects, works like a charm.

britannio commented 4 years ago

I think it's best to start with what's officially recommended by the Flutter team, Provider. https://flutter.dev/docs/development/data-and-backend/state-mgmt/simple

If not then here's a curated list of state management options https://flutter.dev/docs/development/data-and-backend/state-mgmt/options

dna-f commented 4 years ago

Hi, why don't we follow the Bloc approach with a business object that handles the logic through streams IN (commands/sink) and OUT (results for the streambuilders) We could use RxDart and other libraries like RxCommand to extend dart stream and streamController.

Build reactive mobile apps with Flutter (Google I/O '18) https://www.youtube.com/watch?v=RS36gBEp8OI

singhayush1403 commented 4 years ago

Provider with MVVM seems to be the best option. Provider is easy to implement and doesn't have any boilerplate code. Also it's recommended by the Flutter team

ghost commented 4 years ago

Hi, why don't we follow the Bloc approach with a business object that handles the logic through streams IN (commands/sink) and OUT (results for the streambuilders) We could use RxDart and other libraries like RxCommand to extend dart stream and streamController.

Build reactive mobile apps with Flutter (Google I/O '18) https://www.youtube.com/watch?v=RS36gBEp8OI

i agree with you, we can use bloc, and it easy to use unit test in this design pattern

HuzefaGadi commented 4 years ago

I haven't used BLoC or Redux but I have seen the code which seems there is too much boilerplate involved in it. whereas Provider is straight forward and MVVM will make it easier to manage as well.

those are my thoughts

dna-f commented 4 years ago

You can use Provider to pass Bloc objects references down along the tree through the context Otherwise you can register singleton instances of certain bloc objects using GetIt for example This way you can retrive them everywhere and call actions on the bloc, receiving back results through the streams

HuzefaGadi commented 4 years ago

@dna-f I have used GetIt and it is very convenient. I have used the combination of provider along with GetIt for dependency Injection along with MVVM as my design pattern, the code is very compact and it makes the code easy to maintain as well.

icostel commented 4 years ago

Both redux and bloc are easy to test, both do their job well. Redux is more of a MVI approach and with epics you don't even have to use rx in the project, as it's all based on streams. But if state management doesn't seem to complex then Provider can be enough.

britannio commented 4 years ago

Hi, why don't we follow the Bloc approach with a business object that handles the logic through streams IN (commands/sink) and OUT (results for the streambuilders) We could use RxDart and other libraries like RxCommand to extend dart stream and streamController.

Build reactive mobile apps with Flutter (Google I/O '18) https://www.youtube.com/watch?v=RS36gBEp8OI

The 2019 I/O video is more relevant

Pragmatic State Management in Flutter (Google I/O'19) https://www.youtube.com/watch?v=d_m5csmrf7I&

dustincatap commented 4 years ago

+1 on using GetIt for dependency injection. Any thoughts on using it with ScopedModel?

britannio commented 4 years ago

+1 on using GetIt for dependency injection. Any thoughts on using it with ScopedModel?

Provider beats ScopedModel https://flutter.dev/docs/development/data-and-backend/state-mgmt/simple

dna-f commented 4 years ago

I use ScopedModel together with the bloc pattern Imagine you have an infinite grid of posts. To accomplish this, I delegate business logic to retrive paginated data to a Bloc instance, creating a streambuilder attached to the output stream of the bloc class But what if you need to update a single post card, when the user click on a heart icon, to say that he/she likes it? In this case, instead of changing the data model of the post and reply the entire stream, I simply change the model and notify through a ScopedModel only the part of the post card that should be rebuilt. They can go together I think :)

edTheGuy00 commented 4 years ago

So there is clearly already a wide variety of options being thrown around. I think that most of use might believe that we have the perfect architecture and state management approach, but keep in mind that this is a community effort and we need to choose something that majority of contributors are comfortable with. Not only for us now but for future contributors as well.

In my opinion, I would vote for get_it + bloc . But we need someone to decide what is the best approach for this project and spearhead this decision by beginning to implement the skeleton app.

britannio commented 4 years ago

Tom Glider made a good point in the Slack about us not even having any state to manage.

Remember to refer to the ROADMAP

patniemeyer commented 4 years ago

Hi everyone. All input is welcome and I know that there area lot of opinions on this that we will eventually take into account, however it is absolutely paramount right now that we keep this codebase simple and flexible since we don't have all of the requirements. To that end I'd like to just do the simplest thing possible that solves the problems. Google's recommended "simple" approach suggests hoisting state where needed, using streams and change notifiers. I've written some very sophisticated apps relying solely on those and they are fine. I would also be open to using ReactiveX for more advanced stream and async management as that has wide adoption across platforms and will be recognizable to many developers as well.

I'm not sure if we should leave this thread open to collect ideas on this further or whether it will become a distraction and preference war :) Let's try leaving it open for now.

lifenautjoe commented 4 years ago

Keeping it simple should be the most important thing at this point, thanks for that.

In regards to the state management discussion for when the time comes:

We've been building a social network app in Flutter for more than 2 years now (https://github.com/OkunaOrg/okuna-app) since their pre-alpha phase.

We used the Bloc Pattern initially. We deeply regretted it as it resulted in a convoluted code-base with tons of bloc providers which when we had to coordinate between two or more, chaos ensued. Testing was even worse.

We then tried scoped model and it was a big improvement. Easier to test, yet still, many of our contributors were confused over the responsibilities of these "Scoped models" versus the components themselves. We found ourselves having to do tons of explaining.

Finally, having worked with DI systems before in many projects, we decided to build our own version of a service provider (this was before the provider library was available).

It worked wonders. We structured our application logic into independent and well defined small services, used through a unified API in a UserService. Life's been great since then.

I cannot recommend providers and services enough. We also use RX to manage high level application state management and binding of API models to views for representation but that's a story for some other time.

I've written to the WHO contact email about me and my team. Apart from running that social network we have a digital agency for NGOs, charities and non-profits ( https://ankida.agency ) and given our long journey creating a media heavy, multi-lingual (10+ languages) and accessible Flutter app, we would like to help out, absolutely free of any charge or compromise. For the sake of our shared humanity.

If you and your team are interested, please feel free to contact me at joel@ankida.agency .

Best of luck till then!

advayDev1 commented 4 years ago

This is not currently actionable. @patniemeyer if you do want to refactor the client code at some point, let's make a new issue with a specific proposal at that time.