search

LeoAndo / android-engineer-codecheck

Apache License 2.0
1 stars 0 forks source link

コードの気になるところ洗い出し #33

Open LeoAndo opened 1 year ago

LeoAndo commented 1 year ago

課題提出後なのでPRは出さないが、コード上の気になるところをこのイシューに洗い出しておく。

LeoAndo commented 1 year ago

プロガードの記述で消せそうなやつがある

以下、消せそう。 https://github.com/LeoAndo/android-engineer-codecheck/blob/0523ccc41050d425aa057b423dbf53481e830ba9/app/proguard-rules.pro#L13

LeoAndo commented 1 year ago

検索APIのレスポンス値について

pushed_at がnullのケースがあるのでNullableにする必要がある

pushed_atの型指定をString?に変更したい https://github.com/LeoAndo/android-engineer-codecheck/blob/57debd3a9774983cb7bbeb0dbc21c7dad88de86c/app/src/main/kotlin/jp/co/yumemi/android/codecheck/data/api/response/GithubSearchResponse.kt#L72

LeoAndo commented 1 year ago

RecyclerView周り

DiffUtilに指定する一意のキーはリポジトリ名を表すnameだと被るので、idを指定したい

https://github.com/LeoAndo/android-engineer-codecheck/blob/7ecf2d39768eb5310ed987d18a045c0e2ea06384/app/src/main/kotlin/jp/co/yumemi/android/codecheck/ui/repositories/RepositoriesListAdapter.kt#L37

検索APIのResponse値で取れるidをマッピングしたい https://github.com/LeoAndo/android-engineer-codecheck/blob/57debd3a9774983cb7bbeb0dbc21c7dad88de86c/app/src/main/kotlin/jp/co/yumemi/android/codecheck/data/api/response/GithubSearchResponse.kt#L51

https://github.com/LeoAndo/android-engineer-codecheck/blob/57debd3a9774983cb7bbeb0dbc21c7dad88de86c/app/src/main/kotlin/jp/co/yumemi/android/codecheck/data/api/response/GithubSearchResponse.kt#L127

API Reference

LeoAndo commented 1 year ago

package構成 / クラス名の変更

ErrorResult

ErrorResultクラスは、通信系のエラー専用なのでクラス名をApiErrorResultに変更し、別ファイルに切り出しapi配下に移動したい。

https://github.com/LeoAndo/android-engineer-codecheck/blob/7ecf2d39768eb5310ed987d18a045c0e2ea06384/app/src/main/kotlin/jp/co/yumemi/android/codecheck/data/SafeResult.kt#L15

以下のような感じで。 https://github.com/LeoAndo/AndroidGithubSearch/blob/deacbba26117839e5d1eb28fd9a810b291e4c026/app/src/main/java/com/leoleo/androidgithubsearch/data/api/ApiErrorResult.kt#L5

dataOrThrow

dataOrThrowメソッドは、Ktorに特化したエラーハンドリングのためKtorHandlerみたいなクラスを作りインスタンスメソッドで提供する形にし、別ファイルに切り出しapi配下に移動したい。 https://github.com/LeoAndo/android-engineer-codecheck/blob/7ecf2d39768eb5310ed987d18a045c0e2ea06384/app/src/main/kotlin/jp/co/yumemi/android/codecheck/data/SafeResult.kt#L23

以下のような感じで。 https://github.com/LeoAndo/AndroidGithubSearch/blob/c5d76b7ff15bb6ee63af6a99618140b268ea5666/app/src/main/java/com/leoleo/androidgithubsearch/data/api/KtorHandler.kt#L14

https://github.com/LeoAndo/AndroidGithubSearch/blob/c5d76b7ff15bb6ee63af6a99618140b268ea5666/app/src/main/java/com/leoleo/androidgithubsearch/di/NetworkModule.kt#L21

LeoAndo commented 1 year ago

各環境用でアクセスポイントを切り替える仕組み

本課題アプリはGithubが公開している外部APIを使うのみなので必要なさそうですが、 各環境のアクセスポイントを切り替える仕組みを持つことが通常なのでその場合は、以下のようにFlavorで対応する。 参考PR

LeoAndo commented 1 year ago

stub環境に関して

今回の課題アプリでは、出来上がっている 外部サービスのAPIを使っていて導入していないが、 実務においてサーバー API開発前にstubでアプリ側の開発を進めたいことがある。

もしGithub APIが開発段階であれば、以下のようなStubの実装を行います。

LeoAndo commented 1 year ago

マルチモジュール化

今回の課題アプリでは小規模なアプリのため導入を控えたが、 仮に導入する場合は以下のPRのような方針でモジュール化したい。 https://github.com/LeoAndo/AndroidGithubSearch/pull/56