nowinandroid icon indicating copy to clipboard operation
nowinandroid copied to clipboard

Refactor network module

Open MohShMehr opened this issue 5 months ago • 6 comments

What I have done and why

  • Refactored the network module to use dependency injection (DI).
  • Introduced separate interfaces for each entity API.
  • No functional changes; behavior remains the same.

This refactor was inspired by using the NIA project as a base project. It improves maintainability and makes the codebase easier to extend when working with multiple API methods.

Testing

  • Ran ./gradlew testDemoDebug locally; all tests passed.
  • Applied formatting via ./gradlew --init-script gradle/init.gradle.kts spotlessApply.

Related issues

  • N/A

MohShMehr avatar Aug 27 '25 05:08 MohShMehr

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

google-cla[bot] avatar Aug 27 '25 05:08 google-cla[bot]

@googlebot I signed it!

MohShMehr avatar Aug 27 '25 06:08 MohShMehr

@googlebot I signed it!

MohShMehr avatar Aug 27 '25 06:08 MohShMehr

@googlebot I signed it!

MohShMehr avatar Aug 27 '25 07:08 MohShMehr

In the previous codebase, it was easy to see the backend API schema in a single file, whereas now it's split across 2 files. Since we have no plans to change/extend the backend API, I'm struggling to understand the benefit of this change.

dturner avatar Sep 02 '25 12:09 dturner

In the previous codebase, it was easy to see the backend API schema in a single file, whereas now it's split across 2 files. Since we have no plans to change/extend the backend API, I'm struggling to understand the benefit of this change.

Thanks for the thoughtful question! Since Now in Android is often used as a base project, this refactor keeps the backend surface unchanged and focuses on making the client network layer more scalable and maintainable so it can be adopted as‑is. Separation of concerns: Retrofit/OkHttp config lives in DI; the data source consumes typed TopicApi and NewsResourceApi. Testability: Inject fakes for those APIs without bootstrapping Retrofit/OkHttp/Json. Maintainability: One @Singleton Retrofit centralizes timeouts/interceptors/base URL. Clear ownership: Endpoints are grouped by resource, avoiding a single “god” interface. I appreciate the convenience of a single-file view and can accommodate that while preserving these benefits. Would you prefer: Co-locating both interfaces in one file (e.g., api/Apis.kt), or Adding a short endpoint index in core/network/README.md? Happy to update the PR based on your preference.

MohShMehr avatar Sep 02 '25 17:09 MohShMehr