UI をブラッシュアップ #27
Merged
UI をブラッシュアップ #27
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
hennkou sita.
Walkthroughこのプルリクエストでは、アプリケーションの構成を大幅に変更しています。主な変更点は、Kotlinファイルに対する新しい設定の追加、Jetpack Compose関連のプラグインと依存関係の導入、メインアクティビティの変更、リポジトリ関連のデータ構造の更新、APIインターフェースの改善、UIコンポーネントの追加、テストケースの強化などです。これにより、Kotlinの関数命名規則に関する柔軟性が向上し、全体的なアーキテクチャが洗練されています。 Changes
Sequence DiagramsequenceDiagram
participant User
participant SearchScreen
participant ViewModel
participant Repository
participant GitHubAPI
User->>SearchScreen: 検索キーワード入力
SearchScreen->>ViewModel: 検索リクエスト
ViewModel->>Repository: リポジトリ検索
Repository->>GitHubAPI: APIリクエスト
GitHubAPI-->>Repository: リポジトリリスト
Repository-->>ViewModel: 検索結果
ViewModel-->>SearchScreen: リポジトリ一覧表示
Possibly related PRs
Poem
🪧 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: 7
🧹 Nitpick comments (17)
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubServiceApiBuilderInterface.kt (2)
11-14: KDocコメントの追加を推奨しますメソッドの目的、パラメータ、戻り値の説明をKDocで追加することで、APIの使用方法がより明確になります。
以下のような形式を提案します：
+ /** + * 指定された検索語でGitHubリポジトリを検索します + * @param searchWord 検索キーワード + * @return リポジトリ一覧のレスポンス + */ @GET("/search/repositories") suspend fun getRepositoryList( @Query("q") searchWord: String, ): Response<RepositoryList>
Line range hint
1-21: 全体的に良く設計されたインターフェースですGitHubのAPIエンドポイントが適切にマッピングされており、Retrofitの規約に従っています。メソッド名も直感的で、責務が明確です。
このインターフェースを実装する際は、パフォーマンス向上のためにHTTPクライアント（RetrofitとOkHttpClient）をシングルトンとして実装することを推奨します。
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubServiceApi.kt (1)
7-9: APIインターフェースの改善を確認しました！メソッド名の変更により、機能の目的がより明確になっています。
KDocコメントを追加することで、各メソッドの使用方法と期待される動作をより明確にできます。以下のような形式を推奨します：
/** * 指定された検索ワードに基づいてリポジトリのリストを取得します * @param searchWord 検索キーワード * @return リポジトリのリスト */ suspend fun getRepositoryList(searchWord: String): RepositoryList /** * 指定されたIDのリポジトリ詳細を取得します * @param id リポジトリID * @return 単一のリポジトリ情報 */ suspend fun getRepositoryDetail(id: Int): RepositoryItemapp/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/entity/GitHubServiceEntity.kt (1)
Line range hint
8-18: プロパティの順序を最適化することを推奨しますデータクラスのプロパティを以下の順序で整理することで、可読性が向上します：
- 主要な識別子（id）
- 基本情報（name, language）
- 所有者情報（owner）
- 統計情報（stargazersCount, watchersCount, forksCount, openIssuesCount）
data class RepositoryItem( val id: Int, val name: String, + val language: String?, val owner: RepositoryOwner, - val language: String?, @Json(name = "stargazers_count") val stargazersCount: Long, @Json(name = "watchers_count") val watchersCount: Long, @Json(name = "forks_count") val forksCount: Long, @Json(name = "open_issues_count") val openIssuesCount: Long, )app/src/main/kotlin/jp/co/yumemi/android/code_check/MainActivity.kt (1)
14-23: システムUIの処理を改善することを推奨します現在の実装にシステムバーの処理が含まれていません。以下の改善を提案します：
@AndroidEntryPoint class MainActivity : ComponentActivity() { override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) + WindowCompat.setDecorFitsSystemWindows(window, false) setContent { + val systemUiController = rememberSystemUiController() + val useDarkIcons = !isSystemInDarkTheme() + + DisposableEffect(systemUiController, useDarkIcons) { + systemUiController.setSystemBarsColor( + color = Color.Transparent, + darkIcons = useDarkIcons + ) + onDispose {} + } + CodeCheckAppTheme { MainScreen() } } } }必要なインポートを追加してください：
import androidx.compose.foundation.isSystemInDarkTheme import androidx.core.view.WindowCompat import com.google.accompanist.systemuicontroller.rememberSystemUiControllerapp/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/router/MainRouter.kt (2)
16-24: ドキュメンテーションの追加を推奨しますComposable関数の目的と各パラメータの役割を説明するKDocを追加することで、コードの可読性と保守性が向上します。
以下のようなドキュメントの追加を提案します：
+/** + * メインナビゲーションを管理するComposable関数 + * @param toDetailScreen 詳細画面への遷移を処理する関数 + * @param toBackScreen 前の画面に戻る処理を行う関数 + * @param changeTopBarTitle トップバーのタイトルを変更する関数 + * @param showSnackbar スナックバーを表示する関数 + * @param navController ナビゲーションを制御するコントローラー + * @param modifier レイアウトの修飾子 + */ @Composable fun MainRouter(
40-42: ナビゲーション引数の定数化を推奨しますナビゲーション引数の文字列をコード内で直接使用するのではなく、定数として定義することで、タイプミスを防ぎ、保守性を向上させることができます。
以下のような実装を提案します：
+private const val ARG_ID = "id" + composable( - BottomNavigationBarRoute.DETAIL.route + "/{id}", - arguments = listOf(navArgument("id") { type = NavType.IntType }), + BottomNavigationBarRoute.DETAIL.route + "/{$ARG_ID}", + arguments = listOf(navArgument(ARG_ID) { type = NavType.IntType }),app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubServiceApiImpl.kt (1)
47-55: エラーメッセージの定数化とログ機能の追加を推奨しますエラーハンドリングの実装は適切ですが、以下の改善を提案します：
- エラーメッセージの文字列を定数として定義
- APIエラー発生時のログ記録の追加
以下のような実装を提案します：
+companion object { + private const val ERROR_NOT_FOUND = "リポジトリが見つかりませんでした" + private const val ERROR_RATE_LIMIT = "APIレート制限に達しました" + private const val ERROR_SERVER = "サーバーエラーが発生しました" + private const val ERROR_EMPTY_RESPONSE = "レスポンスが空でした" +} if (!response.isSuccessful) { + val errorCode = response.code() + Log.e("GitHubAPI", "API error: $errorCode") throw when (response.code()) { - 404 -> NetworkException("リポジトリが見つかりませんでした") - 403 -> NetworkException("APIレート制限に達しました") - 500 -> NetworkException("サーバーエラーが発生しました") + 404 -> NetworkException(ERROR_NOT_FOUND) + 403 -> NetworkException(ERROR_RATE_LIMIT) + 500 -> NetworkException(ERROR_SERVER) else -> NetworkException("エラーが発生しました: ${response.code()}") } } -return response.body() ?: throw NetworkException("レスポンスが空でした") +return response.body() ?: throw NetworkException(ERROR_EMPTY_RESPONSE)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/GitHubServiceRepositoryImpl.kt (1)
17-33: エンティティマッピングの冗長性を改善できます
RepositoryEntityへのマッピングロジックがfetchSearchResultsとfetchRepositoryDetailで重複しています。共通のマッピング関数を作成することを推奨します。+private fun mapToRepositoryEntity( + name: String, + ownerIconUrl: String, + language: String?, + stargazersCount: Int, + watchersCount: Int, + forksCount: Int, + openIssuesCount: Int, + id: Int +): RepositoryEntity { + return RepositoryEntity( + name = name, + ownerIconUrl = ownerIconUrl, + language = language ?: "none", + stargazersCount = stargazersCount, + watchersCount = watchersCount, + forksCount = forksCount, + openIssuesCount = openIssuesCount, + id = id + ) +}app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubMockData.kt (1)
16-48: モックデータの値をより現実的なものにすることを推奨しますテストデータの値（Star数、Fork数など）が簡略化されすぎています。より現実的なデータを使用することで、エッジケースのテストが改善される可能性があります。
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/MainScreen.kt (2)
41-45: 状態管理の改善が推奨されます
topBarTitleの状態管理をViewModelに移動することで、UIロジックとビジネスロジックの分離が改善されます。
51-67: ナビゲーションアイコンの条件分岐をより簡潔に実装できます現在の実装は少し冗長です。条件演算子を使用することで、コードをより簡潔にできます。
- val navigationIcon: @Composable () -> Unit = - if (navBackStackEntry?.destination?.route != BottomNavigationBarRoute.SEARCH.route) { - { - IconButton(onClick = { - navController.popBackStack() - }) { - Icon( - imageVector = Icons.AutoMirrored.Outlined.ArrowBack, - contentDescription = "Back", - ) - } - } - } else { - { - EmptyCompose() - } - } + val navigationIcon: @Composable () -> Unit = { + if (navBackStackEntry?.destination?.route != BottomNavigationBarRoute.SEARCH.route) { + IconButton(onClick = { navController.popBackStack() }) { + Icon( + imageVector = Icons.AutoMirrored.Outlined.ArrowBack, + contentDescription = "Back", + ) + } + } + }app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceRepositoryImplTest.kt (1)
113-128: テストケースの追加を推奨しますレート制限エラー（429）のテストケースは適切ですが、他のHTTPエラーコード（例：401、403、404、500など）のテストケースも追加することを推奨します。
追加のテストケースの実装をお手伝いしましょうか？
app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceUsecaseImplTest.kt (1)
76-88: エラーメッセージのリソース化を推奨しますオフライン状態のエラーメッセージ「オフライン状態です」がハードコードされています。多言語対応や保守性の観点から、strings.xmlリソースとして定義することを推奨します。
- assertEquals("オフライン状態です", exception.message) + assertEquals(context.getString(R.string.error_offline), exception.message)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt (1)
45-92: エラーハンドリングの改善を推奨します
- エラーメッセージの表示後に
errorMessageをnullにリセットすることで、同じエラーの再表示を防ぐことができます。- エラー発生時のリトライ機能の追加を検討してください。
viewModel.errorMessage.observe(lifecycleOwner) { it?.let { showSnackBar( context.getString(it), true, ) + viewModel.clearError() } isLoading = false }app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/theme/Theme.kt (2)
14-24: カラースキームの改善提案カラースキームの実装について、以下の改善を提案します：
- セマンティックカラーの追加
- エラー状態用の色の定義
- カラーの透明度バリエーションの追加
Also applies to: 25-34
36-59: テーマ実装のベストプラクティステーマの実装は適切ですが、以下の点を考慮することを推奨します：
- プレビュー用のアノテーションの追加
- テーマ固有の型やシェイプの定義
@Composable +@Preview(name = "Light Theme") +@Preview(name = "Dark Theme", uiMode = Configuration.UI_MODE_NIGHT_YES) fun CodeCheckAppTheme( darkTheme: Boolean = isSystemInDarkTheme(), dynamicColor: Boolean = true, content: @Composable () -> Unit, ) {
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (38)
.editorconfig(1 hunks)app/build.gradle(4 hunks)app/src/main/AndroidManifest.xml(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/MainActivity.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/TopActivity.kt(0 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/entity/RepositoryEntity.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/MainScreen.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailFragment.kt(0 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailViewModel.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/router/MainRouter.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositoryListRecyclerViewAdapter.kt(0 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchFragment.kt(0 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchViewModel.kt(2 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/theme/Color.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/theme/Theme.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/theme/Type.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/widget/EmptyCompose.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/widget/ProgressCycle.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubServiceApi.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubServiceApiBuilderInterface.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubServiceApiImpl.kt(2 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/entity/GitHubServiceEntity.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/GitHubServiceRepository.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/GitHubServiceRepositoryImpl.kt(3 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/usecase/GitHubServiceUsecase.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/usecase/GitHubServiceUsecaseImpl.kt(2 hunks)app/src/main/res/layout/activity_top.xml(0 hunks)app/src/main/res/layout/fragment_repository_detail.xml(0 hunks)app/src/main/res/layout/fragment_repository_search.xml(0 hunks)app/src/main/res/layout/layout_item.xml(0 hunks)app/src/main/res/navigation/nav_graph.xml(0 hunks)app/src/main/res/values/strings.xml(1 hunks)app/src/test/kotlin/jp/co/yumemi/android/code_check/ExampleUnitTest.kt(0 hunks)app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubMockData.kt(1 hunks)app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceRepositoryImplTest.kt(6 hunks)app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceUsecaseImplTest.kt(2 hunks)
💤 Files with no reviewable changes (10)
- app/src/test/kotlin/jp/co/yumemi/android/code_check/ExampleUnitTest.kt
- app/src/main/res/layout/activity_top.xml
- app/src/main/res/layout/fragment_repository_detail.xml
- app/src/main/kotlin/jp/co/yumemi/android/code_check/TopActivity.kt
- app/src/main/res/layout/layout_item.xml
- app/src/main/res/layout/fragment_repository_search.xml
- app/src/main/res/navigation/nav_graph.xml
- app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailFragment.kt
- app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchFragment.kt
- app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositoryListRecyclerViewAdapter.kt
✅ Files skipped from review due to trivial changes (1)
- app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/theme/Color.kt
🧰 Additional context used
📓 Learnings (7)
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubServiceApiBuilderInterface.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:0-0
Timestamp: 2025-01-19T21:02:41.511Z
Learning: In jp.co.yumemi.android.code_check, HTTP clients (Retrofit, OkHttpClient) should be implemented as singletons to improve performance by reusing instances instead of creating new ones for each request.
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/usecase/GitHubServiceUsecase.kt (1)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#25
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/usecase/GitHubServiceUsecase.kt:0-0
Timestamp: 2025-01-20T02:33:49.633Z
Learning: In this Android project, the architecture follows clean architecture principles:
1. UseCase layer (domain layer) is kept framework-independent, avoiding direct Android dependencies like Context
2. Network connectivity checks are handled by a separate NetworkConnectivityService
3. Dependencies are properly injected using Hilt's @Inject constructor
4. The GitHubServiceUsecase interface defines the business logic contract without framework dependencies
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchViewModel.kt (7)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#25
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/GitHubServiceRepositoryImpl.kt:14-35
Timestamp: 2025-01-20T01:56:21.501Z
Learning: In Android applications using Clean Architecture, error handling should be implemented using sealed classes to define error types. The Repository layer should map exceptions to these error types, while the ViewModel layer should handle the UI-specific error message presentation.
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#25
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/usecase/GitHubServiceUsecaseImpl.kt:27-33
Timestamp: 2025-01-20T01:56:52.662Z
Learning: In Android clean architecture, avoid passing Context through ViewModel. Instead, create a dedicated service (e.g., NetworkConnectivityService) that handles Context-dependent operations and inject it using Hilt with @ApplicationContext.
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#25
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchViewModel.kt:0-0
Timestamp: 2025-01-20T02:39:26.958Z
Learning: In the Android MVVM architecture of this project, ViewModels should depend on interfaces (e.g., GitHubServiceUsecase) rather than concrete implementations (e.g., GitHubServiceUsecaseImpl) to follow the dependency inversion principle, improve testability, and maintain loose coupling.
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#25
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchViewModel.kt:39-42
Timestamp: 2025-01-20T02:32:22.262Z
Learning: In Android MVVM architecture, ViewModels should not depend on Context. Instead, use string resource IDs (e.g., R.string.error_message) in the ViewModel and convert them to actual strings in the Fragment/Activity using getString(). This improves testability and maintains proper separation of concerns.
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で対応することが望ましい。
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/usecase/GitHubServiceUsecaseImpl.kt (2)
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:0-0
Timestamp: 2025-01-19T21:09:30.232Z
Learning: In Android network operations, wrap results in a sealed class (e.g., NetworkResult) to handle Success and Error cases separately, and implement network availability check using ConnectivityManager before making API calls to provide better user experience in offline scenarios.
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.
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailViewModel.kt (7)
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#25
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchViewModel.kt:39-42
Timestamp: 2025-01-20T02:32:22.262Z
Learning: In Android MVVM architecture, ViewModels should not depend on Context. Instead, use string resource IDs (e.g., R.string.error_message) in the ViewModel and convert them to actual strings in the Fragment/Activity using getString(). This improves testability and maintains proper separation of concerns.
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#25
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchViewModel.kt:0-0
Timestamp: 2025-01-20T02:39:26.958Z
Learning: In the Android MVVM architecture of this project, ViewModels should depend on interfaces (e.g., GitHubServiceUsecase) rather than concrete implementations (e.g., GitHubServiceUsecaseImpl) to follow the dependency inversion principle, improve testability, and maintain loose coupling.
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#25
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchFragment.kt:0-0
Timestamp: 2025-01-20T02:43:53.821Z
Learning: In Android MVVM architecture, ViewModels should not have direct dependencies on Context or View-related components. Instead, use string resource IDs in ViewModels and handle string resolution in the Fragment/Activity layer. This maintains proper separation of concerns and makes ViewModels more testable.
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で対応することが望ましい。
app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceRepositoryImplTest.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:0-0
Timestamp: 2025-01-19T21:02:41.511Z
Learning: In jp.co.yumemi.android.code_check, HTTP clients (Retrofit, OkHttpClient) should be implemented as singletons to improve performance by reusing instances instead of creating new ones for each request.
app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceUsecaseImplTest.kt (1)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#25
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/usecase/GitHubServiceUsecase.kt:0-0
Timestamp: 2025-01-20T02:33:49.633Z
Learning: In this Android project, the architecture follows clean architecture principles:
1. UseCase layer (domain layer) is kept framework-independent, avoiding direct Android dependencies like Context
2. Network connectivity checks are handled by a separate NetworkConnectivityService
3. Dependencies are properly injected using Hilt's @Inject constructor
4. The GitHubServiceUsecase interface defines the business logic contract without framework dependencies
🪛 detekt (1.23.7)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/widget/EmptyCompose.kt
[warning] 6-7: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (20)
app/src/main/res/values/strings.xml (1)
15-16: 新しい文字列リソースの追加を承認します！新しく追加された「詳細」と「検索」の文字列リソースは、既存のリソースと一貫性があり、Jetpack Composeへの移行に適切に対応しています。シンプルで分かりやすい表現になっています。
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/api/GitHubServiceApiBuilderInterface.kt (2)
3-8: インポートの構成が適切です！必要なインポートが整理されており、余分なものがありません。
16-19: エラーハンドリングの検証と文書化が必要です
- 存在しないリポジトリIDの場合の404エラー処理の確認が必要です。
- メソッドの使用方法とエラーケースをKDocで文書化することを推奨します。
以下のような形式を提案します：
+ /** + * 指定されたIDのリポジトリ詳細を取得します + * @param id リポジトリID + * @return リポジトリ詳細のレスポンス + * @throws HttpException リポジトリが存在しない場合は404エラーが発生 + */ @GET("/repositories/{id}") suspend fun getRepositoryDetail( @Path("id") id: Int, ): Response<RepositoryItem>以下のスクリプトで404エラーの処理を確認できます：
✅ Verification successful
エラーハンドリングは適切に実装されています
実装を確認したところ、以下の点で十分なエラー処理が行われています：
GitHubServiceApiImplでレスポンスの成功確認- リポジトリ層での例外ハンドリング
- テストケースでの各種エラーシナリオの検証
ただし、提案したKDocによる文書化は、APIの使用者にとって有用な追加情報となりますので、実装することを推奨します。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify error handling in the codebase # Search for error handling patterns in the implementation rg -A 5 "getRepositoryDetail.*catch"Length of output: 38
Script:
#!/bin/bash # Find implementation of GitHubServiceApiBuilderInterface ast-grep --pattern 'class $_ : GitHubServiceApiBuilderInterface' # Find test files related to repository detail fd -e kt test.*repository -x cat {} # Search for any usage of getRepositoryDetail rg "getRepositoryDetail" -B 2 -A 2Length of output: 5032
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/entity/RepositoryEntity.kt (1)
7-8: エンティティの改善を確認しました！クラス名の変更とIDフィールドの追加は、クリーンアーキテクチャの原則に沿った適切な変更です。
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/usecase/GitHubServiceUsecase.kt (1)
7-9: ユースケースの実装が適切です！クリーンアーキテクチャの原則に従い、フレームワークに依存しない実装となっています。
NetworkResultを使用したエラーハンドリングも適切です。app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/GitHubServiceRepository.kt (1)
7-9: リポジトリレイヤーの実装が適切です！クリーンアーキテクチャの層構造に従った実装となっています。
実装の整合性を確認するため、以下の点を検証することを推奨します：
✅ Verification successful
実装の検証が完了しました！
リポジトリの実装が以下の点で適切であることを確認しました：
- 両メソッドの実装が完全で整合性があります
- エラーハンドリングが適切に実装されています
- テストカバレッジが十分で、全てのケースがテストされています
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # リポジトリの実装クラスとテストの存在を確認 echo "リポジトリ実装の検証を開始します..." rg -l "class.*GitHubServiceRepositoryImpl.*:.*GitHubServiceRepository" # テストファイルの存在確認 echo "テストファイルの検証を開始します..." fd -g "*GitHubServiceRepositoryImplTest.kt"Length of output: 673
Script:
#!/bin/bash # 実装の詳細を確認 echo "実装クラスの検証:" rg "class GitHubServiceRepositoryImpl" -A 10 echo "\nテストケースの検証:" # テストメソッドの存在確認 rg "fun.*test.*fetch(SearchResults|RepositoryDetail)" app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceRepositoryImplTest.kt # エラーハンドリングのテスト確認 echo "\nエラーハンドリングテストの検証:" rg "test.*Error" app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceRepositoryImplTest.ktLength of output: 3687
Script:
#!/bin/bash # 実装クラスの完全な内容を確認 echo "実装クラスの完全な内容:" cat app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/GitHubServiceRepositoryImpl.kt echo "\nテストファイルの完全な内容:" cat app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceRepositoryImplTest.ktLength of output: 9204
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/usecase/GitHubServiceUsecaseImpl.kt (2)
16-21: 実装が適切です！クリーンアーキテクチャの原則に従い、以下の点が適切に実装されています：
- ネットワーク接続確認の事前チェック
- エラーハンドリングの一貫性
- 依存性の適切な注入
23-28: 新しいメソッドの実装も同様に適切です！
fetchRepositoryDetailメソッドも同じパターンで実装されており、コードの一貫性が保たれています。app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchViewModel.kt (1)
30-31: データ型の変更が適切に実装されています！
RepositoryItemからRepositoryEntityへのデータ型の変更が、以下の点で適切に実装されています：
- クリーンアーキテクチャの原則に従ったデータ型の選択
- LiveDataの可視性の適切な制御
- インターフェースへの依存による疎結合の維持
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailViewModel.kt (2)
24-28: LiveDataの公開方法が適切ですprivateな
MutableLiveDataとpublicなLiveDataの組み合わせで、適切なカプセル化が実現されています。
60-69: エラーハンドリングの網羅性が高く、適切です各種エラーケースに対して適切なメッセージリソースが割り当てられています。
app/src/main/kotlin/jp/co/yumemi/android/code_check/features/github/reposiotory/GitHubServiceRepositoryImpl.kt (1)
49-77: エラーハンドリングが適切に実装されていますHTTPエラー、JSONパース、IOエラーなど、想定される例外が適切にハンドリングされています。
app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubMockData.kt (1)
106-118: エラーケースのモックが適切に実装されていますネットワークエラーやAPIエラーなど、様々なエラーケースに対応するモックが用意されており、テストの網羅性が確保されています。
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/MainScreen.kt (1)
112-126: カスタムSnackbarの実装が適切ですエラー状態に応じて適切にスタイルを変更しており、ユーザーにわかりやすいフィードバックを提供しています。
app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceRepositoryImplTest.kt (2)
96-111: 実装が適切です！テストケースが適切に構造化されており、Arrange-Act-Assertパターンに従っています。成功ケースの検証も適切に実装されています。
Line range hint
31-42: プロパティ名の不一致を修正してくださいコメントでは
ownerIconUrlからavatarUrlに変更されたと記載されていますが、アサーションでは依然としてownerIconUrlが使用されています。一貫性を保つため、どちらかに統一する必要があります。app/src/test/kotlin/jp/co/yumemi/android/code_check/features/github/GitHubServiceUsecaseImplTest.kt (1)
45-59: 実装が適切です！モックデータの使用、アサーションの実装、テストの構造化が適切に行われています。
.editorconfig (1)
3-3: 設定が適切に追加されています！Jetpack Compose関数の命名規則を適切に処理するための設定が追加されており、プロジェクトの方向性と一致しています。
app/src/main/AndroidManifest.xml (1)
17-17: アクティビティの変更を確認してくださいTopActivityからMainActivityへの移行が行われていますが、以下の点を確認してください：
- MainActivityクラスが正しく実装されていること
- 既存の機能が新しいActivityに移行されていること
✅ Verification successful
アクティビティの実装が確認できました
MainActivityは以下の通り適切に実装されています：
- ComponentActivityを継承し、Jetpack Composeベースのアプリケーションとして実装
- Hiltによる依存性注入の設定が完了
- MainScreenコンポーザブルを使用したUI実装が行われている
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # MainActivityの実装を確認 ast-grep --pattern 'class MainActivity : ComponentActivity { $$$ }'Length of output: 71
Script:
#!/bin/bash # MainActivityファイルを検索 fd "MainActivity" --type f # MainActivity実装を検索（Kotlin） ast-grep --pattern 'class MainActivity $_ { $$$ }' # MainActivity実装を検索（Java） ast-grep --pattern 'public class MainActivity $_ { $$$ }'Length of output: 211
Script:
#!/bin/bash # MainActivityの実装内容を確認 cat app/src/main/kotlin/jp/co/yumemi/android/code_check/MainActivity.ktLength of output: 751
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/widget/EmptyCompose.kt (1)
5-7: 空のComposable関数の目的を確認してくださいこの空のComposable関数について：
- 目的が不明確です
- 実装が必要な場合は、TODOコメントを追加することを推奨します
- 不要な場合は削除を検討してください
✅ Verification successful
EmptyComposeは適切に使用されています
この空のComposable関数は、検索画面（SEARCH route）でナビゲーションアイコンを非表示にするために意図的に使用されています。これはComposeでよく見られるパターンで、TopAppBarのAPIで必要とされる実装です。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # この関数の使用箇所を確認 rg "EmptyCompose\(\)" -A 2Length of output: 532
Script:
#!/bin/bash # MainScreenでのEmptyComposeの使用コンテキストを確認 rg -B 5 -A 5 "EmptyCompose\(\)" app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/MainScreen.ktLength of output: 288
Script:
#!/bin/bash # MainScreenの条件分岐の完全なコンテキストを確認 rg -B 15 -A 15 "EmptyCompose\(\)" app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/MainScreen.ktLength of output: 1092
🧰 Tools
🪛 detekt (1.23.7)
[warning] 6-7: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/theme/Type.kt
Show resolved
Hide resolved
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/widget/ProgressCycle.kt
Show resolved
Hide resolved
...in/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailViewModel.kt
Show resolved
Hide resolved
.../main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt
Show resolved
Hide resolved
.../main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt
Show resolved
Hide resolved
.../main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (7)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/theme/Type.kt (1)
9-52: タイポグラフィの実装が完全になりました前回のレビューで提案された改善点が適切に実装されています：
bodyLarge、titleLarge、titleMedium、bodyMedium、labelSmallの各スタイルが実装されています- フォントサイズやレタースペーシングがMaterial Designのガイドラインに準拠しています
- 各スタイルの用途が明確です
ただし、さらなる改善として以下の検討をお願いします：
- 日本語フォントのサポート
- ダークテーマ対応時の可読性確保
val Typography = Typography( bodyLarge = TextStyle( - fontFamily = FontFamily.Default, + // 日本語フォントファミリーを追加 + fontFamily = FontFamily( + Font(R.font.noto_sans_jp_regular) + ), fontWeight = FontWeight.Normal, fontSize = 16.sp, lineHeight = 24.sp, letterSpacing = 0.5.sp, ),app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (2)
176-207: アクセシビリティとローカライゼーションの改善が必要です
- アイコンのコンテンツ記述がハードコードされています
- 統計情報の文字列がハードコードされています
- フォントサイズがハードコードされています
以下の改善を提案します：
-contentDescription = "Stars", +contentDescription = context.getString(R.string.stars_icon_description), -Text("Stars: ${repository.stargazersCount}", fontSize = 18.sp, fontWeight = FontWeight.Medium) +Text( + text = context.getString(R.string.stars_count_format, repository.stargazersCount), + style = MaterialTheme.typography.bodyLarge, + fontWeight = FontWeight.Medium +)同様の変更を
ForksとOpen Issuesにも適用してください。
212-246: プレビュー機能の拡充を推奨します現在のプレビューは基本的なケースのみをカバーしています。以下のような追加のプレビューケースの実装を推奨します：
- 長いリポジトリ名のケース
- 言語が設定されていないケース
- 大きな数値（例：100万以上のスター数）のケース
@Preview(showBackground = true, name = "Long Repository Name") @Composable fun PreviewRepositoryOverviewCardLongName() { RepositoryOverviewCard( repository = RepositoryEntity( id = 1, name = "Very Long Repository Name That Might Wrap", ownerIconUrl = "https://via.placeholder.com/150", language = "Kotlin", stargazersCount = 123, forksCount = 45, openIssuesCount = 2, watchersCount = 1, ) ) }app/src/main/res/values/strings.xml (1)
15-23: 文字列リソースの改善を推奨します新しく追加された文字列リソースについて、以下の改善を提案します：
- アクセシビリティのために、各文字列に
description属性を追加- 開発者向けにコメントで使用コンテキストを明記
- <string name="invalid_repository_id">正しいIDが返されませんでした</string> + <!-- Used when repository ID validation fails --> + <string name="invalid_repository_id" description="リポジトリIDの検証エラーメッセージ">正しいIDが返されませんでした</string> - <string name="search_icon_description">検索アイコン</string> + <!-- Used for search icon in CustomSearchBar --> + <string name="search_icon_description" description="検索機能を表すアイコンの説明">検索アイコン</string>app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/widget/ProgressCycle.kt (1)
20-49: Composableの柔軟性向上を推奨します現在の実装に以下の改善を提案します：
- プログレスインジケータのサイズをカスタマイズ可能に
- テーマカラーの適用
- アニメーション速度の制御オプション
@Composable fun ProgressCycle( message: String = stringResource(R.string.searching), contentDescription: String = stringResource(R.string.loading_content_description), + size: Dp = 48.dp, + color: Color = MaterialTheme.colorScheme.primary, + animationSpec: InfiniteRepeatableSpec<Float> = infiniteRepeatable( + animation = tween(1000, easing = LinearEasing), + repeatMode = RepeatMode.Restart + ) ) { Column( verticalArrangement = Arrangement.Center, horizontalAlignment = Alignment.CenterHorizontally, modifier = Modifier .fillMaxWidth() .fillMaxHeight() .padding(8.dp) .semantics { isTraversalGroup = true this.contentDescription = contentDescription }, ) { CircularProgressIndicator( + modifier = Modifier + .size(size) + .semantics { + this.contentDescription = contentDescription + }, + color = color, + strokeWidth = (size / 8).coerceAtLeast(2.dp), ) Text( text = message, modifier = Modifier.padding(top = 8.dp), + color = color ) } }app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt (2)
95-125: リストビューのパフォーマンス最適化を推奨します以下の改善点を提案します：
- アイテムのキー設定
- スクロールパフォーマンスの最適化
- アイテムの再利用
@Composable fun RepositoryListView( repositoryList: List<RepositoryEntity>, onTapping: (Int) -> Unit = {}, ) { LazyColumn( modifier = Modifier .fillMaxWidth() .padding(8.dp) .semantics { isTraversalGroup = true }, ) { - items(repositoryList.size) { index -> + items( + items = repositoryList, + key = { it.id } + ) { repository -> Column( modifier = Modifier - .clickable { onTapping(repositoryList[index].id) } + .clickable { onTapping(repository.id) } .fillMaxWidth() .padding(vertical = 8.dp, horizontal = 16.dp), ) { Text( - text = repositoryList[index].name, + text = repository.name, style = MaterialTheme.typography.bodyLarge, color = MaterialTheme.colorScheme.primary, ) Spacer(modifier = Modifier.height(8.dp)) HorizontalDivider(color = MaterialTheme.colorScheme.onBackground.copy(alpha = 0.2f)) } } } }
127-186: 検索バーのユーザビリティ向上を推奨します以下の改善点を提案します：
- 入力クリアボタンの追加
- 検索履歴の実装
- 入力バリデーションの強化
@Composable fun CustomSearchBar( inputText: String = "", onValueChange: (String) -> Unit = {}, searchAction: (String) -> Unit, + onClear: () -> Unit = {}, + searchHistory: List<String> = emptyList(), + onHistoryItemClick: (String) -> Unit = {}, ) { val context = LocalContext.current val keyboardController = LocalSoftwareKeyboardController.current + var isExpanded by remember { mutableStateOf(false) } TextField( value = inputText, onValueChange = onValueChange, placeholder = { Text(context.getString(R.string.searchInputText_hint)) }, leadingIcon = { Icon( Icons.Sharp.Search, contentDescription = context.getString(R.string.search_icon_description), tint = MaterialTheme.colorScheme.primary, ) }, + trailingIcon = { + if (inputText.isNotEmpty()) { + IconButton(onClick = onClear) { + Icon( + Icons.Default.Clear, + contentDescription = "クリア", + tint = MaterialTheme.colorScheme.primary + ) + } + } + }, // ... 既存のコード ... ) + + if (isExpanded && searchHistory.isNotEmpty()) { + SearchHistoryDropdown( + history = searchHistory, + onItemClick = onHistoryItemClick + ) + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailViewModel.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/theme/Type.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/widget/ProgressCycle.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailViewModel.kt
🧰 Additional context used
📓 Learnings (3)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/widget/ProgressCycle.kt (1)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#27
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/widget/ProgressCycle.kt:0-0
Timestamp: 2025-01-20T10:19:59.546Z
Learning: The ProgressCycle composable in jp.co.yumemi.android.code_check follows accessibility best practices by providing content descriptions and using string resources for localization. The implementation includes semantic properties for both the container and the progress indicator.
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt (1)
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.
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (1)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#27
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt:0-0
Timestamp: 2025-01-20T10:20:25.249Z
Learning: For image loading in Compose using Coil, the team prefers using the system's launcher icon (`ic_launcher_foreground`) as a fallback/placeholder image instead of creating custom placeholder drawables.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (1)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/theme/Type.kt (1)
1-8: パッケージ構造とインポートが適切に設定されていますMaterial Design 3のタイポグラフィを実装するために必要なすべてのコンポーネントが正しくインポートされています。パッケージ構造も
core.presenter.themeとして適切に階層化されています。
.../main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt
Outdated
Show resolved
Hide resolved
.../main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt
Show resolved
Hide resolved
.../main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (6)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt (4)
59-128: 検索機能の実装について改善提案があります
searchJobのキャンセル処理が適切に実装されていますが、LaunchedEffect内でのメモリリークを防ぐため、クリーンアップも考慮すべきです。- エラー状態の管理が
isErrorとerrorMessageで二重に行われており、単一の状態として管理することを推奨します。以下の改善を提案します：
-var isError by remember { mutableStateOf(false) } +sealed class SearchState { + object Initial : SearchState() + object Loading : SearchState() + object Empty : SearchState() + data class Error(val message: Int) : SearchState() + data class Success(val data: List<RepositoryEntity>) : SearchState() +} +var searchState by remember { mutableStateOf<SearchState>(SearchState.Initial) }
130-160: パフォーマンス最適化の提案
LazyColumnの実装は適切ですが、以下の最適化を推奨します：
- リストアイテムのリコンポジションを最小限に抑えるため、
keyパラメータを追加- クリック可能な領域を明確にするため、
rippleエフェクトを追加-items(repositoryList.size) { index -> +items( + items = repositoryList, + key = { it.id } +) { repository -> Column( modifier = Modifier - .clickable { onTapping(repositoryList[index].id) } + .clickable( + interactionSource = remember { MutableInteractionSource() }, + indication = rememberRipple(bounded = true) + ) { onTapping(repository.id) }
162-221: ユーザーフィードバックの改善提案検索バーの実装は適切ですが、以下の改善を提案します：
- 検索中の状態をUIに反映
- 入力文字数の制限を追加
- 検索履歴機能の追加
TextField( value = inputText, onValueChange = onValueChange, + enabled = !isLoading, + maxLength = 100, // ... other properties )
303-348: プレビュー機能の拡張を推奨します現在のプレビュー実装に以下の追加を推奨します：
- ダークモードのプレビュー
- 異なる画面サイズのプレビュー
- 異なる状態（エラー、ロード中など）のプレビュー
-@Preview(showBackground = true) +@Preview(name = "Light Mode", showBackground = true) +@Preview(name = "Dark Mode", showBackground = true, uiMode = Configuration.UI_MODE_NIGHT_YES) +@Preview(name = "Tablet", showBackground = true, device = Devices.TABLET) @Composable fun EmptyStatePreview() { CodeCheckAppTheme { EmptyState() } }app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (2)
97-117: アクセシビリティの改善が必要ですローディング状態とコンテンツの状態変化をスクリーンリーダーに通知する必要があります。
以下の修正を提案します：
Box( modifier = Modifier .fillMaxSize() - .padding(8.dp), + .padding(8.dp) + .semantics { + contentDescription = if (isLoading) { + context.getString(R.string.loading_repository_details) + } else { + context.getString(R.string.repository_details_loaded) + } + }, ) {
strings.xmlに以下を追加してください：<string name="loading_repository_details">リポジトリの詳細を読み込み中</string> <string name="repository_details_loaded">リポジトリの詳細を表示中</string>
219-253: プレビュー機能の拡張を推奨しますダークテーマのプレビューを追加することで、両方のテーマでのUIの見た目を確認できます。
以下の修正を提案します：
-@Preview(showBackground = true) +@Preview(name = "Light Theme", showBackground = true) @Composable fun PreviewRepositoryOverviewCard() { RepositoryOverviewCard(/* ... */) } +@Preview(name = "Dark Theme", showBackground = true, uiMode = Configuration.UI_MODE_NIGHT_YES) +@Composable +fun PreviewRepositoryOverviewCardDark() { + RepositoryOverviewCard(/* ... */) +} -@Preview(showBackground = true) +@Preview(name = "Light Theme", showBackground = true) @Composable fun PreviewRepositoryStatsCard() { RepositoryStatsCard(/* ... */) } +@Preview(name = "Dark Theme", showBackground = true, uiMode = Configuration.UI_MODE_NIGHT_YES) +@Composable +fun PreviewRepositoryStatsCardDark() { + RepositoryStatsCard(/* ... */) +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt(1 hunks)app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
📓 Learnings (2)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (1)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#27
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt:0-0
Timestamp: 2025-01-20T10:20:25.249Z
Learning: For image loading in Compose using Coil, the team prefers using the system's launcher icon (`ic_launcher_foreground`) as a fallback/placeholder image instead of creating custom placeholder drawables.
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt (5)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#27
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt:46-93
Timestamp: 2025-01-20T10:32:50.258Z
Learning: The EmptyState composable in the app follows Material3 design guidelines and displays a search icon with a two-tier text structure (title and description) to guide users when no search results are found. It supports accessibility with content descriptions and uses the app's theme colors for visual consistency.
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#27
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt:46-93
Timestamp: 2025-01-20T10:28:36.346Z
Learning: The ErrorState composable in the app follows Material3 design guidelines and includes an error icon, error message, and retry button for handling error states in a user-friendly way. It supports accessibility with content descriptions and uses the app's theme colors.
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#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で対応することが望ましい。
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.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (3)
app/src/main/res/values/strings.xml (1)
15-29: 文字列リソースの実装が適切です！新しく追加された文字列リソースは、アプリケーションの国際化とアクセシビリティの要件を満たしており、UIコンポーネントの各状態を適切にサポートしています。
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/search/RepositorySearchScreen.kt (1)
223-301: エラー状態と空の状態の実装が適切です！
ErrorStateとEmptyStateの実装は以下の点で優れています：
- Material3デザインガイドラインに準拠
- アクセシビリティのサポートが適切
- ユーザーフレンドリーなエラーメッセージとアクション
- 一貫性のあるデザインパターン
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (1)
119-171: 画像読み込みの実装が適切ですチームの推奨事項に従って、システムのランチャーアイコンをプレースホルダーとして使用しており、適切に実装されています。
.../main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt
Show resolved
Hide resolved
.../main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (5)
app/src/main/res/values/strings.xml (2)
Line range hint
7-8: フォーマット指定子の一貫性を改善する必要があります既存の文字列では
%1$dを使用していますが、新しい文字列では単純な%dを使用しています。フォーマット指定子の使用方法を統一することで、将来の保守性が向上します。以下の変更を適用することを提案します：
- <string name="stars_count_format">Stars: %d</string> - <string name="forks_count_format">Forks: %d</string> - <string name="issues_count_format">Open Issues: %d</string> + <string name="stars_count_format">Stars: %1$d</string> + <string name="forks_count_format">Forks: %1$d</string> + <string name="issues_count_format">Open Issues: %1$d</string>Also applies to: 30-32
20-22: アクセシビリティの説明をより具体的にすることを推奨します現在のアイコンの説明は基本的すぎるかもしれません。スクリーンリーダーのユーザーにより良い体験を提供するため、各アイコンの目的や機能をより詳細に説明することを検討してください。
以下の変更を提案します：
- <string name="search_icon_description">検索アイコン</string> - <string name="search_bar_description">検索バー</string> - <string name="owner_icon_description">ユーザーアイコン</string> + <string name="search_icon_description">GitHubリポジトリを検索する</string> + <string name="search_bar_description">リポジトリ検索キーワードを入力</string> + <string name="owner_icon_description">リポジトリオーナーのプロフィール画像</string>Also applies to: 26-27
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (3)
49-54: エラー処理の改善を提案しますエラー状態の型安全性を向上させるため、以下の改善を提案します：
- エラーの種類を表す sealed class の導入
showSnackBarパラメータの型を具体的なエラータイプに変更sealed class RepositoryDetailError { data class NetworkError(val message: String) : RepositoryDetailError() data class NotFoundError(val message: String) : RepositoryDetailError() // 他のエラータイプを追加 } // 関数シグネチャの変更 fun RepositoryDetailScreen( toBack: () -> Unit, repositoryId: Int, viewModel: RepositoryDetailViewModel = hiltViewModel(), showSnackBar: (RepositoryDetailError) -> Unit, )
106-111: レスポンシブデザインの改善を提案します異なる画面サイズに対応するため、以下の改善を提案します：
Box( modifier = Modifier .fillMaxSize() - .padding(8.dp), + .padding( + horizontal = dimensionResource(R.dimen.screen_horizontal_margin), + vertical = dimensionResource(R.dimen.screen_vertical_margin) + ), )
dimens.xmlに以下を追加してください：<resources> <dimen name="screen_horizontal_margin">16dp</dimen> <dimen name="screen_vertical_margin">16dp</dimen> </resources>
191-191: アイコンのコンテンツ記述をリソース化してくださいアクセシビリティのため、以下のハードコードされた文字列をリソース化することを推奨します：
-contentDescription = "Stars", +contentDescription = context.getString(R.string.stars_icon_description), -contentDescription = "Forks", +contentDescription = context.getString(R.string.forks_icon_description), -contentDescription = "Open Issues", +contentDescription = context.getString(R.string.issues_icon_description),
strings.xmlに以下を追加してください：<string name="stars_icon_description">スター数</string> <string name="forks_icon_description">フォーク数</string> <string name="issues_icon_description">オープンな課題の数</string>Also applies to: 206-206, 221-221
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
📓 Learnings (1)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (1)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#27
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt:0-0
Timestamp: 2025-01-20T10:20:25.249Z
Learning: For image loading in Compose using Coil, the team prefers using the system's launcher icon (`ic_launcher_foreground`) as a fallback/placeholder image instead of creating custom placeholder drawables.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (4)
app/src/main/res/values/strings.xml (2)
23-23: エラーメッセージとEmpty状態の表示が適切に実装されていますユーザーフレンドリーなメッセージと明確な指示が提供されており、UXの観点から良い実装です。
Also applies to: 28-29
18-19: ローディング状態の表示が適切に実装されていますユーザーに対して現在の状態を明確に伝えており、Jetpack Composeの導入に合わせて適切に実装されています。
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (2)
122-174: 実装が適切です！
- スクロール可能なレイアウト
- 文字列リソースの適切な使用
- 画像読み込みの適切な実装（プレースホルダーとエラーハンドリング）
236-270: プレビュー実装が適切です！
- 適切なサンプルデータの使用
- 背景表示の有効化
- 複数のプレビューコンポーネントの提供
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
app/src/main/res/values/strings.xml (1)
23-23: 🛠️ Refactor suggestion文字列リソースの一貫性について
language_formatの文字列が英語で定義されていますが、他のリソースは日本語で定義されています。アプリの一貫性を保つため、日本語での定義に変更することを推奨します。- <string name="language_format">Language: %s</string> + <string name="language_format">使用言語: %s</string>
🧹 Nitpick comments (1)
app/src/main/res/values/strings.xml (1)
18-21: アクセシビリティの記述についてアクセシビリティのための説明文が適切に実装されており、スクリーンリーダーのユーザーにとって分かりやすい内容になっています。
ただし、より詳細な状態も伝えられるように、以下のような拡張を検討してください：
- <string name="loading_content_description">データを読み込んでいます</string> + <string name="loading_content_description">リポジトリのデータを読み込んでいます</string>Also applies to: 25-26
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
app/src/main/res/values/strings.xml(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (1)
app/src/main/res/values/strings.xml (1)
27-28: 空の状態のメッセージについて空の状態のメッセージが適切に日本語化されており、ユーザーにとって分かりやすい説明になっています。
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
app/src/main/res/values/strings.xml (3)
17-18: ローディング状態の文字列を最適化
searchingとloading_content_descriptionの文字列が似たような目的で使用されています。UXの一貫性を保つため、以下のいずれかの対応を検討してください：
- 両方の状態で同じメッセージを使用
- より具体的な使用場面の違いがある場合は、その違いが分かるような命名に変更
19-19: 検索アイコンの説明が重複
search_icon_descriptionとempty_state_icon_descriptionが同じ "検索アイコン" という説明を使用しています。空の状態を表すアイコンの場合、より具体的な説明に変更することを推奨します。例えば：
- <string name="empty_state_icon_description">検索アイコン</string> + <string name="empty_state_icon_description">検索結果が空の状態を表すアイコン</string>Also applies to: 26-26
23-23: フォーマット指定子の一貫性について既存の文字列（例：
stars_count）では%1$dという形式を使用していますが、新しいlanguage_formatでは単純な%sを使用しています。コードベース全体での一貫性を保つため、同じフォーマットスタイルの使用を推奨します：- <string name="language_format">Language: %s</string> + <string name="language_format">Language: %1$s</string>app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (3)
81-89: アクセシビリティの改善が必要ですエラー表示をスクリーンリーダーにより適切に伝えるため、以下の改善を提案します：
Column( verticalArrangement = Arrangement.Center, horizontalAlignment = Alignment.CenterHorizontally, modifier = Modifier .fillMaxWidth() - .fillMaxHeight(), + .fillMaxHeight() + .semantics(mergeDescendants = true) { + contentDescription = error + }, ) { Text( - text = error!! + text = error!!, + role = Role.Alert ) }
112-114: ローディング状態のユーザーフィードバックを改善ローディング中であることをユーザーにより明確に伝えるため、以下の改善を提案します：
if (isLoading) { + Box( + modifier = Modifier + .fillMaxSize() + .semantics { + contentDescription = context.getString(R.string.loading_repository_details) + } + ) { ProgressCycle() + } }
189-194: アイコンのアクセシビリティを改善アイコンの説明テキストをリソース化し、より具体的な説明を提供することを推奨します：
Icon( imageVector = Icons.Default.Star, - contentDescription = "Stars", + contentDescription = context.getString(R.string.stars_icon_description), modifier = Modifier.size(24.dp), tint = MaterialTheme.colorScheme.primary, ) Icon( imageVector = Icons.Default.Share, - contentDescription = "Forks", + contentDescription = context.getString(R.string.forks_icon_description), modifier = Modifier.size(24.dp), tint = MaterialTheme.colorScheme.primary, ) Icon( imageVector = Icons.Default.Report, - contentDescription = "Open Issues", + contentDescription = context.getString(R.string.issues_icon_description), modifier = Modifier.size(24.dp), tint = MaterialTheme.colorScheme.primary, )Also applies to: 204-209, 219-224
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt(1 hunks)app/src/main/res/values/strings.xml(1 hunks)
🧰 Additional context used
📓 Learnings (1)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (1)
Learnt from: harutiro
PR: harutiro/android-engineer-codecheck#27
File: app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt:0-0
Timestamp: 2025-01-20T10:20:25.249Z
Learning: For image loading in Compose using Coil, the team prefers using the system's launcher icon (`ic_launcher_foreground`) as a fallback/placeholder image instead of creating custom placeholder drawables.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: test
🔇 Additional comments (2)
app/src/main/kotlin/jp/co/yumemi/android/code_check/core/presenter/detail/RepositoryDetailScreen.kt (2)
236-270: プレビュー実装が適切ですサンプルデータを使用した両方のカードのプレビューが適切に実装されており、UIのテストカバレッジが確保されています。
1-270: 全体的な実装が適切ですJetpack Composeのベストプラクティスに従い、適切に構造化されたコードになっています。提案した改善点は主にアクセシビリティに関するものですが、現状の実装も十分に機能的です。
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
概要
UI をブラッシュアップを行うためにJetpackComposeを導入した。
関連Issue
このセクションでは、このPRが関連するIssueやタスクをリンクしてください。以下のように記述します。
変更点
このセクションでは、具体的な変更点や修正箇所を箇条書きでリストアップしてください。
テスト
このセクションでは、このPRに関連するテストケースやテスト方法を記載してください。
Summary by CodeRabbit
リリースノート
新機能
改善点
バグ修正
その他の変更