search

AnalogIO / coffeecard_app

Cross-platform coffee card app for Cafe Analog
https://www.cafeanalog.dk/app
MIT License
6 stars 1 forks source link

Refactor `NetworkRequestExecutor`; utilise `Unit`; remove need for dummy providers targeting `dynamic` types #503

Closed marfavi closed 10 months ago

marfavi commented 10 months ago

Overview:

This PR refactors the NetworkRequestExecutor class to improve its readability, maintainability, and testability. It also updates all dependent components to align with the new executor methods.

Notably, it also removes the need to provide dummy values for certain dynamic types, e.g. for the type Either<NetworkFailure, dynamic>, which triggered linter warnings.

Changes to NetworkRequestExecutor:

  1. Added New Typedefs: Introduced _NetworkRequest<BodyType> and _ExecutorResult<R> to avoid repetition.

  2. Method Renaming and Refactoring:

    • Renamed call to execute and updated its signature.
    • Renamed logResponse to _logResponse and slightly simplified its logic.
    • Added executeAndDiscard for requests where the response body is not needed.
      • This change prevents the need to provide dummy values for dynamic types as mentioned in the overview.

  3. New Mapping Extensions:

    • Added ExecutorMapX and ExecutorMapAllX to handle mapping of results. The call to map() or mapAll() should be chained after an execute() call.
    • Calling mapAll() is only possible when dealing with executor calls where the expected response body is an Iterable.
    • The aim of these are to provide a better DX, and to aid the transition into fpdart's TaskEither class in the future.
    • These are added to a new file network_request_executor_mapping.dart, which is a part of network_request_executor.dart.

  4. Added Documentation: Added doc comments to all methods above.

How it affects code

Take a look at how code is affected by this refactor:

leaderboard_remote_data_source.dart occupation_remote_data_source.dart
```diff Future>> getLeaderboard( LeaderboardFilter category, int top, - ) async { - return executor( - () => apiV2.apiV2LeaderboardTopGet(preset: category.label, top: top), - ).bindFuture( - (result) => result.map(LeaderboardUserModel.fromDTO).toList(), - ); + ) { + return executor + .execute( + () => apiV2.apiV2LeaderboardTopGet(preset: category.label, top: top), + ) + .mapAll(LeaderboardUserModel.fromDTO); } Future> getLeaderboardUser( LeaderboardFilter category, - ) async { - return executor( - () => apiV2.apiV2LeaderboardGet(preset: category.label), - ).bindFuture(LeaderboardUserModel.fromDTO); + ) { + return executor + .execute(() => apiV2.apiV2LeaderboardGet(preset: category.label)) + .map(LeaderboardUserModel.fromDTO); } } ``` ```diff - Future>> getOccupations() async { - final result = await executor( - () => api.apiV1ProgrammesGet(), - ); - return result - .map((result) => result.map(OccupationModel.fromDTOV1).toList()); + Future>> getOccupations() { + return executor + .execute(api.apiV1ProgrammesGet) + .mapAll(OccupationModel.fromDTOV1); } ```

Other changes

  1. Updates in Other Components:

    • Updated EnvironmentRemoteDataSource, OpeningHoursRemoteDataSource, ProductRemoteDataSource, etc., to use the new executor methods.
      • The map()/mapAll() extensions remove either_extensions.dart as a dependency to these files.
    • Removed OpeningHoursRepository (and Impl); replaced with new class OpeningHoursModel to better align with the rest of the codebase.
    • Changed OpeningHoursRemoteDataSource.getOpeningHours() to return an entity rather than a generated DTO class to better align the rest of the codebase.

  2. Test Updates: Updated tests to align with the new executor methods.

  3. Removed a barrel file that wasn't supposed to exist in feature Opening Hours

    • This accounts for ~9 files changed just for changed imports.

  4. ticket_remote_data_source.dart: Moved mapper method out of the large chain of calls to improve readability.

Why This is Necessary:

  1. Readability: Method renaming and new typedefs make the code easier to understand.

  2. Testability: The executeAndDiscard method simplifies testing for requests where the response body is not important.

  3. Maintainability: Improved documentation and more descriptive method names make the code easier to maintain.

Please review and let me know if any changes are required.

ghost commented 10 months ago
👇 Click on the image for a new way to code review #### [![Review these changes using an interactive CodeSee Map](https://s3.us-east-2.amazonaws.com/maps.codesee.io/images/github/AnalogIO/coffeecard_app/503/a246c4b3/03cc4a6857d9666254f53ae851c33962bcf4137c.svg)](https://app.codesee.io/r/reviews?pr=503&src=https%3A%2F%2Fgithub.com%2FAnalogIO%2Fcoffeecard_app) #### Legend CodeSee Map legend
codecov[bot] commented 10 months ago

