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

Refactoring of codebase and separation on data/UI #715

Closed
6 tasks done
CyberWake opened this issue Apr 25, 2021 · 8 comments
Closed
6 tasks done

Refactoring of codebase and separation on data/UI #715

CyberWake opened this issue Apr 25, 2021 · 8 comments
Assignees

Comments

@CyberWake
Copy link
Contributor

CyberWake commented Apr 25, 2021

  • Issue Description:
    This issue is raised to refactor the code base to increase the readability and future maintainability, Following things could be done to refactor the code and can be worked upon as a child issue individually.

    • Migration of all the success and error toast to a common widget in success_toast.dart
      The widget here needs some modifications to make it universal on. Like needs two required parameters message and isSuccess with a option parameter duration. After which this widget needs to be used at all the places in the project by just passing the message, isSuccess and duration if needed. After successful accomplishing the task the file toast_tile.dart can be deleted with the file success_toast.dart renamed to something more appropriate like toast.dart etc.
    • Currently there in the codebase there are two controllers named OrgController in two different files named org_controller.dart and organization_controller.dart. Where the OrgControllerdefined in organization_controller.dart in not used in the project this file needs to be deleted.
    • Reducing the use of unnecessary wrapping of child widgets in all the files when not needed and when the desired output could be achieved with the child property itself eg: Container wrapped in Align or Padding widget, Nested Row and Columns``` etc.
    • Modification of widget declared in loading.dart to make it a universal loading widget. This widget can be provided with following properties like isCurrentOrgNull resulting in asking the user to join an organization. isNetworkError resulting in showing a error image and appropriate text. refresh function and message resulting in text displaying the message provided (like no post to show create one, no events to show create one etc) and a refresh button which calls the refresh function again to fetch the data if other users add data to the server.
    • Need to modify data modal classes in the project for easy conversion of json maps to class objects enabling easy traversal and retrieval of response data accessibility. As currently everywhere the the response map is directly iterated to search key which doesn't come handy in every place. And removal of user_info.dart not used in project and the user data model is much more bigger than the one defined. Better to create a new one. Currently declared models are less efficient and some of them are not even used. Model should be look alike the models declared in the api. We also need to have its implementation done on getting a successful response data from the server. Migrating all model to view_models or view_models to model keeping all in one place.
    • Making a universal class that stores all the data fetching functions currently written individually for each and every file for their own use. Doing so would help in separating the Data fetching functions to a separate place and UI in separate place. Also there is and advantage of this as well some functions need not be written again like the userinfo again and again.
  • Issue Severity:
    Medium to Low.

  • Environment Details:
    Project Level

  • Observed Behaviour:
    My Finding detailed above, all are free tell their we will add them as well if they fall in the category.

  • Expected Behaviour:
    Removal of unused widgets, unused files. Generation of reusable widgets, Reduction of code readability by separating data fetching and UI in separate places.

  • Does this issue need immediate attention?
    Yes, if this is done now then the removal and addition of features would be much easier in coming time.

  • Are you willing to work on this issue:
    Yes, Since this is a parent issue it needs child issues and needs to have multiple contributors who will to.

  • Other information:
    Suggestions are welcomed discussion thread

@github-actions github-actions bot added discussion unapproved Unapproved, needs to be triaged labels Apr 25, 2021
@utkarshshendge
Copy link
Contributor

utkarshshendge commented Apr 25, 2021

@Sagar2366 sir, I would like to work on the first child issue. Should I create an issue for it?

  • Migration of all the success and error toast to a common widget in success_toast.dart
    The widget here needs some modifications to make it universal on. Like needs two required parameters message and isSuccess with a option parameter duration. After which this widget needs to be used at all the places in the project by just passing the message, isSuccess and duration if needed. After successful accomplishing the task the file toast_tile.dart can be deleted with the file success_toast.dart renamed to something more appropriate like toast.dart etc.

@palisadoes
Copy link
Contributor

palisadoes commented Apr 25, 2021

@utkarshshendge Create all six child issues first so others can work on them. Reference the child issues in this one. We'll assign the one of your choice to you.

Let us know when you are done.

@utkarshshendge
Copy link
Contributor

Hi @CyberWake, many times it's necessary to wrap the Container in Align, padding, etc to get the desired behaviour.
I was not able to find a case where there is the unnecessary wrapping of child widgets.
Could you please mention such a case?

  • Reducing the use of unnecessary wrapping of child widgets in all the files when not needed and when the desired output could be achieved with the child property itself eg: Container wrapped in Align or Padding widget, Nested Row and Columns``` etc.

@CyberWake
Copy link
Contributor Author

Hi @CyberWake, many times it's necessary to wrap the Container in Align, padding, etc to get the desired behaviour.
I was not able to find a case where there is the unnecessary wrapping of child widgets.
Could you please mention such a case?

  • Reducing the use of unnecessary wrapping of child widgets in all the files when not needed and when the desired output could be achieved with the child property itself eg: Container wrapped in Align or Padding widget, Nested Row and Columns``` etc.

That was just a reference example plus is this is found anywhere then for padding as parent we may use margin property of container and for align the container itself has a alignment property to do so with it's child.

@palisadoes
Copy link
Contributor

@CyberWake please add all the child issues @utkarshshendge created to the first comment of the issue. That will help a lot with the resolution

@palisadoes
Copy link
Contributor

The following child issues have been created from this parent:

  1. Reduce unnecessary wrapping of child widgets. Reduce unnecessary wrapping of child widgets. #718
  2. Modification of Loading widget to make it a universal loading widget Modification of Loading widget to make it a universal loading widget #719
  3. Create classes to store the response map. Create classes to store the response map. #720
  4. Separate business logic from UI Separate business logic from UI #721
  5. Migration of all toasts to a common widget Migration of all toasts to a common widget #716
  6. Remove unused organization controller Remove unused organization controller #717

@palisadoes
Copy link
Contributor

@CyberWake I just realized that all the child issues that @utkarshshendge created have been assigned to others.

  1. Do you have any other suggestions to add as issues?
  2. Do you want to work jointly with the others who have been assigned. For example have them connect with you about how they want to approach the tasks?

Are there any other ways you'd like to work with this?

@palisadoes
Copy link
Contributor

Issue #700 has now been integrated into the pull request process. It will flag all Dart files that are longer than 300 lines to help with readability. Please make sure your contribution reduces file sizes to within this limit.

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

No branches or pull requests

3 participants