-
Notifications
You must be signed in to change notification settings - Fork 39
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
🚀 4단계 - 장바구니(수량) #125
base: dothebestmayb
Are you sure you want to change the base?
🚀 4단계 - 장바구니(수량) #125
Conversation
/** | ||
* Room과 같은 역할을 한다고 가정하고 작성했습니다. | ||
*/ | ||
class ProductLocalDataSource { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
CartRepository에서 장바구니에 담긴 아이템 정보를 가지고 있던 CartLocalDataSource를 ProductLocalDataSource로 이름을 변경하고 ProdcutRepository로 옮겼습니다.
private val _itemsFlow = MutableStateFlow<List<ProductEntity>>(emptyList()) | ||
val itemsFlow: Flow<List<ProductEntity>> = _itemsFlow.asStateFlow() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
일반 Flow로 선언하면 데이터를 변경할 수 없어서 StateFlow로 선언했습니다.
StateFlow 말고 더 좋은 방법이 있는지 궁금합니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 방법 말고 떠오르는게 지금은 없네요 😅
fun update(product: ProductEntity) { | ||
_itemsFlow.update { items -> | ||
items.map { | ||
if (it.id == product.id) { | ||
product | ||
} else { | ||
it | ||
} | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
add, remove 등의 함수를 삭제하고, 단지 state를 갱신하도록 변경했습니다.
fun replaceAll(items: List<ProductEntity>) { | ||
_itemsFlow.update { beforeItems -> | ||
items.map { item -> | ||
item.copy( | ||
cartQuantity = beforeItems.firstOrNull { it.id == item.id }?.cartQuantity ?: 0 | ||
) | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상품 목록 정보를 불러온 후, 이전에 존재하던 아이템은 장바구니에 담긴 수량을 유지하도록 구현했습니다.
suspend fun fetch(): List<ProductEntity> { | ||
suspend fun fetch(): List<ProductResponse> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
API로부터 정보를 불러오는 데이터는 XxxResponse로 이름을 변경하고
API로 불러와서 Local에 들고 있는 데이터를 XxxEntity로 명명했습니다.
// Q. 화면이 파괴되면 Composable도 recomposition에서 제외되므로, Fragment에서 처럼 repeatOnLifecycle 등으로 Lifecycle을 지정하지 않아도 되나요? | ||
LaunchedEffect(Unit) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Composable이 Composition에서 벗어나면(화면에 더 이상 보이지 않는 경우), LaunchedEffect의 코루틴도 자동으로 취소된다고 이해했습니다.
productRepository.products | ||
.onStart { | ||
state = state.copy( | ||
isInitialLoading = false, | ||
) | ||
} | ||
.distinctUntilChanged() | ||
.map { items -> | ||
items.filter { | ||
it.cartQuantity > 0 | ||
} | ||
} | ||
.collect { items -> | ||
state = state.copy(cartItems = items.map { it.toUi() }) | ||
state = state.copy(products = items.map { it.toUi() }) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체 상품 목록에서 filter를 이용해 장바구니에 담긴 아이템을 선별하는 과정을 없애고 싶은데, 어떻게 설계해야 할지 아이디어가 떠오르지 않았습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filter를 Screen에서 처리하지않고, ProductRepository에서 하면 어떨까요?
productRepository.productInBasket 라는 느낌 어떠신가요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
장바구니 아이템과 상품 아이템을 별도로 관리하도록 수정했습니다.
onRemoveCartItemClick = { cartRepository.removeAll(it.product.toEntity()) }, | ||
onIncreaseQuantityClick = { cartRepository.addOne(it.product.toEntity()) }, | ||
onDecreaseQuantityClick = { cartRepository.removeOne(it.product.toEntity()) }, | ||
onRemoveCartItemClick = { productRepository.update(it.copy(cartQuantity = 0).toEntity()) }, | ||
onIncreaseQuantityClick = { | ||
productRepository.update( | ||
it.copy(cartQuantity = it.cartQuantity + 1).toEntity() | ||
) | ||
}, | ||
onDecreaseQuantityClick = { | ||
productRepository.update( | ||
it.copy(cartQuantity = it.cartQuantity - 1).toEntity() | ||
) | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상품의 상태 변경에 대한 책임을 Data Layer에서 Presentation Layer로 수정했습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다! 이런 로직은 나중에 ViewModel에서도 할 수 있겠네요!
val quantityModifier = if (product.cartQuantity < 1) { | ||
Modifier | ||
.align(Alignment.BottomEnd) | ||
.padding(8.dp) | ||
} else { | ||
Modifier | ||
.align(Alignment.BottomCenter) | ||
.padding(bottom = 8.dp) | ||
} | ||
) | ||
quantityHandler(quantityModifier) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Box Layout에서 수량 변경 Composable의 위치를 조정하기 위해 Modifier를 if문으로 조정했습니다.
작성하면서 코드가 어색하다고 느껴지는데, 어떻게 개선하면 좋을지 힌트를 얻고 싶습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
두 곳에서 처리하고 있어서 어색하다는 느낌을 받습니다.
컴포넌트 변경 없이 quantity 변경을 위한 ui를 받으려면 전부 외부에 위임하는게 좋고, 만약 ProductListItem의 quantity 변경이 변경될 일이 없으면 내부에서 전체 처리해도 좋다고 생각합니다.
결론적으로 외부에서만 처리하거나, 내부에서만 처리하거나 하면 좋을 것 같아요
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
quantityModifier는 Box Layout 내부에서 QuantityHandler의 위치를 조절하기 위한 값입니다. 이 modifier를 외부에서 생성하고 전달되는 QuantityHandler Composable에 적용하는 것은 어색하다고 생각했습니다.
외부에서 Box Scope Modifier를 설정한다는 것은 ProductListItem 내부에서 Box Layout을 사용한다는 것을 안다는 것이고, 이것은 결합성 증가를 의미하며, 내부에서 QuantityHandler Composable을 직접 생성 하는 것과 똑같기 때문입니다.
그래서 외부에서 전달하던 quantityHandler Composable을 내부에서 생성하도록 수정했습니다.
refactor: 장바구니에 담긴 아이템 수량에 따라 QuantityHandler UI 내부에서 변경하도록 수정
// 초기 데이터 로딩 | ||
LaunchedEffect(Unit) { | ||
// 초기에만 로딩되도록 조건문 추가 | ||
// Navigation에 의해 Composition이 파괴되고 다시 생성될 때, 다시 목록을 불러오지 않도록 방지 | ||
if (state.isInitialLoading) { | ||
delay(500L) | ||
state = state.copy( | ||
isLoadingShow = true, | ||
) | ||
productRepository.fetch() | ||
} | ||
} | ||
|
||
// Unit으로 설정할 경우, configuration change가 발생해도 호출된다. | ||
// 따라서 초기로딩이 되지 않은 경우에만 호출되도록 관련 state를 key로 설정 | ||
LaunchedEffect(state.isInitialLoading) { | ||
state = state.copy( | ||
products = productRepository.fetch().map { it.toUi() }, | ||
isInitialLoading = false, | ||
) | ||
LaunchedEffect(Unit) { | ||
productRepository.products | ||
.onStart { | ||
state = state.copy( | ||
isInitialLoading = false, | ||
) | ||
} | ||
.collect { | ||
state = state.copy( | ||
products = it.map { it.toUi() }, | ||
selectedItemCount = it.sumOf { it.cartQuantity }, | ||
) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
상품 목록 화면에서 데이터를 처음에만 불러오도록 설정했습니다.
이렇게 LaunchedEffect를 여러 개 사용해도 되는지 궁금합니다. 이러한 요구조건에서 더 좋은 코드 패턴이 있나요?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 지금은 이렇게 할 수 밖에 없겠네요.
나중에는 ViewModel을 만들어서 해볼 수 있겠어요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
장바구니 담긴 정보 관리 방식
장바구니 관련해서 고민이 많이 되셨을것 같아요.
지금은 더미데이터와 로컬에서만 만드는 데이터로 하다보니 ProductEntity에 모든 정보를 담았는데 만약 서버에서 상품정보와 장바구니 정보가 따로 온다면 어떻게 구현해야할까요? 아니면 서버에서는 상품정보만 내려주고, 장바구니 정보를 로컬에 저장해야한다면 어떻게 해야할까요?
이런 관점으로 봤을 때 상품정보의 source와 장바구니의 source는 분리될 수 있다고 생각해요.
Parcelable body의 property 관련 질문
남겨주신 warning 같은 경우는 단어 그대로 경고일 뿐이고 이 값은 Parcelable 되지 않아도 되는 정보라고 생각합니다. 추천하는 것처럼 @IgnoredOnParcel 를 추가해줘도 될 것 같아요.
테스트코드나 추가 구현사항 주시면 리뷰 더 하도록 할게요!
미션을 이만 마치고 싶다면 바로 리뷰요청 해주세요~
private val _itemsFlow = MutableStateFlow<List<ProductEntity>>(emptyList()) | ||
val itemsFlow: Flow<List<ProductEntity>> = _itemsFlow.asStateFlow() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 방법 말고 떠오르는게 지금은 없네요 😅
productRepository.products | ||
.onStart { | ||
state = state.copy( | ||
isInitialLoading = false, | ||
) | ||
} | ||
.distinctUntilChanged() | ||
.map { items -> | ||
items.filter { | ||
it.cartQuantity > 0 | ||
} | ||
} | ||
.collect { items -> | ||
state = state.copy(cartItems = items.map { it.toUi() }) | ||
state = state.copy(products = items.map { it.toUi() }) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
filter를 Screen에서 처리하지않고, ProductRepository에서 하면 어떨까요?
productRepository.productInBasket 라는 느낌 어떠신가요?
onRemoveCartItemClick = { cartRepository.removeAll(it.product.toEntity()) }, | ||
onIncreaseQuantityClick = { cartRepository.addOne(it.product.toEntity()) }, | ||
onDecreaseQuantityClick = { cartRepository.removeOne(it.product.toEntity()) }, | ||
onRemoveCartItemClick = { productRepository.update(it.copy(cartQuantity = 0).toEntity()) }, | ||
onIncreaseQuantityClick = { | ||
productRepository.update( | ||
it.copy(cartQuantity = it.cartQuantity + 1).toEntity() | ||
) | ||
}, | ||
onDecreaseQuantityClick = { | ||
productRepository.update( | ||
it.copy(cartQuantity = it.cartQuantity - 1).toEntity() | ||
) | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
좋습니다! 이런 로직은 나중에 ViewModel에서도 할 수 있겠네요!
// 초기 데이터 로딩 | ||
LaunchedEffect(Unit) { | ||
// 초기에만 로딩되도록 조건문 추가 | ||
// Navigation에 의해 Composition이 파괴되고 다시 생성될 때, 다시 목록을 불러오지 않도록 방지 | ||
if (state.isInitialLoading) { | ||
delay(500L) | ||
state = state.copy( | ||
isLoadingShow = true, | ||
) | ||
productRepository.fetch() | ||
} | ||
} | ||
|
||
// Unit으로 설정할 경우, configuration change가 발생해도 호출된다. | ||
// 따라서 초기로딩이 되지 않은 경우에만 호출되도록 관련 state를 key로 설정 | ||
LaunchedEffect(state.isInitialLoading) { | ||
state = state.copy( | ||
products = productRepository.fetch().map { it.toUi() }, | ||
isInitialLoading = false, | ||
) | ||
LaunchedEffect(Unit) { | ||
productRepository.products | ||
.onStart { | ||
state = state.copy( | ||
isInitialLoading = false, | ||
) | ||
} | ||
.collect { | ||
state = state.copy( | ||
products = it.map { it.toUi() }, | ||
selectedItemCount = it.sumOf { it.cartQuantity }, | ||
) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
넵 지금은 이렇게 할 수 밖에 없겠네요.
나중에는 ViewModel을 만들어서 해볼 수 있겠어요!
contentDescription = null, | ||
tint = Color.Red, | ||
) | ||
Box() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Box()에 파라미터가 없다면 ()를 제거해도 될 것 같아요~
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
val quantityModifier = if (product.cartQuantity < 1) { | ||
Modifier | ||
.align(Alignment.BottomEnd) | ||
.padding(8.dp) | ||
} else { | ||
Modifier | ||
.align(Alignment.BottomCenter) | ||
.padding(bottom = 8.dp) | ||
} | ||
) | ||
quantityHandler(quantityModifier) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
두 곳에서 처리하고 있어서 어색하다는 느낌을 받습니다.
컴포넌트 변경 없이 quantity 변경을 위한 ui를 받으려면 전부 외부에 위임하는게 좋고, 만약 ProductListItem의 quantity 변경이 변경될 일이 없으면 내부에서 전체 처리해도 좋다고 생각합니다.
결론적으로 외부에서만 처리하거나, 내부에서만 처리하거나 하면 좋을 것 같아요
[작성중] 장바구니 아이템 관리장바구니에 담긴 아이템을 상품 목록 한 개에서 관리하지 않고, 장바구니에 담긴 아이템을 별도의 LinkedHashMap으로 관리하도록 분리했습니다. 상품 목록의 각 아이템이 장바구니에 존재하는지 O(1)만에 확인할 수 있도록 장바구니 아이템을 HashMap을 고려했습니다. |
면접 준비로 인해 구현이 늦어진 점 양해 부탁드립니다. 🙇♂️
구현 내용 및 고민
🛒 1. 장바구니 담긴 정보 관리 방식 개선
🔄 2. 담긴 수량 변경의 책임
Repository가 아닌 Presentation Layer에서 처리하도록 수정하였습니다.
🔍 3. 아이템 Fetch 최적화
🏷 4. Parcelable body의 property 관련 질문
→ 관련하여 리뷰어님의 의견을 듣고 싶습니다.
🔗 이전 PR
구현 결과
bandicam.2025-03-06.04-18-54-584.mp4
🚀 4단계 계획
리뷰해주셔서 감사합니다! 🙏
의견이나 피드백 주시면 적극 반영하겠습니다. 🚀