feat: 6주차 미션_묵은지#47
Conversation
kimdoyeon1234
left a comment
kimdoyeon1234
left a comment
There was a problem hiding this comment.
수고하셨습니다!
이번 코드는 ShoppingRepository로 LocalRepository와 RemoteRepository를
통합해서 ViewModel이 하나의 창구만 바라보도록 구성한 점,
그리고 AppModule에서 필요한 의존성들을 @provides로 잘 등록한 점이 좋았어요!
다만 이번 미션의 핵심은 패키지 분리입니다!
현재 DataStoreManager, Product, UserData, Fragment, ViewModel 등
대부분의 파일이 루트 패키지(com.example.week3)에 몰려있어요.
data/local/, data/model/, data/remote/, data/repository/, di/,
ui/home/, ui/profile/, ui/purchase/ 처럼 역할에 맞게 패키지를 나눠주세요
추가로 Hilt를 도입했는데 RetrofitClient object가 아직 남아있으니 삭제해주시고,
MainActivity에서 DataStoreManager를 직접 생성해서 더미 데이터를 초기화하는 부분은
Repository나 ViewModel로 옮겨주세요!
PurchaseFragment에 상품 데이터가 하드코딩되어 있는 부분과
위시리스트 상태 확인 로직이 Fragment에 있는 부분도 ViewModel로 이동시켜주세요!
Retrofit Callback 방식도 suspend 함수 + viewModelScope.launch로 바꾸면
훨씬 코틀린답고 깔끔한 코드가 될 거예요 :)
전반적으로 Repository 구조 자체는 잘 잡으셨으니
패키지 분리와 Fragment의 비즈니스 로직 정리만 해주시면
훨씬 완성도 있는 코드가 될 것 같습니다. 고생 많으셨어요! 😊
There was a problem hiding this comment.
AppModule에서 이미 Hilt로 UserService를 제공하고 있는데 RetrofitClient object가 별도로 남아있어요!
Hilt를 도입했다면 RetrofitClient는 삭제하고 AppModule만 사용해주세요 :)
There was a problem hiding this comment.
AppModule에서 RetrofitClient.instance를 그대로 가져다 쓰고 있어요. Retrofit 객체 자체를 @provides로 만들고, 그걸 주입받아서 retrofit.create(UserService::class.java)로 생성하는 방식으로 바꿔주세요 :)
There was a problem hiding this comment.
Hilt로 DI를 구성했는데 MainActivity에서 DataStoreManager를 직접 생성하고 있어요! 더미 데이터 초기화 로직은 ViewModel이나 Repository로 옮기고, DataStoreManager는 직접 생성하지 말고 Hilt로 주입받아 사용해주세요
| fun loadMyData(userId: Int) { | ||
| repository.getUser(userId, object : Callback<UserResponse> { | ||
| override fun onResponse(call: Call<UserResponse>, response: Response<UserResponse>) { | ||
| if (response.isSuccessful) { | ||
| _userData.value = response.body()?.data | ||
| } | ||
| } | ||
|
|
||
| override fun onFailure(call: Call<UserResponse>, t: Throwable) {} | ||
| }) | ||
| } | ||
|
|
There was a problem hiding this comment.
Retrofit의 Callback 방식 대신 suspend 함수로 바꾸면 더 코틀린답게 작성할 수 있어요! 정답 코드처럼 viewModelScope.launch + runCatching을 사용해보면은 더 좋을거 같습니다!
|
|
||
| private val products = listOf( | ||
| Product("Nike Everyday Plus Cushioned", "$10", R.drawable.socks), | ||
| Product("Nike Everyday Plus Cushioned2", "$10", R.drawable.socks), | ||
| Product("Nike Everyday Plus Cushioned3", "$10", R.drawable.socks), | ||
| Product("Nike Everyday Plus Cushioned4", "$10", R.drawable.socks) | ||
| ) | ||
|
|
There was a problem hiding this comment.
상품 데이터가 Fragment에 하드코딩되어 있어요! 더미 데이터는 Repository나 DataStore에서 관리하고,
Fragment는 ViewModel을 통해 데이터를 받아오도록 수정해주세요 !
|
|
||
| val isNowWishlisted = viewModel.wishlist.value?.any { | ||
| it.name == products[index].name | ||
| } ?: false | ||
|
|
||
| Toast.makeText( | ||
| context, | ||
| if (isNowWishlisted) "위시리스트 제거" else "위시리스트 추가!", | ||
| Toast.LENGTH_SHORT | ||
| ).show() | ||
| } | ||
| } |
There was a problem hiding this comment.
위시리스트 상태 확인 로직이 Fragment에 있어요! 이런 비즈니스 로직은 ViewModel에서 처리하고,
Fragment는 ViewModel의 상태를 관찰해서 Toast만 띄우도록 수정해주세요 !
| class ProductAdapter( | ||
| private val items: List<Product>, | ||
| private val listener: ProductClickListener | ||
| ) : RecyclerView.Adapter<ProductAdapter.ViewHolder>() { | ||
|
|
There was a problem hiding this comment.
List를 직접 받는 방식 대신 ListAdapter + DiffUtil을 사용하면 변경된 항목만 업데이트해서 성능이 훨씬 좋아져요 !
There was a problem hiding this comment.
ProductAdapter와 동일하게 ListAdapter + DiffUtil로 교체해주세요 :)
There was a problem hiding this comment.
이번 미션의 핵심은 패키지 분리입니다!
DataStoreManager → data/local/
Product, UserData → data/model/
Network(RetrofitClient) → data/remote/
패키지를 만들어서 이동시켜주세요!
There was a problem hiding this comment.
UI 파일들도 루트 패키지에 몰려있어요! ui/home/, ui/profile/, ui/purchase/, ui/wishlist/ 처럼 화면별로 패키지를 나눠주세요
2 participants
📌 PR 제목
🔗 관련 이슈
Closes #이슈번호
✨ 변경 사항
🔍 테스트
📸 스크린샷 (선택)
🚨 추가 이슈