Codecov Report

Patch coverage: 88.09% and project coverage change: +5.27% :tada:

Comparison is base (4aed248) 79.58% compared to head (03cc4a6) 84.85%. Report is 2 commits behind head on develop.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## develop #503 +/- ## =========================================== + Coverage 79.58% 84.85% +5.27% =========================================== Files 114 114 Lines 1195 1195 =========================================== + Hits 951 1014 +63 + Misses 244 181 -63 ``` | [Flag](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO) | `84.85% <88.09%> (+5.27%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#carryforward-flags-in-the-pull-request-comment) to find out more. | [Files Changed](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO) | Coverage Δ | | |---|---|---| | [...ening\_hours/domain/usecases/check\_open\_status.dart](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#diff-bGliL2ZlYXR1cmVzL29wZW5pbmdfaG91cnMvZG9tYWluL3VzZWNhc2VzL2NoZWNrX29wZW5fc3RhdHVzLmRhcnQ=) | `100.00% <ø> (ø)` | | | [...\_hours/presentation/cubit/opening\_hours\_cubit.dart](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#diff-bGliL2ZlYXR1cmVzL29wZW5pbmdfaG91cnMvcHJlc2VudGF0aW9uL2N1Yml0L29wZW5pbmdfaG91cnNfY3ViaXQuZGFydA==) | `100.00% <ø> (ø)` | | | [...atures/register/domain/usecases/register\_user.dart](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#diff-bGliL2ZlYXR1cmVzL3JlZ2lzdGVyL2RvbWFpbi91c2VjYXNlcy9yZWdpc3Rlcl91c2VyLmRhcnQ=) | `80.00% <ø> (ø)` | | | [...user/domain/usecases/request\_account\_deletion.dart](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#diff-bGliL2ZlYXR1cmVzL3VzZXIvZG9tYWluL3VzZWNhc2VzL3JlcXVlc3RfYWNjb3VudF9kZWxldGlvbi5kYXJ0) | `100.00% <ø> (ø)` | | | [...user/data/datasources/user\_remote\_data\_source.dart](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#diff-bGliL2ZlYXR1cmVzL3VzZXIvZGF0YS9kYXRhc291cmNlcy91c2VyX3JlbW90ZV9kYXRhX3NvdXJjZS5kYXJ0) | `56.25% <53.33%> (-2.58%)` | :arrow_down: | | [.../data/datasources/purchase\_remote\_data\_source.dart](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#diff-bGliL2ZlYXR1cmVzL3B1cmNoYXNlL2RhdGEvZGF0YXNvdXJjZXMvcHVyY2hhc2VfcmVtb3RlX2RhdGFfc291cmNlLmRhcnQ=) | `75.00% <70.00%> (+11.36%)` | :arrow_up: | | [...r/data/datasources/voucher\_remote\_data\_source.dart](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#diff-bGliL2ZlYXR1cmVzL3ZvdWNoZXIvZGF0YS9kYXRhc291cmNlcy92b3VjaGVyX3JlbW90ZV9kYXRhX3NvdXJjZS5kYXJ0) | `83.33% <75.00%> (+3.33%)` | :arrow_up: | | [...e/data/datasources/account\_remote\_data\_source.dart](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#diff-bGliL2NvcmUvZGF0YS9kYXRhc291cmNlcy9hY2NvdW50X3JlbW90ZV9kYXRhX3NvdXJjZS5kYXJ0) | `83.33% <81.25%> (+7.14%)` | :arrow_up: | | [...ta/datasources/leaderboard\_remote\_data\_source.dart](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#diff-bGliL2ZlYXR1cmVzL2xlYWRlcmJvYXJkL2RhdGEvZGF0YXNvdXJjZXMvbGVhZGVyYm9hcmRfcmVtb3RlX2RhdGFfc291cmNlLmRhcnQ=) | `64.28% <85.71%> (+7.14%)` | :arrow_up: | | [lib/core/network/network\_request\_executor.dart](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO#diff-bGliL2NvcmUvbmV0d29yay9uZXR3b3JrX3JlcXVlc3RfZXhlY3V0b3IuZGFydA==) | `100.00% <100.00%> (ø)` | | | ... and [11 more](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO) | | ... and [6 files with indirect coverage changes](https://app.codecov.io/gh/AnalogIO/coffeecard_app/pull/503/indirect-changes?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=AnalogIO)

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.