Skip to content
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

Separate business logic from UI - (Locator - ViewModel - View) #758

Closed
rutvik11062000 opened this issue May 6, 2021 · 11 comments · Fixed by #759
Closed

Separate business logic from UI - (Locator - ViewModel - View) #758

rutvik11062000 opened this issue May 6, 2021 · 11 comments · Fixed by #759
Assignees
Labels
test Testing application

Comments

@rutvik11062000
Copy link
Contributor

Is your feature request related to a problem? Please describe.
Parent issue: #721

The parent issue only describes or deals with separating data fetching services and the UI.
As per the model - view - ViewModel architecture there must be a ViewModel layer between the service and UI layer.

Separating only the services layer will reduce the file size but it won't reduce files below 300 lines, as the after fetching functions are still executing inside the UI file.

Also by not separating the changing state or data modification function it will be difficult to test the UI and state changing logic.

Describe the solution you'd like

image

Creating a middle ViewModel layer which deals with "State change logic" and fetching data from the "services".
Creating an only View file that deals only with the UI and showing data that it receives from the respective ViewModel.

Approach to be followed (optional)

Steps:

  1. Register services in the locator.
  2. Register ViewModel in the locator.
  3. Separate XYZPage.dart -> (XYZView.dart and XYZViewModel.dart)
  4. Connect these two using BaseView and BaseViewModel class.
  5. Attach service to the ViewModel.

Additional context
This issue will work on all the pages the app has. for implementation, reference see PR: #728
The aftermath of solving this issue:

  1. reduced lines (<300) of every page.
  2. Separate and structured layers.
  3. Suitable for Unit testing and integration testing.
@github-actions github-actions bot added test Testing application unapproved Unapproved, needs to be triaged labels May 6, 2021
@CoderMayhem
Copy link
Contributor

I was working with the events page and I think this needs to be implemented there as the code is really clustered into a single view dart file. Implementing the MVVM architecture would be a great step to refactor the code. @Sagar2366 sir, I would like to open a PR refactoring the code into a viewModel for the events.dart page. Let me know if I can go ahead with this.

@rutvik11062000
Copy link
Contributor Author

@CoderMayhem, if you're working on the PR related to events.dart and if we are implementing MVVM you can pull our recent merge, I've already setup the locator and made other necessary changes😊.

@CoderMayhem
Copy link
Contributor

Sure. Will do👍

@rutvik11062000
Copy link
Contributor Author

@Sagar2366 At first I would like to work on the page files that are huge (around 500+ lines):

  1. update_profile_page.dart
  2. register_form.dart
  3. create_organization.dart
  4. profile_page.dart

@Sagar2366 Sagar2366 added enhancement and removed unapproved Unapproved, needs to be triaged labels May 6, 2021
@StrangeNoob
Copy link
Contributor

@rutvik11062000 If you don't mind, Can we divide those pages and so that everyone else can work on it?
Then We will be able to refactor in time

@rutvik11062000
Copy link
Contributor Author

Sure @StrangeNoob 😊, if anyone wants to take this issue they can implement MMVM in the below page/pages:

  1. queries_.dart
  2. add_event_page.dart
  3. events.dart (@CoderMayhem )
  4. edit_event_dialog.dart:
  5. set_url_page.dart
  6. news_article.dart
  7. register_form_test.dart (May take a different approach to reduce lines MVVM doesn't apply).

@rutvik11062000
Copy link
Contributor Author

I recommend making a PR for every page instead of making a single PR, this will reduce the code checking (review) that has to be done by the mentors, and also we can roll back if a page is not properly structured.

@rutvik11062000
Copy link
Contributor Author

@CoderMayhem @StrangeNoob, please ask Sagar sir, to assign the pages you want to work on. 👍

@utkarshshendge
Copy link
Contributor

@Sagar2366 Sir, @rutvik11062000 maybe we can do something like issue #326 .
Everyone will get a chance to work on the issue and stacking of issues will be prevented.

@Sagar2366
Copy link
Contributor

@Sagar2366 Sir, @rutvik11062000 maybe we can do something like issue #326 .
Everyone will get a chance to work on the issue and stacking of issues will be prevented.

Right.
@rutvik11062000 can you please create child issues as well for different screens . Other contributors can work on them.

Before that, at least any one screen changes should be made available to reference it as a template.

@rutvik11062000
Copy link
Contributor Author

Sure @Sagar2366 sir👍, I'm currently working on the update profile page once it is done it will work as a template, @utkarshshendge I'll create child issues for pages that need MVVM and will also make a discussion thread in which there will be general standard methods to implement this architecture so all the contributors will be on the same page.

(All these things will be done till evening).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
test Testing application
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants