fit : homeScreen API 연동 ( 날씨 API 연동 완료 )#24
Hidden character warning
Conversation
Walkthrough홈 화면을 위치 권한 기반으로 재구성하고 Weather API를 쿼리 파라미터 방식으로 전환했으며, 디자인시스템 버튼/Weather 컴포넌트에 반응형 크기·상태 변경을 도입하고 여러 저장소·인터페이스 시그니처를 조정했습니다. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant HomeScreen
participant FusedLocationClient
participant HomeViewModel
participant WeatherRepository
participant BoardRepository
participant StudyRepository
User->>HomeScreen: 진입
HomeScreen->>FusedLocationClient: 위치 권한 요청 / 위치 조회
alt 권한 승인
FusedLocationClient-->>HomeScreen: 좌표 반환
HomeScreen->>HomeViewModel: loadWithLocation(fusedClient)
else 권한 거부
HomeScreen->>HomeViewModel: load(fallback coords)
end
HomeViewModel->>WeatherRepository: getWeather(latitude, longitude)
WeatherRepository-->>HomeViewModel: WeatherResult
HomeViewModel->>BoardRepository: getBestSinglePost()
BoardRepository-->>HomeViewModel: BestPostResult
HomeViewModel->>StudyRepository: getRecommendedStudies()
StudyRepository-->>HomeViewModel: StudyResultList
HomeViewModel-->>HomeScreen: uiState 업데이트
HomeScreen->>User: UI 렌더링
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🔇 Additional comments (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (4)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageButton.kt (1)
120-147:BlankButton의shape파라미터가 실제 배경(ShapeBox)에 반영되지 않음 (클리핑/배경 모양 불일치)Line 130에서는
clip(shape)를 쓰는데, Line 141에서ShapeBox(shape = SpotShapes.Hard)로 하드코딩되어 호출자가shape를 바꿔도 배경 모양이 따라가지 않습니다(시각/터치 영역 불일치 가능).제안 수정 diff
- ShapeBox( - shape = SpotShapes.Hard, + ShapeBox( + shape = shape, color = colors.bg, // 상태별 배경색 borderWidth = 0.dp, // 테두리 없음 borderColor = null, // 안전하게 보더 비활성화 modifier = Modifier .matchParentSize() // 부모 Box와 동일 크기 )feature/main/src/main/java/com/umcspot/spot/main/MainNavHost.kt (2)
67-83:onStudyClick/onStudyMoreClick가 no-op이면 UI 상 클릭만 먹는 상태가 될 수 있음
연동 전이라면 TODO를 명시하거나(또는 버튼/영역을 비활성)로 UX 혼선을 줄이는 쪽을 권장합니다.
44-48:popUpTo(0)백스택 클리어 방식은 안전하지 않은 패턴숫자 0을 popUpTo 대상으로 사용하는 것은 Android Navigation의 표준 패턴이 아니며, 타입 안전성과 유지보수성을 떨어뜨립니다. 코드베이스의 다른 곳에서는
popUpTo(Landing),popUpTo<RegisterStudy>같은 타입 안전 방식을 사용하고 있으며(MainNavigator.kt:192 참조),navController.graph.id도 이미 사용 중인 패턴입니다(BoardNavigation.kt:29).다음 중 하나로 수정하세요:
해결안 1: navController.graph.id 사용 (제안한 방식)
- val clearStackNavOptions = navOptions { - popUpTo(0) { inclusive = true } + fun createClearStackNavOptions() = navOptions { + popUpTo(navigator.navController.graph.id) { inclusive = true } launchSingleTop = true restoreState = false }해결안 2: Landing 라우트 사용 (코드베이스 패턴과 일치)
+ val clearStackNavOptions = navOptions { + popUpTo(Landing) { inclusive = true } launchSingleTop = true restoreState = false }data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt (1)
12-19:isSuccess검증 누락 +currentTime파싱 실패 시 NPE 위험코드베이스의 다른 저장소들(StudyRepositoryImpl, PostRepositoryImpl)은 모두
if (!response.isSuccess)로 명시적으로 검증하고 있으나, 이곳은 생략되어 있습니다. 서버가 "200 OK + isSuccess=false"로 응답하면runCatching이 예외를 캡처하지 못해 크래시합니다. 추가로toDomain()내currentTime.toLocalTimeOrNull()!!은 시간 파싱 실패 시 NPE를 던지는데, 이 역시 처리되지 않습니다.제안 수정안
override suspend fun getWeather(longitude : Long, latitude : Long): Result<WeatherResult> = runCatching { val response = weatherService.getWeather(longitude, latitude) + if (!response.isSuccess) error("Weather API failed: ${response.code} ${response.message}") response.result.toDomain() }.recoverCatching { // API 미연결/예외 시 더미로 복구 WeatherResult.dummyFrom() }
🤖 Fix all issues with AI agents
In
@data/board/src/main/java/com/umcspot/spot/board/repositoryimpl/BoardRepositoryImpl.kt:
- Around line 39-46: The getBestSinglePost implementation directly indexes
lists.hotPosts[0], which can throw IndexOutOfBounds; update getBestSinglePost to
safely access the first element (e.g., use firstOrNull()/getOrNull(0)) and
return a sensible fallback BestPostResult when the list is empty by chaining
recoverCatching (or similar) consistent with
getRecentBoard/getBestBoard/getFilteredPosts; also update the Log.e message
inside onFailure to "getBestSinglePost failed" and ensure Log.e logs the
exception passed to it.
In
@data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt:
- Around line 65-70: The error log in StudyRepositoryImpl.getRecommendedStudies
currently uses the wrong message text ("getPreferLocationStudies failed");
update the onFailure Log.e call to use the correct message
"getRecommendedStudies failed" so the log reflects the actual failing function
and aids debugging.
In @data/study/src/main/java/com/umcspot/spot/study/service/StudyService.kt:
- Around line 20-22: In HomeViewModel update the outdated error log strings that
still say "loadgetPopularStudies error" to reflect the migrated endpoint:
replace occurrences related to the call to StudyService.getRecommendedStudies
with a clear message such as "loadgetRecommendedStudies error" or
"loadRecommendedStudies error"; locate the two log calls in HomeViewModel that
surround the method invoking getRecommendedStudies (previously for popular
studies) and change both messages to the new recommended-study wording so logs
accurately match StudyService.getRecommendedStudies.
In @feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt:
- Around line 117-118: The log message incorrectly labels the error as
"loadgetPopularStudies error" even though the failure block is from
getRecommendedStudies(); update the Log.e call inside the onFailure lambda in
HomeViewModel (the failure handler for getRecommendedStudies()) to use a
correct, descriptive tag/message such as "loadRecommendedStudies error" so the
log reflects the actual operation that failed.
- Around line 64-68: In HomeViewModel's onFailure handlers (currently logging
only), restore/upsert the _uiState.update call to set UiState.Failure with the
error message (e.message ?: "날씨 불러오기 실패") so the UI leaves Loading and shows an
error; update every onFailure occurrence (the one around loadWeather and the
similar blocks at lines referenced in the comment) to call _uiState.update {
it.copy(weatherInfo = UiState.Failure(...)) } (or the appropriate field name)
rather than leaving the update commented out.
🧹 Nitpick comments (11)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageButton.kt (1)
80-97:XOUTLINETransparentState의 disabled 배경이 White라 “Transparent” 네이밍/의도와 어긋날 수 있음Line 82는
Color.Transparent인데, Line 86이White라 비활성화 시 갑자기 배경이 생깁니다. 디자인 의도(비활성은 흰 배경으로 보여주기)가 맞는지 확인하거나, 아니라면 disabled도 투명으로 맞추거나 상태명을 조정하는 게 안전합니다.gradle/libs.versions.toml (1)
135-138:kotlinx-coroutines-play-services:1.8.1은 Maven Central에 존재하며 호환됨Play Services
Task.await()연동(예: Location)을 위한 적절한 추가입니다. 프로젝트에서 coroutines 관련 라이브러리를 자주 함께 사용한다면bundles.coroutine에 포함하는 것을 검토해볼 만합니다.domain/home/src/main/java/com/umcspot/spot/home/repository/HomeRepository.kt (1)
4-6: 빈 인터페이스는 제거 권장
HomeRepository인터페이스가 모든 메서드가 제거되어 비어있습니다. 아키텍처 리팩토링으로 이 레포지토리가 더 이상 필요하지 않다면, 인터페이스와 구현체(HomeRepositoryImpl)를 완전히 제거하는 것이 좋습니다. 향후 사용 계획이 있다면 TODO 주석으로 의도를 명시해주세요.다음 스크립트로
HomeRepository사용처를 확인할 수 있습니다:#!/bin/bash # HomeRepository 참조 확인 echo "=== Checking HomeRepository usage ===" rg -nP --type=kotlin '\bHomeRepository\b' -C3 echo -e "\n=== Checking HomeRepositoryImpl usage ===" rg -nP --type=kotlin '\bHomeRepositoryImpl\b' -C3data/home/src/main/java/com/umcspot/spot/home/repositoryimpl/HomeRepositoryImpl.kt (1)
4-5: 미사용 import 제거
StudyRepository와WeatherRepositoryimport가 더 이상 사용되지 않으므로 제거해주세요.core/model/src/main/java/com/umcspot/spot/model/Global.kt (1)
3-10: core/model에 하드코딩된 한글 label은 i18n/문구변경 파급이 큼(매핑 레이어로 분리 권장)
QuickMenuType는 식별자만 유지하고, 표시 문자열은 UI(또는 디자인시스템)에서 매핑하는 구조가 확장에 유리합니다.data/study/src/main/java/com/umcspot/spot/study/datasourceimpl/StudyDataSourceImpl.kt (1)
23-26: 서비스 호출은 가급적 named argument로 고정하세요(특히null끼워 넣는 형태는 유지보수 리스크 큼)
studyService.getPreferLocationStudies(recruitingStatus, feeCategory, categories, null, sortType, ...)처럼 중간에null이 들어가면, 서비스 시그니처 변경/오버로드 추가 시 런타임까지 조용히 잘못 매핑될 가능성이 큽니다.getPreferLocationStudies(...)는 named argument로 바꾸는 걸 권장합니다.가능한 형태(시그니처에 맞춰 파라미터명만 정렬)
- studyService.getPreferLocationStudies(recruitingStatus, feeCategory, categories, null, sortType, cursor, size, regionCodes) + studyService.getPreferLocationStudies( + recruitingStatus = recruitingStatus, + feeCategory = feeCategory, + categories = categories, + /* TODO: 여기 null이 의미하는 파라미터명을 명시 */ + sortType = sortType, + cursor = cursor, + size = size, + regionCodes = regionCodes + )Also applies to: 37-47
data/weather/src/main/java/com/umcspot/spot/weather/datasourceimpl/WeatherDataSourceImpl.kt (1)
14-19: (longitude, latitude) 순서/단위 혼동 방지를 위해 여기서도 named argument 고려현재는 단순 포워딩이라 문제 없어 보이지만, 좌표 단위/순서가 중요한 API라
weatherService.getWeather(longitude = ..., latitude = ...)처럼 명시하면 실수 방지에 도움이 됩니다.feature/home/src/main/java/com/umcspot/spot/home/navigation/HomeNavigation.kt (1)
20-37:homeGraph(...)파라미터 확장: 콜백 묶기(옵션) 고려현재 단일 호출부는 모든 파라미터를 정상적으로 전달하고 있으며, 명명된 인자(named argument) 사용으로 순서 실수 위험이 최소화되어 있습니다. 다만 콜백이 5개로 많아진 점을 고려할 때,
HomeNavigationCallbacks같은 데이터 클래스로 묶으면 가독성 및 향후 확장성이 개선될 수 있습니다. (선택사항)feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt (1)
35-35: Fallback 좌표가 매직 넘버로 중복 정의됨.
1269780L to 375668L좌표가 두 곳에 하드코딩되어 있습니다. 상수로 추출하면 유지보수성이 향상됩니다.♻️ 리팩토링 제안
companion object { private val DEFAULT_LOCATION = 1269780L to 375668L // 서울시청 }Also applies to: 44-44
feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (2)
326-333: 참조 동등성 비교로 인한 잠재적 오류.
item != items.last()비교는 참조 동등성을 사용합니다. 동일한 데이터가 다른 객체 인스턴스로 존재할 경우 마지막 항목에도 Divider가 표시될 수 있습니다. 인덱스 기반 비교가 더 안전합니다.Lines 384-391에도 동일한 패턴이 있습니다.
♻️ 수정 제안
- items.forEach { item -> + items.forEachIndexed { index, item -> Spacer(Modifier.padding(screenHeightDp(4.dp))) StudyListItem( item = item, modifier = Modifier .fillMaxWidth(), onClick = { onItemClick(item) } ) - if (item != items.last()) { + if (index < items.lastIndex) { Spacer(Modifier.padding(screenHeightDp(4.dp))) HorizontalDivider( color = SpotTheme.colors.G300, thickness = 1.dp ) } }
591-597: getIconForType을 일반 함수로 변경 고려.이 함수는
@Composable로 선언되어 있지만,painterResource호출만 수행합니다. 아이콘 리소스 ID를 반환하는 일반 함수로 변경하고, 호출부에서painterResource를 사용하는 것이 더 명확할 수 있습니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (30)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageButton.ktcore/designsystem/src/main/java/com/umcspot/spot/designsystem/component/weather/Weather.ktcore/model/src/main/java/com/umcspot/spot/model/Global.ktdata/board/src/main/java/com/umcspot/spot/board/repositoryimpl/BoardRepositoryImpl.ktdata/home/src/main/java/com/umcspot/spot/home/mapper/HomeMapper.ktdata/home/src/main/java/com/umcspot/spot/home/repositoryimpl/HomeRepositoryImpl.ktdata/study/src/main/java/com/umcspot/spot/study/datasource/StudyDataSource.ktdata/study/src/main/java/com/umcspot/spot/study/datasourceimpl/StudyDataSourceImpl.ktdata/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.ktdata/study/src/main/java/com/umcspot/spot/study/service/StudyService.ktdata/weather/src/main/java/com/umcspot/spot/weather/datasource/WeatherDataSource.ktdata/weather/src/main/java/com/umcspot/spot/weather/datasourceimpl/WeatherDataSourceImpl.ktdata/weather/src/main/java/com/umcspot/spot/weather/mapper/WeatherMapper.ktdata/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.ktdata/weather/src/main/java/com/umcspot/spot/weather/service/WeatherService.ktdomain/board/src/main/java/com/umcspot/spot/domain/board/repository/BoardRepository.ktdomain/home/build.gradle.ktsdomain/home/src/main/java/com/umcspot/spot/home/model/Home.ktdomain/home/src/main/java/com/umcspot/spot/home/model/HomeResult.ktdomain/home/src/main/java/com/umcspot/spot/home/repository/HomeRepository.ktdomain/study/src/main/java/com/umcspot/spot/study/repository/StudyRepository.ktdomain/weather/src/main/java/com/umcspot/spot/weather/model/Weather.ktdomain/weather/src/main/java/com/umcspot/spot/weather/repository/WeatherRepository.ktfeature/home/build.gradle.ktsfeature/home/src/main/java/com/umcspot/spot/home/HomeScreen.ktfeature/home/src/main/java/com/umcspot/spot/home/HomeState.ktfeature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.ktfeature/home/src/main/java/com/umcspot/spot/home/navigation/HomeNavigation.ktfeature/main/src/main/java/com/umcspot/spot/main/MainNavHost.ktgradle/libs.versions.toml
💤 Files with no reviewable changes (6)
- domain/weather/src/main/java/com/umcspot/spot/weather/model/Weather.kt
- data/weather/src/main/java/com/umcspot/spot/weather/mapper/WeatherMapper.kt
- domain/home/build.gradle.kts
- domain/home/src/main/java/com/umcspot/spot/home/model/HomeResult.kt
- domain/home/src/main/java/com/umcspot/spot/home/model/Home.kt
- data/home/src/main/java/com/umcspot/spot/home/mapper/HomeMapper.kt
🧰 Additional context used
🧬 Code graph analysis (5)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageButton.kt (1)
core/ui/src/main/java/com/umcspot/spot/ui/extension/DimensionExt.kt (1)
screenWidthDp(18-23)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/weather/Weather.kt (1)
core/ui/src/main/java/com/umcspot/spot/ui/extension/DimensionExt.kt (2)
screenWidthDp(18-23)screenHeightDp(11-16)
feature/home/src/main/java/com/umcspot/spot/home/navigation/HomeNavigation.kt (1)
feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (1)
HomeScreen(72-132)
feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt (1)
feature/alert/src/main/java/com/umcspot/spot/alert/AlertViewmodel.kt (1)
load(34-57)
feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (4)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageButton.kt (1)
BlankButton(112-151)core/ui/src/main/java/com/umcspot/spot/ui/extension/DimensionExt.kt (2)
screenWidthDp(18-23)screenHeightDp(11-16)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/study/StudyItem.kt (1)
StudyListItem(40-100)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/weather/Weather.kt (1)
WeatherCard(33-108)
🔇 Additional comments (14)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageButton.kt (2)
101-110:resolveColors()우선순위에서checked가pressed를 덮어씀 → 눌림 피드백 소실 가능현재 Line 107이 Line 108보다 먼저라서, “선택된(checked) 상태에서 누르는 중”에도
selected만 적용됩니다. 의도라면 OK인데, 일반적으로는pressed가 우선되는 UX가 많아 확인이 필요합니다.(선택) pressed 우선으로 바꾸는 diff
): ImageButtonColors = when { !enabled -> disabled - checked -> selected isPressed -> pressed + checked -> selected else -> normal }
48-48: designsystem 모듈이core:ui의screenWidthDp에 의존 (모듈 경계/순환 의존 리스크)Preview 편의를 위해 Line 48처럼
com.umcspot.spot.ui.extension.screenWidthDp를 designsystem에서 직접 import하면, Gradle 의존성 방향에 따라 계층 역전/순환 참조가 생길 수 있습니다. (대안: designsystem 내부에 동일 유틸 제공, 또는 Preview 전용 파일을 ui 쪽으로 이동/분리)Also applies to: 160-173
domain/board/src/main/java/com/umcspot/spot/domain/board/repository/BoardRepository.kt (1)
3-3: LGTM! 인터페이스 정의가 명확합니다.새로운
getBestSinglePost()메서드가 기존 패턴을 잘 따르고 있으며, 도메인 계층의 Repository 인터페이스로 적절합니다.Also applies to: 21-22
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/weather/Weather.kt (2)
29-30: 반응형 크기 적용 잘 구현됨
screenWidthDp,screenHeightDp를 활용한 반응형 크기 적용이 일관되게 잘 구현되었습니다. 다양한 화면 크기에서 디자인 비율이 유지될 것으로 보입니다.Also applies to: 65-66, 80-80, 90-90, 92-92
101-101: 텍스트 스타일 변경 확인 필요메시지 텍스트의 스타일이
small_500 (fontSize 14.sp)에서regular_500로 변경되었습니다. 이 변경이 의도된 디자인 업데이트인지 확인해주세요.regular_500의 기본 폰트 크기가 이전 14.sp와 다를 수 있습니다.data/study/src/main/java/com/umcspot/spot/study/datasource/StudyDataSource.kt (1)
13-13: 인터페이스 변경사항 적절함
getRecommendedStudies()메서드로 통합하고createStudy()메서드를 추가한 것이 서비스 레이어 변경사항과 일치하며 적절합니다.Also applies to: 25-25
feature/home/build.gradle.kts (1)
8-15: Location/Play Services 의존성 추가로 권한/가용성/테스트 환경 영향 확인 필요
libs.google.location,libs.kotlinx.coroutines.play.services추가는 런타임 권한(예: 위치 권한), Play Services 미탑재 기기/에뮬레이터, 그리고 모듈 크기/의존성 충돌 가능성까지 확인이 필요합니다.data/weather/src/main/java/com/umcspot/spot/weather/service/WeatherService.kt (1)
13-17: 경도/위도 쿼리 파라미터 타입 확인 완료:Double→Long변환이 E6 스케일(1,000,000배)로 올바르게 구현되어 있습니다.
HomeViewModel.kt에서 FusedLocationProviderClient로부터 받은 Double 좌표값을(value * 1_000_000).toLong()으로 변환하는 것이 확인되었으며, 폴백값1269780L,375668L(서울시청)도 이 스케일을 따릅니다. 서버 계약이 스케일된 정수(E6) 형식이며, 클라이언트의 변환 로직이 정확하게 구현되어 있으므로 추가 조치가 불필요합니다.data/weather/src/main/java/com/umcspot/spot/weather/datasource/WeatherDataSource.kt (1)
6-8: I need the review comment to be rewritten. Please provide the review comment text within<review_comment>tags.domain/study/src/main/java/com/umcspot/spot/study/repository/StudyRepository.kt (1)
10-12: API 마이그레이션 완료됨 - 추가 작업 불필요코드베이스 전체를 확인한 결과, 구버전 메서드(
getPopularStudies,getRecommendStudies)는 완벽하게 제거되었으며, 신규 메서드getRecommendedStudies()는 인터페이스 정의에서부터 구현체(StudyRepositoryImpl, StudyDataSourceImpl), 서비스 계층(StudyService), 그리고 호출부(HomeViewModel)까지 일관되게 적용되어 있습니다. 마이그레이션이 전체 레이어에서 완벽하게 진행되었으므로 추가 확인은 불필요합니다.domain/weather/src/main/java/com/umcspot/spot/weather/repository/WeatherRepository.kt (1)
6-6: 위도/경도에 Long 타입 사용 검토 필요.위도/경도는 일반적으로
Double타입을 사용합니다.Long타입은 마이크로도(micro-degrees) 표현으로 보이나, 명시적인 문서화가 없으면 혼란을 줄 수 있습니다. 또한, 파라미터 순서가(longitude, latitude)인데, 지리 좌표 표준(ISO 6709)은(latitude, longitude)순서입니다.API 서버에서 기대하는 좌표 형식(타입, 순서, 단위)을 확인해 주세요.
feature/home/src/main/java/com/umcspot/spot/home/HomeState.kt (1)
8-13: LGTM!도메인별로
UiState를 분리한 구조가 명확하고, 각 섹션의 독립적인 로딩/에러 상태 관리가 가능합니다.feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (2)
449-449: 불필요한 캐스팅 검토 필요.
weatherState.data.weatherType as WeatherType?캐스팅이 사용되고 있습니다.WeatherResult.weatherType이 이미WeatherType?타입이라면 캐스팅이 불필요하고, 다른 타입이라면ClassCastException위험이 있습니다.
WeatherResult.weatherType의 실제 타입을 확인해 주세요.
100-111: LaunchedEffect 내 권한 요청 패턴 적절함.권한 확인 후 조건부로 요청하거나 위치 기반 로드를 수행하는 흐름이 명확합니다.
| override suspend fun getBestSinglePost( | ||
| ): Result<BestPostResult> = | ||
| runCatching { | ||
| val lists = boardService.getBestBoard(SortType.RECENT).result.toDomainList() | ||
| lists.hotPosts[0] | ||
| }.onFailure { e -> | ||
| Log.e("BoardRepository", "getBestBoard failed", e) | ||
| } |
There was a problem hiding this comment.
배열 인덱스 접근 시 경계 확인이 필요합니다.
Line 43에서 lists.hotPosts[0]에 직접 접근하고 있는데, hotPosts 리스트가 비어있을 경우 IndexOutOfBoundsException이 발생합니다.
추가 권장사항:
- 다른 메서드들(
getRecentBoard,getBestBoard,getFilteredPosts)과 달리 이 메서드에는recoverCatching을 통한 fallback 처리가 없습니다. 일관성을 위해 더미 데이터 복구 로직을 추가하는 것이 좋습니다. - Line 45의 로그 메시지가 "getBestBoard failed"인데 "getBestSinglePost failed"가 더 정확합니다.
🐛 안전한 접근 및 일관된 에러 처리를 위한 수정안
override suspend fun getBestSinglePost(
): Result<BestPostResult> =
runCatching {
val lists = boardService.getBestBoard(SortType.RECENT).result.toDomainList()
- lists.hotPosts[0]
+ lists.hotPosts.firstOrNull()
+ ?: throw NoSuchElementException("No hot posts available")
}.onFailure { e ->
- Log.e("BoardRepository", "getBestBoard failed", e)
+ Log.e("BoardRepository", "getBestSinglePost failed", e)
- }
+ }.recoverCatching {
+ bestPostDummies().hotPosts.first()
+ }🤖 Prompt for AI Agents
In
@data/board/src/main/java/com/umcspot/spot/board/repositoryimpl/BoardRepositoryImpl.kt
around lines 39 - 46, The getBestSinglePost implementation directly indexes
lists.hotPosts[0], which can throw IndexOutOfBounds; update getBestSinglePost to
safely access the first element (e.g., use firstOrNull()/getOrNull(0)) and
return a sensible fallback BestPostResult when the list is empty by chaining
recoverCatching (or similar) consistent with
getRecentBoard/getBestBoard/getFilteredPosts; also update the Log.e message
inside onFailure to "getBestSinglePost failed" and ensure Log.e logs the
exception passed to it.
data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt
Show resolved
Hide resolved
| .onFailure { e -> | ||
| Log.e("HomeViewModel", "loadWeather error", e) | ||
| // _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } | ||
| } | ||
| } |
There was a problem hiding this comment.
실패 시 UI 상태 업데이트가 주석 처리됨 — Loading 상태에 영구적으로 머물 수 있음.
onFailure 블록에서 UiState.Failure 업데이트가 주석 처리되어 있어, API 호출 실패 시 UI가 계속 로딩 스피너를 표시합니다. 사용자 경험을 위해 에러 상태를 표시하거나, 최소한 로딩을 종료해야 합니다.
이 패턴이 Lines 86-89, 104-108, 117-121, 133-137에도 반복됩니다.
🤖 Prompt for AI Agents
In @feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt around
lines 64 - 68, In HomeViewModel's onFailure handlers (currently logging only),
restore/upsert the _uiState.update call to set UiState.Failure with the error
message (e.message ?: "날씨 불러오기 실패") so the UI leaves Loading and shows an error;
update every onFailure occurrence (the one around loadWeather and the similar
blocks at lines referenced in the comment) to call _uiState.update {
it.copy(weatherInfo = UiState.Failure(...)) } (or the appropriate field name)
rather than leaving the update commented out.
| .onFailure { e -> | ||
| Log.e("HomeViewModel", "loadgetPopularStudies error", e) |
There was a problem hiding this comment.
로그 메시지 오류.
Line 118의 로그 태그가 "loadgetPopularStudies error"로 되어 있으나, 실제로는 getRecommendedStudies() 호출입니다. "loadRecommendedStudies error"로 수정해 주세요.
🤖 Prompt for AI Agents
In @feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt around
lines 117 - 118, The log message incorrectly labels the error as
"loadgetPopularStudies error" even though the failure block is from
getRecommendedStudies(); update the Log.e call inside the onFailure lambda in
HomeViewModel (the failure handler for getRecommendedStudies()) to use a
correct, descriptive tag/message such as "loadRecommendedStudies error" so the
log reflects the actual operation that failed.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (6)
feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (6)
90-111: 하드코딩된 좌표값에 대한 설명이 필요합니다.
1269780L, 375668L값이 무엇을 의미하는지 명확하지 않습니다. 서울의 기본 좌표로 추정되지만, 매직 넘버 대신 상수로 추출하고 주석을 추가하는 것이 좋습니다.♻️ 상수 추출 제안
// 파일 상단 또는 companion object에 추가 private const val DEFAULT_LONGITUDE = 1269780L // 서울 경도 (x10000 스케일) private const val DEFAULT_LATITUDE = 375668L // 서울 위도 (x10000 스케일)} { granted -> if (granted) { viewModel.loadWithLocation(fusedClient) } else { - viewModel.load(1269780L, 375668L) + viewModel.load(DEFAULT_LONGITUDE, DEFAULT_LATITUDE) } }
154-175: BlurEffect 사용 시 성능 고려 사항각 QuickMenu 아이템마다
BlurEffect를 사용한 그림자 렌더링은 저사양 기기에서 성능에 영향을 줄 수 있습니다. 현재 4개 아이템에 대해 이중 렌더링이 발생합니다.성능 이슈가 발생할 경우
elevation또는 사전 렌더링된 그림자 에셋 사용을 고려해 보세요.
317-334:items.last()반복 호출로 인한 비효율적인 비교
forEach루프 내에서items.last()를 매번 호출하면 O(n) 연산이 반복되어 전체 O(n²) 복잡도가 됩니다.forEachIndexed를 사용하여 인덱스 기반 비교로 개선할 수 있습니다.♻️ 개선 제안
- items.forEach { item -> - Spacer(Modifier.padding(screenHeightDp(4.dp))) - StudyListItem( - item = item, - modifier = Modifier - .fillMaxWidth(), - onClick = { onItemClick(item) } - ) - - if (item != items.last()) { - Spacer(Modifier.padding(screenHeightDp(4.dp))) - - HorizontalDivider( - color = SpotTheme.colors.G300, - thickness = 1.dp - ) - } - } + items.forEachIndexed { index, item -> + Spacer(Modifier.padding(screenHeightDp(4.dp))) + StudyListItem( + item = item, + modifier = Modifier.fillMaxWidth(), + onClick = { onItemClick(item) } + ) + + if (index < items.lastIndex) { + Spacer(Modifier.padding(screenHeightDp(4.dp))) + HorizontalDivider( + color = SpotTheme.colors.G300, + thickness = 1.dp + ) + } + }
374-392: 동일한items.last()비효율 패턴
PopularStudyNow와 동일한 패턴으로,forEachIndexed를 사용하여 개선하세요.♻️ 개선 제안
- items.forEach { item -> - Spacer(Modifier.padding(screenHeightDp(4.dp))) - - StudyListItem(...) - - if (item != items.last()) { + items.forEachIndexed { index, item -> + Spacer(Modifier.padding(screenHeightDp(4.dp))) + + StudyListItem(...) + + if (index < items.lastIndex) {
540-541:Color.Red대신 테마 색상 사용 권장에러 상태 텍스트에
Color.Red를 직접 사용하고 있습니다. 일관성을 위해SpotTheme.colors에서 정의된 에러 색상을 사용하는 것이 좋습니다.♻️ 테마 색상 적용
- is UiState.Failure -> Text("인기 스터디를 불러오지 못했어요", color = Color.Red) + is UiState.Failure -> Text("인기 스터디를 불러오지 못했어요", color = SpotTheme.colors.error) // 또는 적절한 테마 색상- Text("추천 스터디를 불러오지 못했어요", color = Color.Red) + Text("추천 스터디를 불러오지 못했어요", color = SpotTheme.colors.error)Also applies to: 574-577
427-435: 타입 안전성을 위한 명시적 타입 선언 고려
Triple을 사용하면 타입 정보가 유지되지만, line 449의 캐스트(weatherType as WeatherType?)가 필요한 상황입니다. 데이터 클래스를 사용하면 더 명확하고 안전해집니다.♻️ 데이터 클래스 사용 제안
private data class WeatherDisplayData( val temperature: Double?, val weatherType: WeatherType?, val currentTime: LocalTime ) // 사용 시 val weatherData = when (weatherState) { is UiState.Success -> WeatherDisplayData( weatherState.data.weatherTemp, weatherState.data.weatherType, weatherState.data.currentTime ) else -> WeatherDisplayData(null, null, LocalTime.now()) }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (5)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageButton.kt (1)
BlankButton(112-151)core/ui/src/main/java/com/umcspot/spot/ui/extension/DimensionExt.kt (2)
screenWidthDp(18-23)screenHeightDp(11-16)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/study/StudyItem.kt (1)
StudyListItem(40-100)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/weather/Weather.kt (1)
WeatherCard(33-108)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/Spinner.kt (1)
SpotSpinner(23-61)
🔇 Additional comments (2)
feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (2)
190-273: LGTM!
PopularPostNow컴포넌트의 구조와 클릭 핸들러가 적절하게 구현되어 있습니다.
593-599: LGTM!
getIconForType함수가 모든QuickMenuType을 처리하며 적절하게 구현되어 있습니다.
starshape7
changed the title
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @data/weather/src/main/java/com/umcspot/spot/weather/mapper/WeatherMapper.kt:
- Around line 49-51: The temp value must not default to 0.0 because a missing
temperature can incorrectly trigger WeatherType.COLD; change the t assignment to
preserve null (e.g., val t = temp) and update any downstream checks that use t
(the comparison that returns WeatherType.COLD) to explicitly handle null (e.g.,
if (t != null && t <= 9.0) { ... } else { /* handle null case: treat as
unknown/skip cold logic or pick an appropriate fallback */ }), referencing
rain1h, wind, temp and the WeatherType.COLD branch in WeatherMapper.kt so
missing temp is treated separately instead of assumed 0.0.
In
@domain/weather/src/main/java/com/umcspot/spot/weather/repository/WeatherRepository.kt:
- Line 6: The repository method signature suspend fun getWeather(longitude :
Double, latitude : Double): Result<WeatherResult> uses (longitude, latitude) but
should follow standard (latitude, longitude) ordering; update the function
signature to suspend fun getWeather(latitude: Double, longitude: Double):
Result<WeatherResult> and then update every call site accordingly—specifically
change the invocation in HomeViewModel (the call at line ~61) to pass (latitude,
longitude) in the new order to preserve correct semantics.
In @feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt:
- Around line 118-122: Update the Log.e call in HomeViewModel's onFailure
handler for loadRecommendedStudies: replace the incorrect message string
"loadgetPopularStudies error" with "loadRecommendedStudies error" so the log
accurately reflects the failure context (the affected code is the onFailure { e
-> Log.e("HomeViewModel", ... , e) block).
- Around line 57-70: The onFailure handlers in loadWeather, loadPosts and
reLoadRecommendedStudies leave _uiState in Loading because the UiState.Failure
update is commented out; restore or implement failure-state updates by updating
_uiState (the same pattern used for success) to UiState.Failure with a
user-facing message (e.g., e.message ?: "...") in each onFailure block and keep
the Log.e for diagnostics so the UI transitions out of Loading into an error
state.
🧹 Nitpick comments (16)
feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt (1)
37-50:@SuppressLint("MissingPermission")사용에 대한 주의사항
loadWithLocation함수에서 위치 권한 체크를HomeScreen에 위임하고 있습니다. 이 접근 방식은 동작하지만, ViewModel이 권한 상태를 모르기 때문에 직접 호출 시 런타임 예외가 발생할 수 있습니다. 권한이 부여되었을 때만 이 함수가 호출된다는 것을 문서화하거나, 권한 상태를 파라미터로 받는 것을 고려해 주세요.feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (3)
294-301:items.last()비교 대신forEachIndexed사용 권장매 반복마다
items.last()를 호출하면 리스트 끝을 찾기 위해 O(n) 비용이 발생합니다.forEachIndexed를 사용하면 인덱스로 마지막 항목을 O(1)에 판별할 수 있습니다.RecommendStudyNow(lines 323-330)에도 동일하게 적용 가능합니다.♻️ forEachIndexed 사용 제안
- items.forEach { item -> + items.forEachIndexed { index, item -> Spacer(Modifier.padding(screenHeightDp(4.dp))) StudyListItem( item = item, modifier = Modifier .fillMaxWidth(), onClick = { onItemClick(item) } ) - if (item != items.last()) { + if (index < items.lastIndex) { Spacer(Modifier.padding(screenHeightDp(4.dp))) HorizontalDivider( color = SpotTheme.colors.G300, thickness = 1.dp ) } }
509-510: 에러 상태 UI 일관성 개선 필요인기 스터디 실패 시(line 509) 텍스트만 표시되지만, 추천 스터디 실패 시(lines 571-581)는 새로고침 버튼이 함께 제공됩니다. 또한
Color.Red를 직접 사용하는 대신 테마 색상을 사용하면 일관성이 향상됩니다.♻️ 일관된 에러 UI 제안
- is UiState.Failure -> Text("인기 스터디를 불러오지 못했어요", color = Color.Red) + is UiState.Failure -> { + Text( + "인기 스터디를 불러오지 못했어요", + color = SpotTheme.colors.error // 또는 적절한 테마 색상 + ) + }
590-596:getIconForType함수 가시성 제한 고려이 함수는
HomeScreen.kt내부에서만 사용됩니다.private으로 선언하여 모듈 외부 노출을 방지하는 것이 좋습니다.♻️ private 선언 제안
@Composable -fun getIconForType(type: QuickMenuType) = when (type) { +private fun getIconForType(type: QuickMenuType) = when (type) { QuickMenuType.REGION -> painterResource(R.drawable.prefer_location) QuickMenuType.INTERESTS -> painterResource(R.drawable.heart_clear) QuickMenuType.RECRUITING -> painterResource(R.drawable.recruiting) QuickMenuType.BOARD -> painterResource(R.drawable.bulletin_board) }data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt (3)
9-9: 사용되지 않는 import 제거 권장
WeatherService가 import되었지만 실제로 사용되지 않습니다.WeatherDataSource를 통해 API를 호출하고 있으므로 이 import는 불필요합니다.♻️ 제안된 수정
-import com.umcspot.spot.weather.service.WeatherService
52-56: 에러 처리 전략 검토 필요실패 시 더미 데이터로 복구하는 방식은 사용자에게 잘못된 날씨 정보를 표시할 수 있습니다. 로그만 남기고 더미 데이터를 반환하면 실제 문제를 인지하기 어려울 수 있습니다.
에러 상황에서 UI에 적절한 에러 상태를 표시하거나, 캐시된 이전 데이터를 사용하는 것이 더 나을 수 있습니다.
38-48: 매직 스트링을 상수로 추출 고려
"03","NO_DATA","JSON"등의 값들을 companion object 상수로 추출하면 유지보수성이 향상됩니다.♻️ 제안된 수정
companion object { private const val DATA_TYPE_JSON = "JSON" private const val RESULT_CODE_NO_DATA = "03" private const val RESULT_MSG_NO_DATA = "NO_DATA" }core/buildconfig/src/main/java/com/umcspot/spot/buildconfig/di/WeatherConfigModule.kt (2)
4-6: 사용되지 않는 import 제거 권장.
BuildConfigFieldProvider와dagger.Binds가 import되어 있지만 실제로 사용되지 않습니다.♻️ 제안 수정
import com.umcspot.spot.buildconfig.impl.WeatherConfigFieldsProviderImpl -import com.umcspot.spot.common.BuildConfigFieldProvider import com.umcspot.spot.common.WeatherConfigFieldProvider -import dagger.Binds import dagger.Module import dagger.Provides
16-20: 더 효율적인 @BINDS 패턴 사용 권장
WeatherConfigFieldsProviderImpl은@Inject constructor()를 가지고 있어 올바르게 주입됩니다. 단순한 인터페이스-구현체 바인딩의 경우,@Provides대신@Binds를 사용하면 더 간결하고 효율적입니다.@InstallIn(SingletonComponent::class) @Module abstract class WeatherConfigModule { @Binds @Singleton abstract fun bindWeatherConfigFieldsProvider( impl: WeatherConfigFieldsProviderImpl ): WeatherConfigFieldProvider }build-logic/convention/src/main/java/com/umcspot/spot/convention/BuildConfig.kt (1)
18-19: 빈 문자열 기본값에 대한 런타임 검증 권장.
WEATHER_BASE_URL과WEATHER_TOKEN이local.properties에 설정되지 않은 경우 빈 문자열로 기본 설정됩니다. 이는 기존 패턴과 일관되지만, 날씨 API 호출 시 런타임에서 명확한 오류 대신 조용히 실패할 수 있습니다.
local.properties에 해당 키가 추가되었는지 확인하고, 필요하다면 README나 설정 문서에 필수 환경변수로 명시해주세요.data/alert/src/main/java/com/umcspot/spot/alert/di/AlertServiceModule.kt (1)
17-19: 함수명providesDummyService가 실제 반환 타입과 불일치합니다.
AlertService를 반환하지만 함수명이providesDummyService로 되어 있어 혼란을 줄 수 있습니다.providesAlertService로 이름 변경을 권장합니다.♻️ 제안 수정
- fun providesDummyService(@SpotApi retrofit: Retrofit): AlertService = retrofit.create( + fun providesAlertService(@SpotApi retrofit: Retrofit): AlertService = retrofit.create( AlertService::class.java )data/weather/src/main/java/com/umcspot/spot/weather/datasource/WeatherDataSource.kt (1)
5-6: 파라미터가 많아 Request DTO 사용을 고려해 보세요.6개의 파라미터가 있어 가독성과 유지보수성이 저하될 수 있습니다. 기상청 API 요청 형식에 맞춘 것으로 보이지만, 내부적으로는 request DTO로 캡슐화하면 더 깔끔합니다.
♻️ 선택적 개선안
data class WeatherRequest( val serviceKey: String, val dataType: String, val baseDate: String, val baseTime: String, val nx: Int, val ny: Int ) interface WeatherDataSource { suspend fun getWeather(request: WeatherRequest): WeatherResponseDto }현재 구현도 기능적으로는 문제없으며, 이 리팩토링은 선택사항입니다.
data/weather/src/main/java/com/umcspot/spot/weather/dto/response/WeatherResponseDto.kt (2)
3-7: 사용되지 않는 import가 있습니다.
WeatherType과LocalTime은 이 파일에서 사용되지 않습니다. 제거해 주세요.♻️ 제안하는 수정
package com.umcspot.spot.weather.dto.response import android.annotation.SuppressLint -import com.umcspot.spot.model.WeatherType import kotlinx.serialization.SerialName import kotlinx.serialization.Serializable -import java.time.LocalTime
9-14:@SuppressLint("UnsafeOptInUsageError")대신@OptIn사용을 권장합니다.모든 DTO 클래스에
@SuppressLint가 적용되어 있습니다. kotlinx.serialization의 실험적 API를 사용하는 경우,@OptIn(ExperimentalSerializationApi::class)어노테이션이 더 적절합니다.♻️ 제안하는 수정 예시
+import kotlinx.serialization.ExperimentalSerializationApi + +@OptIn(ExperimentalSerializationApi::class) -@SuppressLint("UnsafeOptInUsageError") @Serializable data class WeatherResponseDto(모든 DTO 클래스에 동일하게 적용하거나, 파일 레벨에서
@file:OptIn(ExperimentalSerializationApi::class)을 사용할 수 있습니다.Also applies to: 16-24, 26-34, 36-53, 55-60, 62-82
core/network/src/main/java/com/umcspot/spot/network/di/NetworkModule.kt (2)
46-54: Weather API 클라이언트에 타임아웃 설정을 추가하는 것을 권장합니다.외부 API(기상청) 호출 시 네트워크 지연이나 서버 응답 지연으로 인해 무한 대기가 발생할 수 있습니다. 타임아웃 설정을 추가해 주세요.
♻️ 제안하는 수정
+import java.util.concurrent.TimeUnit + @Provides @Singleton @WeatherApi fun providesWeatherOkHttpClient( loggingInterceptor: HttpLoggingInterceptor, ): OkHttpClient = OkHttpClient.Builder() .addInterceptor(loggingInterceptor) + .connectTimeout(30, TimeUnit.SECONDS) + .readTimeout(30, TimeUnit.SECONDS) + .writeTimeout(30, TimeUnit.SECONDS) .build()
56-61: 공유 Json 컨버터에ignoreUnknownKeys옵션 추가를 고려해 주세요.기상청 API 등 외부 API의 응답에 예상치 못한 필드가 추가될 경우 파싱 실패가 발생할 수 있습니다. 현재
providesConverterFactory()는 @spotapi와 @weatherAPI 양쪽 모두에서 사용되는 공유 컨버터이므로,Json { ignoreUnknownKeys = true }를 적용하면 외부 API 변경에 대한 견고성을 높일 수 있습니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (27)
build-logic/convention/src/main/java/com/umcspot/spot/convention/BuildConfig.ktcore/buildconfig/src/main/java/com/umcspot/spot/buildconfig/di/WeatherConfigModule.ktcore/buildconfig/src/main/java/com/umcspot/spot/buildconfig/impl/WeatherConfigFieldsProviderImpl.ktcore/common/src/main/java/com/umcspot/spot/common/WeatherConfigFieldProvider.ktcore/common/src/main/java/com/umcspot/spot/common/WeatherConfigFields.ktcore/designsystem/src/main/java/com/umcspot/spot/designsystem/component/weather/Weather.ktcore/designsystem/src/main/java/com/umcspot/spot/designsystem/component/weather/WeatherPreview.ktcore/model/src/main/java/com/umcspot/spot/model/Global.ktcore/network/src/main/java/com/umcspot/spot/network/di/NetworkModule.ktcore/network/src/main/java/com/umcspot/spot/network/di/Qualifier.ktdata/alert/src/main/java/com/umcspot/spot/alert/di/AlertServiceModule.ktdata/board/src/main/java/com/umcspot/spot/board/di/BoardServiceModule.ktdata/home/src/main/java/com/umcspot/spot/home/di/HomeServiceModule.ktdata/login/src/main/java/com/umcspot/spot/login/di/LoginServiceModule.ktdata/post/src/main/java/com/umcspot/spot/post/di/PostServiceModule.ktdata/study/src/main/java/com/umcspot/spot/study/di/StudyServiceModule.ktdata/user/src/main/java/com/umcspot/spot/user/di/UserServiceModule.ktdata/weather/src/main/java/com/umcspot/spot/weather/datasource/WeatherDataSource.ktdata/weather/src/main/java/com/umcspot/spot/weather/datasourceimpl/WeatherDataSourceImpl.ktdata/weather/src/main/java/com/umcspot/spot/weather/di/WeatherServiceModule.ktdata/weather/src/main/java/com/umcspot/spot/weather/dto/response/WeatherResponseDto.ktdata/weather/src/main/java/com/umcspot/spot/weather/mapper/WeatherMapper.ktdata/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.ktdata/weather/src/main/java/com/umcspot/spot/weather/service/WeatherService.ktdomain/weather/src/main/java/com/umcspot/spot/weather/repository/WeatherRepository.ktfeature/home/src/main/java/com/umcspot/spot/home/HomeScreen.ktfeature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt
💤 Files with no reviewable changes (1)
- core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/weather/WeatherPreview.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/weather/Weather.kt
🧰 Additional context used
🧬 Code graph analysis (2)
feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt (1)
feature/alert/src/main/java/com/umcspot/spot/alert/AlertViewmodel.kt (1)
load(34-57)
feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (3)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageButton.kt (1)
BlankButton(112-151)core/ui/src/main/java/com/umcspot/spot/ui/extension/DimensionExt.kt (2)
screenWidthDp(18-23)screenHeightDp(11-16)feature/study/src/main/java/com/umcspot/spot/study/recruiting/RecruitingStudyFilterScreen.kt (1)
getIconForType(276-280)
🔇 Additional comments (23)
core/common/src/main/java/com/umcspot/spot/common/WeatherConfigFieldProvider.kt (1)
1-5: LGTM!Weather 설정 제공을 위한 인터페이스가 깔끔하게 정의되어 있습니다. DI를 통한 설정 분리 패턴을 잘 따르고 있습니다.
core/network/src/main/java/com/umcspot/spot/network/di/Qualifier.kt (1)
1-11: LGTM!Spot API와 Weather API를 구분하기 위한 Qualifier 어노테이션이 올바르게 정의되어 있습니다.
AnnotationRetention.BINARY설정도 Dagger/Hilt에 적합합니다.core/model/src/main/java/com/umcspot/spot/model/Global.kt (2)
3-10: LGTM!
QuickMenuTypeenum에label속성이 추가되어 UI에서 직접 활용할 수 있게 되었습니다.
12-12:HEAVYRAIN제거는 다른 코드에 영향을 주지 않습니다.코드베이스 전체를 검색한 결과,
HEAVYRAIN타입에 대한 참조가 없습니다. 안전하게 제거되었습니다.feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (2)
100-111: LGTM!위치 권한 요청 로직이
LaunchedEffect(Unit)으로 적절하게 구현되어 있습니다. 권한 허용 시loadWithLocation, 거부 시 기본 좌표로load를 호출하는 분기 처리가 잘 되어 있습니다.
348-364: 스크롤 복원 로직 구현이 잘 되어 있습니다.
rememberSaveable을 사용한LazyListState저장과LaunchedEffect를 통한 스크롤 위치 복원 로직이 잘 구현되어 있습니다. 추천 스터디 새로고침 시 사용자 경험을 개선하는 좋은 패턴입니다.data/board/src/main/java/com/umcspot/spot/board/di/BoardServiceModule.kt (1)
17-19: LGTM!@spotapi 한정자를 사용하여 올바른 Retrofit 인스턴스를 주입받도록 변경되었습니다. 다른 서비스 모듈들과 일관된 DI 패턴입니다.
core/common/src/main/java/com/umcspot/spot/common/WeatherConfigFields.kt (1)
1-6: LGTM!날씨 API 설정값을 담는 불변 데이터 클래스입니다.
WeatherConfigFieldProvider와 함께 DI를 통해 설정값을 제공하는 깔끔한 구조입니다.data/login/src/main/java/com/umcspot/spot/login/di/LoginServiceModule.kt (1)
17-18: LGTM!@spotapi 한정자를 통해 올바른 Retrofit 인스턴스가 주입됩니다. 프로젝트 전반의 DI 패턴과 일관성을 유지합니다.
data/user/src/main/java/com/umcspot/spot/user/di/UserServiceModule.kt (1)
17-19: LGTM!@spotapi 한정자가 적용되어 다른 서비스 모듈들과 동일한 DI 패턴을 따릅니다.
data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt (1)
70-110: LGTM!기상청 격자 좌표 변환 알고리즘(Lambert Conformal Conic projection)이 정확하게 구현되었습니다. 상수값들이 기상청 공식 문서와 일치합니다.
data/study/src/main/java/com/umcspot/spot/study/di/StudyServiceModule.kt (1)
3-3: LGTM!
@SpotApiqualifier 추가가 다른 서비스 모듈들과 일관되게 적용되었습니다. DI 바인딩이 올바르게 구성되어 있습니다.Also applies to: 17-19
data/home/src/main/java/com/umcspot/spot/home/di/HomeServiceModule.kt (1)
4-4: LGTM!
@SpotApiqualifier가 올바르게 적용되어 HomeService가 적절한 Retrofit 인스턴스를 사용하도록 구성되었습니다.Also applies to: 17-19
build-logic/convention/src/main/java/com/umcspot/spot/convention/BuildConfig.kt (1)
54-64: 날씨 설정 buildConfigField 추가 확인.buildConfigField 추가가 기존 패턴과 일관되게 구현되었습니다.
data/weather/src/main/java/com/umcspot/spot/weather/di/WeatherServiceModule.kt (1)
3-3: LGTM! Weather API 전용 Retrofit 인스턴스가 올바르게 바인딩되었습니다.
@WeatherApiqualifier를 통해 날씨 API 전용 Retrofit 인스턴스(별도 base URL 및 설정)가 주입됩니다. Spot API와 Weather API를 분리하는 아키텍처가 잘 적용되었습니다.Also applies to: 17-19
core/buildconfig/src/main/java/com/umcspot/spot/buildconfig/impl/WeatherConfigFieldsProviderImpl.kt (2)
9-15: LGTM! 깔끔한 Provider 구현입니다.
@Inject생성자를 사용한 간결한 구현이며, DI 모듈과의 통합이 명확합니다.
3-4: WEATHER_TOKEN 관리는 보안 모범 사례를 따르고 있습니다.
WEATHER_TOKEN은build-logic/convention의BuildConfig.kt에서gradleLocalProperties를 통해local.properties로부터 로드되므로, 버전 관리에 직접 포함되지 않습니다.local.properties는.gitignore에 properly 등록되어 있어 secrets 노출의 위험이 없습니다.data/post/src/main/java/com/umcspot/spot/post/di/PostServiceModule.kt (1)
3-3: 모든 서비스 모듈에서 일관되게 적용된 DI qualifier 패턴입니다.
@SpotApiqualifier를 사용하여 올바른 Retrofit 인스턴스를 주입받도록 변경되었습니다. StudyServiceModule, UserServiceModule, HomeServiceModule, AlertServiceModule, BoardServiceModule, LoginServiceModule과 동일한 패턴으로 통일되어 있습니다.data/weather/src/main/java/com/umcspot/spot/weather/mapper/WeatherMapper.kt (2)
42-86: 날씨 타입 결정 로직이 잘 구현되어 있습니다.각 조건별로 주석이 명확하게 달려 있고, PTY/RN1/WSD/T1H 값에 따른 분기 처리가 적절합니다.
9-11: 직접 체이닝 접근은 구조적으로 안전합니다만, API 응답 구조 변경에 대비한 방어 처리를 고려하세요.
response.body.items.item체이닝 접근은 모든 중간 필드가 non-nullable로 정의되어 있어 NullPointerException 자체는 발생하지 않습니다. 다만 API 응답 구조가 예상과 다르거나 필드가 누락된 경우 역직렬화 단계에서 SerializationException이 발생할 수 있습니다. 외부 API와의 연동에서 이를 대비한 try-catch 또는 Result 타입 등의 에러 핸들링 추가를 권장합니다.Likely an incorrect or invalid review comment.
data/weather/src/main/java/com/umcspot/spot/weather/service/WeatherService.kt (1)
10-20: 기상청 API 연동 구현이 적절합니다.
ServiceKey에encoded = true설정으로 URL 인코딩된 API 키를 올바르게 처리하고 있습니다. 기본값 설정도 적절합니다.data/weather/src/main/java/com/umcspot/spot/weather/datasourceimpl/WeatherDataSourceImpl.kt (1)
13-28: 구현이 간결하고 명확합니다.
WeatherService의pageNo,numOfRows파라미터가 DataSource에서 노출되지 않아 기본값(1, 1000)이 항상 사용됩니다. 의도된 설계라면 문제없습니다.core/network/src/main/java/com/umcspot/spot/network/di/NetworkModule.kt (1)
77-87: Weather API Retrofit 구성이 적절합니다.
@WeatherApi한정자를 사용한 분리된 Retrofit 인스턴스 구성이 깔끔합니다.WeatherConfigFieldProvider를 통한 base URL 주입도 적절합니다.
| val r = (rain1h ?: 0.0).coerceAtLeast(0.0) | ||
| val w = (wind ?: 0.0).coerceAtLeast(0.0) | ||
| val t = temp ?: 0.0 |
There was a problem hiding this comment.
temp 기본값 0.0으로 인해 잘못된 날씨 타입이 결정될 수 있습니다.
temp가 null일 때 기본값 0.0이 설정되면, 이후 t <= 9.0 조건에서 WeatherType.COLD가 반환됩니다. 기온 데이터가 없는 경우 실제로는 추운 날씨가 아닐 수 있으므로, null 처리를 별도로 하거나 기본값을 재고해야 합니다.
🐛 제안하는 수정 방안
- val r = (rain1h ?: 0.0).coerceAtLeast(0.0)
- val w = (wind ?: 0.0).coerceAtLeast(0.0)
- val t = temp ?: 0.0
+ val r = (rain1h ?: 0.0).coerceAtLeast(0.0)
+ val w = (wind ?: 0.0).coerceAtLeast(0.0)
+
+ // temp가 null이면 기온 기반 판단(COLD, HOT)을 건너뛰고 SUNNY로 처리
+ if (temp == null) {
+ if ((pty == "1" || pty == "2" || pty == "4") && r > 0.0) return WeatherType.RAIN
+ if (pty == "3") return WeatherType.SNOW
+ if (w >= 8.0) return WeatherType.WIND
+ return WeatherType.SUNNY
+ }
+ val t = temp🤖 Prompt for AI Agents
In @data/weather/src/main/java/com/umcspot/spot/weather/mapper/WeatherMapper.kt
around lines 49 - 51, The temp value must not default to 0.0 because a missing
temperature can incorrectly trigger WeatherType.COLD; change the t assignment to
preserve null (e.g., val t = temp) and update any downstream checks that use t
(the comparison that returns WeatherType.COLD) to explicitly handle null (e.g.,
if (t != null && t <= 9.0) { ... } else { /* handle null case: treat as
unknown/skip cold logic or pick an appropriate fallback */ }), referencing
rain1h, wind, temp and the WeatherType.COLD branch in WeatherMapper.kt so
missing temp is treated separately instead of assumed 0.0.
data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt
Outdated
Show resolved
Hide resolved
|
|
||
| interface WeatherRepository { | ||
| suspend fun getWeather(request : Weather): Result<WeatherResult> | ||
| suspend fun getWeather(longitude : Double, latitude : Double): Result<WeatherResult> |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the WeatherRepository file and check its exact location and content
find . -type f -name "WeatherRepository.kt" 2>/dev/null | head -20Repository: SPOTeam/Android-V2
Length of output: 149
🏁 Script executed:
# Search for all calls to getWeather method in the codebase
rg -n "getWeather\(" --type=kotlin -B 2 -A 3Repository: SPOTeam/Android-V2
Length of output: 9139
🏁 Script executed:
# Also check for any weather-related API calls to understand the convention used
rg -n "longitude|latitude" --type=kotlin -B 1 -A 1 | head -50Repository: SPOTeam/Android-V2
Length of output: 3760
파라미터 순서 수정 필요: 지리적 좌표 관례에 따라 latitude를 먼저 배치하세요
현재 (longitude, latitude) 순서는 Google Maps API, Android Location API 등의 표준 관례인 (latitude, longitude) 순서를 따르지 않습니다. 이 변경 시 HomeViewModel.kt 61번 줄의 호출부도 함께 수정되어야 합니다.
🔧 필수 수정 사항
WeatherRepository.kt:
- suspend fun getWeather(longitude : Double, latitude : Double): Result<WeatherResult>
+ suspend fun getWeather(latitude: Double, longitude: Double): Result<WeatherResult>HomeViewModel.kt 61번 줄:
- weatherRepository.getWeather(longitude, latitude)
+ weatherRepository.getWeather(latitude, longitude)📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| suspend fun getWeather(longitude : Double, latitude : Double): Result<WeatherResult> | |
| suspend fun getWeather(latitude: Double, longitude: Double): Result<WeatherResult> |
🤖 Prompt for AI Agents
In
@domain/weather/src/main/java/com/umcspot/spot/weather/repository/WeatherRepository.kt
at line 6, The repository method signature suspend fun getWeather(longitude :
Double, latitude : Double): Result<WeatherResult> uses (longitude, latitude) but
should follow standard (latitude, longitude) ordering; update the function
signature to suspend fun getWeather(latitude: Double, longitude: Double):
Result<WeatherResult> and then update every call site accordingly—specifically
change the invocation in HomeViewModel (the call at line ~61) to pass (latitude,
longitude) in the new order to preserve correct semantics.
| fun loadWeather(longitude : Double, latitude : Double) { | ||
| _uiState.update { it.copy(weatherInfo = UiState.Loading) } | ||
|
|
||
| viewModelScope.launch { | ||
| weatherRepository.getWeather(longitude, latitude) | ||
| .onSuccess { weather -> | ||
| _uiState.update { it.copy(weatherInfo = UiState.Success(weather)) } | ||
| } | ||
| .onFailure { e -> | ||
| Log.e("HomeViewModel", "loadWeather error", e) | ||
| // _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
실패 상태 업데이트가 주석 처리되어 있습니다.
loadWeather, loadPosts, reLoadRecommendedStudies 메서드들에서 onFailure 시 상태 업데이트가 주석 처리되어 있어, 실패 시 UI가 Loading 상태에서 영원히 멈추게 됩니다. 사용자에게 오류 상태를 표시하려면 주석을 해제하거나 적절한 에러 처리를 추가해 주세요.
🐛 실패 상태 업데이트 활성화 제안
.onFailure { e ->
Log.e("HomeViewModel", "loadWeather error", e)
-// _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) }
+ _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) }
}🤖 Prompt for AI Agents
In @feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt around
lines 57 - 70, The onFailure handlers in loadWeather, loadPosts and
reLoadRecommendedStudies leave _uiState in Loading because the UiState.Failure
update is commented out; restore or implement failure-state updates by updating
_uiState (the same pattern used for success) to UiState.Failure with a
user-facing message (e.g., e.message ?: "...") in each onFailure block and keep
the Log.e for diagnostics so the UI transitions out of Loading into an error
state.
| .onFailure { e -> | ||
| Log.e("HomeViewModel", "loadgetPopularStudies error", e) | ||
|
|
||
| // _uiState.update { it.copy(recommendStudies = UiState.Failure(e.message ?: "추천 스터디 실패")) } | ||
| } |
There was a problem hiding this comment.
로그 메시지 오타
Line 119의 로그 메시지가 "loadgetPopularStudies error"로 되어 있는데, 이 위치는 추천 스터디 로딩 실패 시점입니다. "loadRecommendedStudies error"로 수정해 주세요.
📝 로그 메시지 수정
- Log.e("HomeViewModel", "loadgetPopularStudies error", e)
+ Log.e("HomeViewModel", "loadRecommendedStudies error", e)🤖 Prompt for AI Agents
In @feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt around
lines 118 - 122, Update the Log.e call in HomeViewModel's onFailure handler for
loadRecommendedStudies: replace the incorrect message string
"loadgetPopularStudies error" with "loadRecommendedStudies error" so the log
accurately reflects the failure context (the affected code is the onFailure { e
-> Log.e("HomeViewModel", ... , e) block).
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In
@data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt:
- Around line 58-66: calculateBaseDateTime currently returns the raw now
formatted time which causes NO_DATA; change it to map the current LocalDateTime
down to the most recent valid 발표시각 from the set
{02:00,05:00,08:00,11:00,14:00,17:00,20:00,23:00} (if now is before 02:00 use
previous day 23:00), then return the corresponding baseDate and baseTime
strings; also ensure any retry logic that subtracts an hour uses
calculateBaseDateTime(now.minusHours(1)) or re-evaluates against the allowed
times rather than naively subtracting 1 hour so retries land on a valid 발표시각.
In @feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt:
- Around line 46-49: The onFailureListener calls load(FALLBACK_LONGITUDE,
FALLBACK_LATITUDE) but the load(latitude, longitude) function expects latitude
first, so swap the arguments: call load(FALLBACK_LATITUDE, FALLBACK_LONGITUDE)
inside the addOnFailureListener block (in HomeViewModel’s onFailureListener) to
ensure the fallback coordinates are passed in the correct order.
🧹 Nitpick comments (4)
data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt (1)
20-21: 미사용 private 함수
setRecommendDummies가 정의되어 있지만 어디에서도 사용되지 않습니다.getRecommendedStudies에서 폴백으로 사용할 의도였다면 연결이 필요하고, 그렇지 않으면 제거를 고려하세요.data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt (1)
9-9: 미사용 import
WeatherService가 import되어 있지만 코드에서 사용되지 않습니다.WeatherDataSource로 대체되었으므로 제거하세요.🧹 제안된 수정
-import com.umcspot.spot.weather.service.WeatherServicefeature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (2)
294-301: 참조 비교 대신 인덱스 기반 비교 권장
item != items.last()는 참조 비교를 수행합니다. 만약 리스트에 동일한 객체가 중복되면 의도치 않게 divider가 표시되지 않을 수 있습니다. 인덱스 기반 비교가 더 안전합니다.♻️ 제안된 수정 (PopularStudyNow 예시)
- items.forEach { item -> + items.forEachIndexed { index, item -> Spacer(Modifier.padding(screenHeightDp(4.dp))) StudyListItem( item = item, modifier = Modifier .fillMaxWidth(), onClick = { onItemClick(item) } ) - if (item != items.last()) { + if (index < items.lastIndex) { Spacer(Modifier.padding(screenHeightDp(4.dp))) HorizontalDivider( color = SpotTheme.colors.G300, thickness = 1.dp ) } }Also applies to: 323-330
607-613: 함수 위치 재고려
getIconForType은 파일 최하단에 위치해 있습니다. Composable 함수이므로QuickMenu와 함께 위치하거나, 별도 유틸 파일로 분리하는 것이 가독성에 좋습니다.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
data/board/src/main/java/com/umcspot/spot/board/repositoryimpl/BoardRepositoryImpl.ktdata/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.ktdata/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.ktdomain/weather/src/main/java/com/umcspot/spot/weather/repository/WeatherRepository.ktfeature/home/src/main/java/com/umcspot/spot/home/HomeScreen.ktfeature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (1)
- data/board/src/main/java/com/umcspot/spot/board/repositoryimpl/BoardRepositoryImpl.kt
🧰 Additional context used
🧬 Code graph analysis (2)
feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt (1)
feature/alert/src/main/java/com/umcspot/spot/alert/AlertViewmodel.kt (1)
load(34-57)
feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (6)
core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/button/ImageButton.kt (1)
BlankButton(112-151)core/ui/src/main/java/com/umcspot/spot/ui/extension/DimensionExt.kt (2)
screenWidthDp(18-23)screenHeightDp(11-16)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/study/StudyItem.kt (1)
StudyListItem(40-100)data/weather/src/main/java/com/umcspot/spot/weather/dto/response/WeatherResponseDto.kt (1)
item(55-60)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/weather/Weather.kt (1)
WeatherCard(33-110)core/designsystem/src/main/java/com/umcspot/spot/designsystem/component/Spinner.kt (1)
SpotSpinner(23-61)
🔇 Additional comments (6)
data/study/src/main/java/com/umcspot/spot/study/repositoryimpl/StudyRepositoryImpl.kt (1)
65-70: 에러 복구 패턴 불일치
getRecruitingStudies는recoverCatching으로 더미 데이터 폴백을 제공하지만,getRecommendedStudies는 실패 시 로깅만 수행합니다. 이로 인해 호출자 측에서Result.failure를 받게 됩니다.의도된 동작인지 확인 필요합니다. 일관성을 위해 동일한 폴백 패턴 적용을 고려하세요.
domain/weather/src/main/java/com/umcspot/spot/weather/repository/WeatherRepository.kt (1)
5-7: LGTM!인터페이스가
Weather객체 대신 primitive 파라미터(latitude,longitude)를 사용하도록 간결하게 변경되었습니다. API 표면이 명확해졌습니다.data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt (1)
70-110: LGTM!Lambert Conformal Conic 투영법을 사용한 위경도→격자 변환 로직이 기상청 공식 문서와 일치합니다.
feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt (1)
65-68: Failure 상태 업데이트 주석 처리됨모든
onFailure콜백에서 UI 상태 업데이트가 주석 처리되어 있습니다. 현재는 로딩 중에 실패하면 UI가 계속Loading상태로 남게 됩니다.의도된 동작인지 확인하세요.
recoverCatching으로 더미 데이터를 반환하는 패턴이라면 괜찮지만, 그렇지 않은 경우 사용자에게 무한 로딩으로 보일 수 있습니다.Also applies to: 87-90, 105-109, 118-122
feature/home/src/main/java/com/umcspot/spot/home/HomeScreen.kt (2)
100-111: LGTM!위치 권한 플로우가 올바르게 구현되었습니다.
LaunchedEffect(Unit)로 최초 진입 시 권한 확인 후 요청하고, 결과에 따라 적절히 분기합니다.
348-364: LGTM!
rememberSaveable과LazyListState.Saver를 사용한 스크롤 위치 복원 로직이 잘 구현되었습니다. 새로고침 후 사용자 스크롤 위치를 유지하는 UX가 좋습니다.
| private fun calculateBaseDateTime(now: LocalDateTime): Pair<String, String> { | ||
| val formatterDate = DateTimeFormatter.ofPattern("yyyyMMdd") | ||
| val formatterTime = DateTimeFormatter.ofPattern("HHmm") | ||
|
|
||
| val baseDate = now.format(formatterDate) | ||
| val baseTime = now.format(formatterTime) | ||
|
|
||
| return baseDate to baseTime | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
기상청 단기예보 API base_time 발표시각 요구사항
💡 Result:
요약: 기상청 단기/초단기 예보 API의 base_time(발표시각)은 임의의 현재시각을 넣을 수 없고, 기상청이 정한 발표시각(고정 시각)만 허용됩니다.
- 초단기(예: getUltraSrtFcst) 계열: base_time은 30분 단위(예: 0630 등)로 사용되며, 각 발표 직후(예: 06:30 발표 데이터는 06:40~06:50 이후 등) API로 제공됩니다. [1]
- 단기/동네예보(블로그 정리 기준): base_time은 하루 8회 고정시각만 허용 — 02:00, 05:00, 08:00, 11:00, 14:00, 17:00, 20:00, 23:00(각 발표 후 약 10분 이후부터 API 제공). 따라서 요청할 때 base_time은 위 목록 중 하나로 맞춰야 합니다(그 외 값 사용 시 오류 코드 발생). [2]
참고자료:
[1] 기상청 API 허브 - 초단기/단기 API 요청인자(예: base_time 0630 표기).
[2] 개발자 블로그 정리 — 단기예보 base_time 허용값(0200/0500/.../2300) 및 API 제공시각(발표 후 약 10분).
🏁 Script executed:
cd /tmp && find . -type f -name "WeatherRepositoryImpl.kt" 2>/dev/nullRepository: SPOTeam/Android-V2
Length of output: 44
🏁 Script executed:
git ls-files | grep -i weatherreposRepository: SPOTeam/Android-V2
Length of output: 321
🏁 Script executed:
find . -type f -name "*.kt" | grep -i weatherRepository: SPOTeam/Android-V2
Length of output: 1619
🏁 Script executed:
wc -l data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.ktRepository: SPOTeam/Android-V2
Length of output: 157
🏁 Script executed:
cat -n data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.ktRepository: SPOTeam/Android-V2
Length of output: 4699
기상청 API base_time을 유효한 발표시각으로 조정해야 합니다
현재 구현은 현재 시간을 그대로 baseTime으로 사용하고 있습니다. 기상청 단기예보 API는 오직 특정 발표 시각(02:00, 05:00, 08:00, 11:00, 14:00, 17:00, 20:00, 23:00)만 허용하며, 다른 시간을 사용하면 즉시 NO_DATA 오류가 발생합니다.
현재 시각을 그대로 사용하면 대부분의 요청에서 NO_DATA 응답이 발생하며, 1시간 전으로 재시도하는 로직도 역시 유효한 발표시각으로 조정되지 않아 문제를 해결하지 못합니다(예: 현재 시간이 03:30이면 02:30을 시도하게 되어 여전히 실패).
calculateBaseDateTime 메서드에서 현재 시간을 가장 가까운 이전 발표 시각으로 올바르게 조정해야 합니다.
🤖 Prompt for AI Agents
In
@data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt
around lines 58 - 66, calculateBaseDateTime currently returns the raw now
formatted time which causes NO_DATA; change it to map the current LocalDateTime
down to the most recent valid 발표시각 from the set
{02:00,05:00,08:00,11:00,14:00,17:00,20:00,23:00} (if now is before 02:00 use
previous day 23:00), then return the corresponding baseDate and baseTime
strings; also ensure any retry logic that subtracts an hour uses
calculateBaseDateTime(now.minusHours(1)) or re-evaluates against the allowed
times rather than naively subtracting 1 hour so retries land on a valid 발표시각.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt:
- Around line 105-109: The log message in the onFailure block currently reads
"loadRecommendedStudies error" (with a double space) but this branch handles
loading popular studies (getRecruitingStudies with HITS sorting); update the
Log.e call in HomeViewModel (the onFailure { e -> ... } block) to use a correct,
concise message such as "loadPopularStudies error" or "getRecruitingStudies
error" and remove the extra space so the tag remains "HomeViewModel" and the
message accurately reflects the operation that failed.
♻️ Duplicate comments (5)
data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt (2)
56-72: NO_DATA 재시도 후 성공 여부 검증 누락1시간 전 데이터로 재시도한 후에도
NO_DATA인 경우에 대한 처리가 없습니다. 두 번째 호출도 실패하면dto.toDomain()이 유효하지 않은 데이터를 파싱할 수 있습니다.🐛 수정 제안
val dto = if ( first.response.header.resultCode == "03" && first.response.header.resultMsg == "NO_DATA" ) { val (prevDate, prevTime) = calculateBaseDateTime(now.minusHours(1)) weatherDataSource.getWeather( serviceKey = serviceKey, dataType = "JSON", baseDate = prevDate, baseTime = prevTime, nx = grid.nx, ny = grid.ny ) } else { first } + + // 두 번째 호출도 NO_DATA인 경우 예외 발생 + if (dto.response.header.resultCode == "03" && + dto.response.header.resultMsg == "NO_DATA") { + throw IllegalStateException("Weather data unavailable for the requested time") + }
85-93: 기상청 API base_time을 유효한 발표시각으로 조정해야 합니다현재 구현은 현재 시간을 그대로
baseTime으로 사용하고 있습니다. 기상청 단기예보 API는 오직 특정 발표 시각(02:00, 05:00, 08:00, 11:00, 14:00, 17:00, 20:00, 23:00)만 허용하며, 다른 시간을 사용하면NO_DATA오류가 발생합니다.🐛 수정 제안
private fun calculateBaseDateTime(now: LocalDateTime): Pair<String, String> { val formatterDate = DateTimeFormatter.ofPattern("yyyyMMdd") - val formatterTime = DateTimeFormatter.ofPattern("HHmm") - - val baseDate = now.format(formatterDate) - val baseTime = now.format(formatterTime) - - return baseDate to baseTime + + // 기상청 단기예보 발표시각 (API 제공은 발표 후 약 10분) + val baseTimes = listOf(2, 5, 8, 11, 14, 17, 20, 23) + + val hour = now.hour + val minute = now.minute + + // 발표 후 약 10분 후부터 데이터 제공되므로 10분 여유를 둠 + val adjustedHour = if (minute < 10) hour - 1 else hour + + // 현재 시간보다 작거나 같은 가장 큰 발표시각 찾기 + val baseHour = baseTimes.lastOrNull { it <= adjustedHour } + + return if (baseHour != null) { + val baseDate = now.format(formatterDate) + val baseTime = String.format("%02d00", baseHour) + baseDate to baseTime + } else { + // 02:00 이전이면 전날 23:00 사용 + val yesterday = now.minusDays(1) + val baseDate = yesterday.format(formatterDate) + baseDate to "2300" + } }feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt (3)
72-91: LGTM (성공 케이스), 단 실패 처리 주석 해제 필요.
getBestSinglePost호출 및 성공 처리는 적절합니다. 실패 상태 업데이트는 위에서 언급된 패턴과 동일하게 주석 해제가 필요합니다.
112-140: 로직은 적절하나, 실패 상태 업데이트 주석 해제 필요.추천 스터디 로딩 로직과 재로딩 함수가 잘 구현되어 있습니다.
onFailure블록의 상태 업데이트 주석 해제는 다른 섹션과 동일하게 필요합니다. 로그 메시지는 올바르게 수정되었습니다.
65-68: 실패 상태 업데이트가 여전히 주석 처리되어 있습니다.이전 리뷰에서 지적된 문제가 해결되지 않았습니다. API 호출 실패 시 UI가
Loading상태에서 영원히 멈추게 됩니다.🐛 수정 제안
.onFailure { e -> Log.e("HomeViewModel", "loadWeather error", e) -// _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } + _uiState.update { it.copy(weatherInfo = UiState.Failure(e.message ?: "날씨 불러오기 실패")) } }
🧹 Nitpick comments (2)
data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt (2)
9-9: 사용되지 않는 import 제거 필요
WeatherService가 import되어 있지만 클래스에서 사용되지 않습니다. 이제WeatherDataSource를 사용하므로 이 import를 제거해야 합니다.♻️ 수정 제안
-import com.umcspot.spot.weather.service.WeatherService
24-26: 캐싱 변수의 스레드 안전성 고려여러 코루틴에서 동시에
getWeather를 호출할 경우, 캐싱 변수들이 동기화 없이 접근되어 race condition이 발생할 수 있습니다. 최악의 경우 중복 API 호출이 발생하거나 캐시가 일관되지 않을 수 있습니다.
Mutex또는AtomicReference를 사용하여 스레드 안전성을 보장하는 것을 고려해 주세요.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.ktfeature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt
🧰 Additional context used
🧬 Code graph analysis (1)
feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt (1)
feature/alert/src/main/java/com/umcspot/spot/alert/AlertViewmodel.kt (1)
load(34-57)
🔇 Additional comments (5)
data/weather/src/main/java/com/umcspot/spot/weather/repositoryimpl/WeatherRepositoryImpl.kt (2)
29-54: LGTM!위도/경도를 그리드로 변환하고 캐시를 확인한 후 API를 호출하는 핵심 로직이 잘 구현되어 있습니다.
95-137: LGTM!Lambert Conformal Conic 투영을 사용한 위도/경도 → 그리드 변환 로직이 기상청 공식 문서에 맞게 올바르게 구현되어 있습니다.
feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt (3)
21-35: LGTM!Hilt 의존성 주입과 fallback 좌표 정의가 적절합니다. 서울시청 좌표를 fallback으로 사용하는 것은 합리적인 선택입니다.
37-50: LGTM! 이전 리뷰에서 지적된 파라미터 순서 오류가 수정되었습니다.Fallback 호출 시
load(FALLBACK_LATITUDE, FALLBACK_LONGITUDE)순서가 올바르게 수정되었습니다.@SuppressLint("MissingPermission")은 호출 전에 권한이 확인된다는 전제하에 적절합니다.
52-55: LGTM!로딩 오케스트레이션이 간결하게 구현되어 있습니다.
| .onFailure { e -> | ||
| Log.e("HomeViewModel", "loadRecommendedStudies error", e) | ||
|
|
||
| // _uiState.update { it.copy(popularStudies = UiState.Failure(e.message ?: "인기 스터디 실패")) } | ||
| } |
There was a problem hiding this comment.
로그 메시지가 잘못되었습니다.
Line 106의 로그 메시지가 "loadRecommendedStudies error"로 되어 있지만, 이 블록은 인기 스터디(getRecruitingStudies with HITS 정렬)를 로드하는 부분입니다. 또한 메시지에 불필요한 이중 공백이 있습니다.
📝 로그 메시지 수정
.onFailure { e ->
- Log.e("HomeViewModel", "loadRecommendedStudies error", e)
+ Log.e("HomeViewModel", "loadPopularStudies error", e)
-// _uiState.update { it.copy(popularStudies = UiState.Failure(e.message ?: "인기 스터디 실패")) }
+ _uiState.update { it.copy(popularStudies = UiState.Failure(e.message ?: "인기 스터디 실패")) }
}🤖 Prompt for AI Agents
In @feature/home/src/main/java/com/umcspot/spot/home/HomeViewModel.kt around
lines 105 - 109, The log message in the onFailure block currently reads
"loadRecommendedStudies error" (with a double space) but this branch handles
loading popular studies (getRecruitingStudies with HITS sorting); update the
Log.e call in HomeViewModel (the onFailure { e -> ... } block) to use a correct,
concise message such as "loadPopularStudies error" or "getRecruitingStudies
error" and remove the extra space so the tag remains "HomeViewModel" and the
message accurately reflects the operation that failed.
1 participant
Related issue 🛠
Work Description 📝
Screenshot 📸
Uncompleted Tasks 😅
PR Point 📌
트러블 슈팅 💥
Summary by CodeRabbit
새로운 기능
UI/UX 개선
버그 수정
기타
✏️ Tip: You can customize this high-level summary in your review settings.