HTTPクライアントがかなり使いずらいのでRetrofitに変更する #20
Conversation
|
""" Walkthroughこのプルリクエストは、Android アプリケーションのネットワーク層を Retrofit と Moshi を使用して再設計します。新しい Changes
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🧹 Nitpick comments (7)
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApi.kt (1)
5-7: 戻り値の型についての提案
Response<RepositoryList>を戻り値として使用することで、HTTPステータスコードやヘッダー情報にアクセスできるようになり、より詳細なエラーハンドリングが可能になります。interface GitHubRepositoryApi { - suspend fun getRepository(searchWord: String): RepositoryList + suspend fun getRepository(searchWord: String): Response<RepositoryList> }app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiBuilderInterface.kt (1)
8-13: GitHub APIのレート制限への対応GitHub APIにはレート制限があるため、以下の対応を推奨します：
- レスポンスヘッダーから制限情報を取得
- 制限に近づいた際のハンドリング
- キャッシュの実装
interface RateLimitHandler { fun handleRateLimit(response: Response<*>) fun getRemainingRequests(): Int fun isRateLimited(): Boolean }app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/entity/GitHubRepositoryEntity.kt (1)
5-17: KDocドキュメントの追加を推奨データクラスの各プロパティの意味や制約を明確にするため、KDocドキュメントの追加を推奨します。特に：
- 各フィールドの説明
- nullableな値の意味
- 数値フィールドの範囲や制約
/** * GitHubリポジトリの検索結果を表現するデータクラス * @property items 検索結果のリポジトリリスト */ data class RepositoryList( val items: List<RepositoryItem>, )app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt (1)
34-34: 例外クラスの改善より詳細なエラーハンドリングのため、具体的なエラー型の定義を推奨します。
sealed class NetworkException(message: String, cause: Throwable? = null) : Exception(message, cause) { class JsonParsingException(cause: Throwable? = null) : NetworkException("JSON解析エラー", cause) class NetworkConnectionException(cause: Throwable? = null) : NetworkException("ネットワークエラー", cause) class RateLimitException : NetworkException("APIレート制限に達しました") }app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt (2)
18-18: NetworkRepositoryの初期化設定を検討する必要がありますデフォルト設定でNetworkRepositoryを初期化していますが、タイムアウトやリトライなどの設定オプションを提供することを検討してください。
- private val networkRepository = NetworkRepository() + private val networkRepository = NetworkRepository( + connectTimeout = 30L, + readTimeout = 30L, + retryCount = 3 + )
40-41: エラーメッセージの国際化対応が必要ですエラーメッセージがハードコードされています。strings.xmlを使用して多言語対応することを推奨します。
- Log.e("NetworkException", e.message, e) - _errorMessage.postValue(e.message) + Log.e(TAG, "Network error occurred", e) + _errorMessage.postValue(context.getString(R.string.network_error_message))app/build.gradle (1)
67-70: 依存関係のバージョン管理の改善が必要です
- バージョン番号をバージョンカタログまたは変数として定義することを推奨します
- 依存関係の更新を容易にするため、バージョン制約をより柔軟にすることを検討してください
+ ext { + retrofit_version = "2.11.0" + moshi_version = "1.14.0" + okhttp_version = "4.12.0" + } + - implementation("com.squareup.retrofit2:converter-moshi:2.9.0") - implementation("com.squareup.okhttp3:logging-interceptor:4.12.0") - implementation("com.squareup.retrofit2:retrofit:2.11.0") - implementation("com.squareup.moshi:moshi-kotlin:1.14.0") + implementation("com.squareup.retrofit2:converter-moshi:$retrofit_version") + implementation("com.squareup.okhttp3:logging-interceptor:$okhttp_version") + implementation("com.squareup.retrofit2:retrofit:$retrofit_version") + implementation("com.squareup.moshi:moshi-kotlin:$moshi_version")
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
app/build.gradle(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/NetworkRepository.kt(0 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt(2 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApi.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiBuilderInterface.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiImpl.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/entity/GitHubRepositoryEntity.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt(1 hunks)build.gradle(1 hunks)
💤 Files with no reviewable changes (1)
- app/src/main/kotlin/jp/co/yumemi/android/code_check/NetworkRepository.kt
🧰 Additional context used
📓 Learnings (1)
app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt (4)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#12
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt:34-48
Timestamp: 2025-01-08T13:51:57.413Z
Learning: The error handling implementation in RepositorySearchViewModel is planned to be addressed in a separate PR to maintain focused changes and better code review process.
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#17
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchFragment.kt:70-70
Timestamp: 2025-01-19T18:29:34.699Z
Learning: In Android MVVM architecture, input validation like empty search query checks should be handled in the ViewModel (e.g., RepositorySearchViewModel.searchRepositories) rather than in the Fragment, following proper separation of concerns. The ViewModel can communicate validation errors back to the Fragment through LiveData.
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#12
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt:31-33
Timestamp: 2025-01-08T14:03:34.415Z
Learning: In Android/Kotlin ViewModels, HttpClient instances should be managed as ViewModel properties and properly closed in onCleared() rather than creating new instances for each request, to improve resource utilization and performance.
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#12
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchFragment.kt:35-35
Timestamp: 2025-01-08T13:52:10.282Z
Learning: ViewModelの初期化方法の改善など、アーキテクチャに関する変更は、コードの可読性向上とは別のPRで対応することが望ましい。
...main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt
Outdated
Show resolved
Hide resolved
...main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt
Outdated
Show resolved
Hide resolved
...c/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiImpl.kt
Outdated
Show resolved
Hide resolved
...c/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiImpl.kt
Outdated
Show resolved
Hide resolved
...c/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiImpl.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt (1)
19-19: 依存性注入の導入を推奨します
NetworkRepositoryのインスタンス化を直接行うのではなく、依存性注入を使用することでテスト容易性が向上します。- private val networkRepository = NetworkRepository() + private val networkRepository: NetworkRepository + + constructor( + application: Application, + networkRepository: NetworkRepository = NetworkRepository() + ) : super(application) { + this.networkRepository = networkRepository + }app/build.gradle (1)
67-71: 依存関係のバージョン管理の改善を推奨しますRetrofitと関連ライブラリのバージョンを変数として定義し、一元管理することを推奨します。
+ ext { + retrofit_version = "2.11.0" + moshi_version = "1.14.0" + okhttp_version = "4.12.0" + } + // Retrofit - implementation("com.squareup.retrofit2:converter-moshi:2.9.0") - implementation("com.squareup.okhttp3:logging-interceptor:4.12.0") - implementation("com.squareup.retrofit2:retrofit:2.11.0") - implementation("com.squareup.moshi:moshi-kotlin:1.14.0") + implementation("com.squareup.retrofit2:converter-moshi:$retrofit_version") + implementation("com.squareup.okhttp3:logging-interceptor:$okhttp_version") + implementation("com.squareup.retrofit2:retrofit:$retrofit_version") + implementation("com.squareup.moshi:moshi-kotlin:$moshi_version")app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiImpl.kt (1)
14-41: シングルトンパターンの実装を推奨しますcompanion objectのプロパティをprivateにし、適切なシングルトンパターンを実装することを推奨します。
companion object { - val client = + private val client = - val moshi = + private val moshi = - val weatherService = + private val githubService =app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt (2)
17-19: 依存性注入の実装をさらに改善できますデフォルト値を提供する代わりに、依存性を必須とすることで、より明示的な依存関係を作成できます。
-class NetworkRepository( - private val gitHubRepositoryApi: GitHubRepositoryApi = GitHubRepositoryApiImpl(), -) { +class NetworkRepository(private val gitHubRepositoryApi: GitHubRepositoryApi) {
35-35: nullの扱いを改善できます言語がnullの場合に"none"を使用していますが、より明示的な値を使用することを推奨します。
- language = item.language ?: "none", + language = item.language ?: "Not specified",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
app/build.gradle(2 hunks)app/src/main/AndroidManifest.xml(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt(2 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiImpl.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt(1 hunks)
🧰 Additional context used
📓 Learnings (3)
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiImpl.kt (1)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#20
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiImpl.kt:13-14
Timestamp: 2025-01-19T20:47:28.451Z
Learning: In Android applications using HttpLoggingInterceptor (OkHttp/Retrofit), the log level should be configured based on the build type using BuildConfig.DEBUG to prevent sensitive information leakage in production:
```kotlin
logging.level = if (BuildConfig.DEBUG) {
HttpLoggingInterceptor.Level.BODY
} else {
HttpLoggingInterceptor.Level.NONE
}
</details>
<details>
<summary>app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt (4)</summary>
Learnt from: harutiro
PR: #12
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt:34-48
Timestamp: 2025-01-08T13:51:57.413Z
Learning: The error handling implementation in RepositorySearchViewModel is planned to be addressed in a separate PR to maintain focused changes and better code review process.
Learnt from: harutiro
PR: #17
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchFragment.kt:70-70
Timestamp: 2025-01-19T18:29:34.699Z
Learning: In Android MVVM architecture, input validation like empty search query checks should be handled in the ViewModel (e.g., RepositorySearchViewModel.searchRepositories) rather than in the Fragment, following proper separation of concerns. The ViewModel can communicate validation errors back to the Fragment through LiveData.
Learnt from: harutiro
PR: #12
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt:31-33
Timestamp: 2025-01-08T14:03:34.415Z
Learning: In Android/Kotlin ViewModels, HttpClient instances should be managed as ViewModel properties and properly closed in onCleared() rather than creating new instances for each request, to improve resource utilization and performance.
Learnt from: harutiro
PR: #12
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchFragment.kt:35-35
Timestamp: 2025-01-08T13:52:10.282Z
Learning: ViewModelの初期化方法の改善など、アーキテクチャに関する変更は、コードの可読性向上とは別のPRで対応することが望ましい。
</details>
<details>
<summary>app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt (3)</summary>
Learnt from: harutiro
PR: #20
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt:11-31
Timestamp: 2025-01-19T20:33:29.369Z
Learning: In Android applications, network connectivity can be checked using ConnectivityManager. For Android M (API 23) and above, use NetworkCapabilities API, and for older versions, use NetworkInfo API. The ACCESS_NETWORK_STATE permission is required in AndroidManifest.xml.
Learnt from: harutiro
PR: #20
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt:8-10
Timestamp: 2025-01-19T20:47:21.710Z
Learning: In the Android application, dependency injection is preferred over direct instantiation to improve testability and flexibility. This applies particularly to service/repository classes like NetworkRepository that depend on other services or APIs.
Learnt from: harutiro
PR: #18
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/NetworkRepository.kt:0-0
Timestamp: 2025-01-19T19:23:10.771Z
Learning: In the Android app for jp.co.yumemi.android.code_check, when throwing custom exceptions like NetworkException, preserve the original exception cause by passing it as a parameter to maintain the error chain for better debugging and logging.
</details>
</details>
</details>
<details>
<summary>🔇 Additional comments (4)</summary><blockquote>
<details>
<summary>app/src/main/AndroidManifest.xml (1)</summary>
`4-4`: **適切なパーミッションが追加されています！**
`ACCESS_NETWORK_STATE`パーミッションの追加は、ネットワーク接続状態の確認に必要で適切です。
</details>
<details>
<summary>app/src/main/kotlin/jp/co/yumemi/android/code_check/RepositorySearchViewModel.kt (1)</summary>
Line range hint `33-45`: **エラーハンドリングの実装が適切です！**
空の入力チェックとネットワークエラーの処理が適切に実装されています。
</details>
<details>
<summary>app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiImpl.kt (1)</summary>
`43-54`: **エラーハンドリングの実装が適切です！**
HTTPステータスコードに応じた適切なエラーメッセージの提供が実装されています。
</details>
<details>
<summary>app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt (1)</summary>
`11-15`: **NetworkResultの実装が適切です！**
シールドクラスを使用した結果のラッピングは、型安全な方法でネットワーク操作の結果を処理できます。
</details>
</blockquote></details>
</details>
<!-- This is an auto-generated comment by CodeRabbit for review status -->
...c/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubRepositoryApiImpl.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt (2)
11-15:NetworkResultにLoadingステートの追加を推奨非同期処理の状態をより適切に表現するため、
Loadingステートの追加を検討してください。これにより、ユーザーインターフェースでローディング状態を適切に表示できます。sealed class NetworkResult<out T> { data class Success<T>(val data: T) : NetworkResult<T>() data class Error(val exception: NetworkException) : NetworkResult<Nothing>() + object Loading : NetworkResult<Nothing>() }
20-44: エラーメッセージのリソース化を推奨エラーメッセージをstrings.xmlリソースとして外部化することで、多言語対応と保守性が向上します。
- return NetworkResult.Error(NetworkException("オフライン状態です")) + return NetworkResult.Error(NetworkException(context.getString(R.string.error_offline))) - NetworkResult.Error(NetworkException("JSONパースエラー", e)) + NetworkResult.Error(NetworkException(context.getString(R.string.error_json_parse), e)) - NetworkResult.Error(NetworkException("ネットワークエラー", e)) + NetworkResult.Error(NetworkException(context.getString(R.string.error_network), e))strings.xmlに追加するリソース：
<resources> <string name="error_offline">オフライン状態です</string> <string name="error_json_parse">JSONパースエラー</string> <string name="error_network">ネットワークエラー</string> </resources>
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt(1 hunks)
🧰 Additional context used
📓 Learnings (1)
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt (3)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#20
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt:11-31
Timestamp: 2025-01-19T20:33:29.369Z
Learning: In Android applications, network connectivity can be checked using ConnectivityManager. For Android M (API 23) and above, use NetworkCapabilities API, and for older versions, use NetworkInfo API. The ACCESS_NETWORK_STATE permission is required in AndroidManifest.xml.
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#20
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt:8-10
Timestamp: 2025-01-19T20:47:21.710Z
Learning: In the Android application, dependency injection is preferred over direct instantiation to improve testability and flexibility. This applies particularly to service/repository classes like NetworkRepository that depend on other services or APIs.
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#18
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/NetworkRepository.kt:0-0
Timestamp: 2025-01-19T19:23:10.771Z
Learning: In the Android app for jp.co.yumemi.android.code_check, when throwing custom exceptions like NetworkException, preserve the original exception cause by passing it as a parameter to maintain the error chain for better debugging and logging.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: gradle
🔇 Additional comments (3)
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/NetworkRepository.kt (3)
17-19: 依存性注入のベストプラクティスに従うコンストラクタでのデフォルト実装は、テスト容易性と柔軟性を低下させます。依存性注入のベストプラクティスに従い、デフォルト実装を削除することを推奨します。
class NetworkRepository( - private val gitHubRepositoryApi: GitHubRepositoryApi = GitHubRepositoryApiImpl(), + private val gitHubRepositoryApi: GitHubRepositoryApi, )
46-52: ネットワーク接続チェックの実装が適切です
ConnectivityManagerを使用したネットワーク接続チェックの実装が、Androidのベストプラクティスに従っており、適切に実装されています。
55-55: 例外チェーンの保持が適切です
NetworkExceptionクラスがcauseパラメータを適切に保持しており、デバッグとロギングのための例外チェーンが維持されています。
概要
HTTPクライアントがかなり使いずらいのでRetrofitに変更する
現状はJsonのパース関係もかなりやりずらいので適応をする
関連Issue
このセクションでは、このPRが関連するIssueやタスクをリンクしてください。以下のように記述します。
変更点
このセクションでは、具体的な変更点や修正箇所を箇条書きでリストアップしてください。
テスト
このセクションでは、このPRに関連するテストケースやテスト方法を記載してください。
Summary by CodeRabbit
新機能
依存関係の更新
リファクタリング