Skip to content

Create Week 9 Mission 1, senior#44

Open
0Whitebird0 wants to merge 1 commit into
mainfrom
Week09/OneStone
Open

Create Week 9 Mission 1, senior#44
0Whitebird0 wants to merge 1 commit into
mainfrom
Week09/OneStone

Conversation

@0Whitebird0
Copy link
Copy Markdown
Contributor

📝 미션 번호

9주차 Misson 1, senior

📋 구현 사항

  • 프로필 페이지에서 api 연동과 horizontal pager로 팔로잉 리스트 구현
  • 팔로잉 리스트 아이템 클릭 시 alert dialog로 사진 보기 가능
  • 사진 pinch zoom으로 확대 가능

📎 스크린샷

KakaoTalk_20260525_223549451 KakaoTalk_20260525_223549451_01

✅ 체크리스트

  • [v] Merge 하려는 브랜치가 올바르게 설정되어 있나요?
  • [v] 에뮬레이터 또는 실제 기기에서 정상 동작하나요?
  • [v] 불필요한 주석 및 Log가 제거되었나요?

🤔 질문 사항

@0Whitebird0 0Whitebird0 requested a review from Dawon-Y May 25, 2026 13:41
@0Whitebird0 0Whitebird0 self-assigned this May 25, 2026
@Dawon-Y
Copy link
Copy Markdown
Contributor

Dawon-Y commented Jun 1, 2026

안녕하세요! 9주차 과제 진행하시느라 고생 많으셨습니다💚


1) 요구사항 충족 여부

마이페이지 UI 전부 구현

  • ProfileScreen에서 헤더/바로가기/혜택/팔로잉/가입일 footer까지 구성되어 있고, 로딩 오버레이도 있어 UI 측면은 충족입니다.

서버 데이터 연동 (userId = 1)

  • ProfileViewModel에서 USER_ID = 1 기준으로 profileRepository.getUsers() 결과에서 id == 1을 찾아 사용 → 요구사항 충족.
  • 다만 요구사항은 “1번 유저 상세 API 호출”도 가능한데, 현재 구현은 GET /api/users 목록에서 필터링하는 구조입니다(문제는 없지만, 의도가 살짝 다름).

팔로잉 리스트 HorizontalPager 구현

  • ProfileScreen에서 FollowingSectionHorizontalPager를 사용하고(파일 일부 생략이지만 import/구조상 확실), 상태는 followingProfiles로 내려오므로 요구사항 충족으로 판단됩니다.

팔로잉 터치 → AlertDialog로 이미지 확대

  • selectedFollowing을 state로 들고 FollowingImageDialog(...)를 띄우는 구조가 이미 구현되어 있음

(선택) Pinch Zoom

  • detectTransformGestures, graphicsLayer, pointerInput import가 있고 Dialog 쪽에서 처리하는 흔적이 있어 구현 의도가 보입니다. (다만 실제 Dialog 구현부가 생략되어 있어 “완전 구현” 여부는 최종 확인이 필요)

2) 가장 큰 구조적 이슈들

(A) ReqResApiService.getMissingUser() 설계가 잘못됨

@GET("api/users/23")
suspend fun getMissingUser(): ReqResUsersResponse
  • GET /api/users/23단일 유저 응답(single user) 형태인데, 리턴 타입을 ReqResUsersResponse(list response)로 두면 런타임 파싱 문제/크래시 가능성이 큽니다.
  • 지금 코드에서 실제로 호출하고 있지 않더라도, API 정의 자체가 오해를 낳고 유지보수에 위험합니다.

개선

  • 단일 유저 DTO를 따로 만들고 ReqResSingleUserResponse로 분리하거나, 필요 없으면 삭제하세요.
  • “missing user(404) 테스트”가 목적이면 리턴 타입은 Unit/Response<...> 형태로 받고 error handling 테스트를 하거나, Retrofit call을 Response<...>로 받아서 code 체크하는 게 정석입니다.

(B) DataStoreManager가 “프로필 과제”와 강하게 뒤엉켜 있음 (설계/유지보수)

DataStoreManager.kt는:

  • DataStore key/저장/로드
  • Product/ PurchaseProduct 초기화 로직
  • 이미지 리소스 검증/중복 id 검증
    까지 한 파일에서 처리하고 있습니다.

이건 프로필 과제 자체와는 별개로, 시니어 관점에선:

  • 단일 책임 원칙(SRP) 위반
  • 파일이 커질수록 변경 영향이 커짐
  • 테스트하기 어려움
  • 네이밍(예: Products 파라미터 대문자)도 컨벤션 깨짐

개선 방향(현실적인 선)

  • 최소한 DataStoreManager를 class로 감싸고(혹은 object),
    ProductDataStore, PurchaseProductDataStore로 파일/책임을 분리하는 것이 좋습니다.
  • Context.dataStore 확장 프로퍼티/Key 들도 internal/private 범위를 줄 수 있으면 줄이세요(현재 전역 public).

(C) FollowingProfileBitmap을 모델로 들고 있는 설계는 위험

data class FollowingProfile(
    val id: Int,
    val avatarBitmap: Bitmap
)
  • Bitmap은 메모리 비용이 크고, data layer/model layer에서 직접 들고 다니면
    • 메모리 누수 위험
    • 재사용/캐싱 전략 어려움
    • 프로세스 죽음/상태 저장 불가
    • diff/equals 연산에도 부적절
  • 다행히 현재 Compose 구현은 URL 기반(FollowingProfileUiState.avatarUrl)로 전환된 것으로 보이고, FollowingProfileAdapter/Fragment는 주석 처리(legacy)라 “현재 사용 안 함”이긴 합니다.

