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

Create classes to store the response map. #720

Closed
utkarshshendge opened this issue Apr 25, 2021 · 10 comments
Closed

Create classes to store the response map. #720

utkarshshendge opened this issue Apr 25, 2021 · 10 comments
Assignees

Comments

@utkarshshendge
Copy link
Contributor

utkarshshendge commented Apr 25, 2021

  • Parent issue : Refactoring of codebase and separation on data/UI #715

  • Issue Description:
    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 response map is directly iterated to the search key which doesn't come in handy at every place. And the removal ofuser_info.dartnot used in the project and the user data model is much bigger than the one defined. Better to create a new one. Currently, declared models are less efficient, and some are not even used. The model should look like the models declared in the API. We also need to have its implementation done on getting successful response data from the server.

  • Issue Severity:
    Medium

  • Environment Details:
    Project Level

  • Observed Behaviour:
    The response map is directly iterated to the search key.

  • Expected Behaviour:
    Use of classes to store the response.

  • Does this issue need immediate attention?
    No

  • Please ask mentors/maintainers to assign if you are willing to work on this issue.

@github-actions github-actions bot added the unapproved Unapproved, needs to be triaged label Apr 25, 2021
@Sagar2366 Sagar2366 added code refactor and removed unapproved Unapproved, needs to be triaged labels Apr 25, 2021
@StrangeNoob
Copy link
Contributor

@Sagar2366 I am free I can work on 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.

@StrangeNoob
Copy link
Contributor

Sure @palisadoes I will keep this in mind while sending PR.

@StrangeNoob
Copy link
Contributor

@palisadoes As This will add more class in the model. I was thinking of changing the directory structure. It will look something like this.

+-- lib/
|     +-- models/
|      |    +-- responses/
|      |    |    +-- fetchUserInfo.dart
|      |    |    +-- fetchOrgById.dart
|      |    +-- comments.dart

@Sagar2366, @palisadoes Can you share your views? @utkarshshendge & @sumitra19jha I will love to take your suggestion on this.

@utkarshshendge
Copy link
Contributor Author

@StrangeNoob your approach looks fine. It will make the code more readable and it will be easy to add tests.

@sumitra19jha
Copy link
Contributor

sumitra19jha commented Apr 26, 2021

@StrangeNoob Can you elaborate a little more

Like the data model are to be build keeping the backend responses in mind. For example: Comment data model will contain comment text and post id. The same is the case with user information which will contain first name, last name, email etc. They both are build keeping backend response in mind so why to assign separate response folder like that

Also, is it good to name a model fetchUserInfo or should we name them as User model and Organization model

@StrangeNoob
Copy link
Contributor

StrangeNoob commented Apr 26, 2021

@StrangeNoob Can you elaborate a little what you mean

@sumitra19jha At a glance, New developer won't be having idea out the response of the every API Call. Converting the response JSON to a class will help it.

Like the data model are to be build keeping the backend responses in mind. For example: Comment data model will contain comment text and post id. The same is the case with user information which will contain first name, last name, email etc. They both are build keeping backend response in mind.

I will look at the response and convert it as per requirement.

Also, is it good to name a model fetchUserInfo or should we name them as User model and Organization model.

Right now, I am not concerned with the name of the files but its directory structure?

@sumitra19jha
Copy link
Contributor

sumitra19jha commented Apr 26, 2021

It will organize the codebase.

What is the purpose of the data model?
It will make sure an organized structure is followed throughout the project just like a data type. Currently, we request server and use the response in crude form in the code itself. This might create confusion for new developer's.

For example:
Let suppose I am a new developer going through the codebase, somewhere I see a query result called user info. If the result is converted into a data model, I will get to know that the results contain the name, email, image etc. But if it's left in its crude form, first I need to visit the query file see the response body and then again visit the widget file and use it accordingly.

So your work will structure the codebase and also help the developers work more effectively.

And in my opinion, the folder structure should be somewhat like lib/models/<model_name>.dart
We can create a nested folder for further categorising the models but I haven't generally seen it in some of the projects I have worked on😅. But yeah it can be done

@sumitra19jha
Copy link
Contributor

sumitra19jha commented Apr 26, 2021

@CyberWake On our work on separation of business logic from UI #721, this creation of data models will help us in coming with a great solution.

During refactoring, we can make sure all the logics are very well implemented using this data model rather than current response map.

@StrangeNoob
Copy link
Contributor

StrangeNoob commented Apr 26, 2021

@sumitra19jha Thanks for the input. I will put it model directory only. Trying to complete as soon as possible to it will help you and @CyberWake to implement logics into it.

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

5 participants