개선

  • 이 모델은 아예 삭제하거나, 최소한 avatarUrl: String로 바꾸는 편이 안전합니다.
  • “레거시를 남겨두고 싶다”면, 레거시는 legacy/ 패키지로 격리하거나, @Deprecated로 명시하세요.

3) ViewModel / Repository 레이어 리뷰

좋은 점

  • ProfileRepository를 인터페이스로 두고 @Binds로 DI 한 점은 정석입니다.
  • ProfileViewModel에서 BaseViewModel + setState { copy(...) } 형태를 쓰는 것도 일관성이 좋아요.
  • “이미 로딩된 상태면 다시 요청 안 함” 체크:
    if (currentState.userName.isNotEmpty()) return
    → 중복 호출 방지 의도가 명확합니다.

⚠️ 개선 포인트

(1) RepositoryImpl이 RetrofitClient에 직접 접근

return ReqResRetrofitClient.api.getUsers().data
  • DI를 쓰고 있는데도, Retrofit service를 주입받지 않고 ReqResRetrofitClient를 직접 참조하면
    • 테스트 어려움
    • 환경(베이스 URL/인터셉터) 교체 어려움
    • DI의 장점이 크게 줄어듭니다.

개선

  • ReqResApiService를 Hilt로 제공하고(@Provides) ProfileRepositoryImpl 생성자에 주입하세요.

(2) 에러 처리가 “HttpException 전용”이라서 실제로 안 잡힐 가능성

현재 ReqResApiServiceResponse<T>가 아니라 T를 바로 리턴합니다.

  • Retrofit이 4xx/5xx면 보통 HttpException 던지긴 하지만,
  • 네트워크 끊김은 UnknownHostException, SocketTimeoutException 등으로 떨어집니다.
  • 현재 메시지 매핑은:
if (this !is HttpException) return message

이라서 네트워크 오류는 message가 null이면 사용자에게 아무것도 안 나올 수 있어요.

개선

  • UnknownHostException을 사용자 친화 문구로 처리하거나,
  • 최소 default fallback 메시지를 보장하세요:
    • "네트워크 오류가 발생했습니다. 인터넷 연결을 확인해주세요."

(3) ProfileUiState를 “전체 교체”하는 방식의 혼용

성공 시:

ProfileUiState(...)

실패 시:

copy(...)
  • 크게 문제는 없지만, 한 프로젝트에서 상태 업데이트 방식이 섞이면 리뷰 포인트가 됩니다.
  • setState { copy(...) } 컨벤션을 유지하려면 성공도 copy로 처리하는 게 일관됩니다.

4) Compose UI 리뷰 (ProfileScreen.kt)

좋은 점

  • LaunchedEffect(uiState.errorMessage)로 Toast를 띄우는 패턴은 단순하고 동작합니다.
  • 로딩 중 AsyncImage 위에 CircularProgressIndicator를 띄워서 UX가 안정적입니다.
  • 선택된 팔로잉을 state로 들고 Dialog를 띄우는 방식도 Compose스럽습니다.

시니어 관점 개선 포인트

(1) Toast 이벤트는 상태로 들고 있으면 반복 가능

  • errorMessage가 state에 남아있으면 재구성/재진입에서 다시 Toast가 뜰 수 있습니다.
  • 이벤트는 SharedFlow/Channel 또는 “consume” 패턴(표시 후 null로 초기화)이 더 안전합니다.

(2) collectAsState()collectAsStateWithLifecycle() 권장

Compose에서 ViewModel state 수집은 lifecycle 기반 수집이 더 안전합니다.

(3) ProfileFragment가 빈 View를 반환하는 구조

return View(requireContext())
  • “Compose migration으로 XML 비활성화”를 설명하려는 의도는 이해하지만,
  • Fragment가 네비게이션 그래프에 남아있는 상태에서 빈 View를 반환하면
    • 화면이 하얗게 비는 문제
    • 실수로 이 route로 접근 시 디버깅이 어려움

개선

  • 완전히 제거하거나,
  • 최소한 명확한 안내 UI(텍스트 1줄이라도)를 보여주거나,
  • 네비게이션에서 해당 destination을 제거하는 게 맞습니다.

5) 레거시 코드 정리 포인트 (FollowingProfileAdapter, FollowingProfile.kt)

  • 현재 Compose로 넘어왔는데도 Adapter/Bitmap 모델이 남아있어 “프로젝트 탐색 시 혼란”이 큽니다.
  • 유지하려면 legacy 패키지로 분리하고, 사용처가 없으면 삭제가 더 낫습니다.

6) 최종 권장 액션(우선순위)

  1. ReqResApiService.getMissingUser() 반환 타입/DTO 수정 또는 삭제 (가장 위험)
  2. ProfileRepositoryImpl에서 RetrofitClient 직접 참조 제거하고 DI로 service 주입
  3. Bitmap 기반 FollowingProfile/Adapter 레거시 제거 또는 격리
  4. DataStoreManager 책임 분리 + 네이밍/범위(private/internal) 정리
  5. (UX) errorMessage를 1회성 이벤트로 처리 + lifecycle collect 적용

워크북 진행하시느라 수고하셨습니다! 궁금한 점 있으시면 언제든 말씀해 주세요!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants