[스캇] 1, 2단계 쇼핑 장바구니 제출합니다. #14
Merged
Conversation
This file contains 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
galcyurio
requested changes
May 11, 2023
app/src/main/java/woowacourse/shopping/presentation/productlist/GridLayoutSizeManager.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/shopping/presentation/cart/CartPresenter.kt
Show resolved
Hide resolved
Comment on lines
17
to
26
| private var page = Counter(1) | ||
| private val products: Products = Products() | ||
| override fun initCart() { | ||
| setProducts() | ||
| view.setPage(page.value) | ||
| val cartProducts = products.getProductsInRange((page.value - 1) * 5, 5) | ||
| view.initCartProductModels(cartProducts.toPresentation()) | ||
| checkRightPageAble() | ||
| checkLeftPageAble() | ||
| } |
There was a problem hiding this comment.
managePaging 이라는 함수에 페이징 관련 함수들을 넣었습니다
There was a problem hiding this comment.
여전히 페이징 처리에 관한 코드들이 Presenter의 이곳 저곳에 흩어져 있습니다.
page가 쓰이는 곳을 검색하고 해당 코드들을 최대한 하나의 객체로 모아보세요.
app/src/main/java/woowacourse/shopping/presentation/cart/viewholder/CartItemViewHolder.kt
Outdated
Show resolved
Hide resolved
...src/main/java/woowacourse/shopping/presentation/productlist/product/ProductItemViewHolder.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/shopping/presentation/productlist/ProductListActivity.kt
Outdated
Show resolved
Hide resolved
app/src/main/java/woowacourse/shopping/presentation/productlist/product/ProductListAdapter.kt
Show resolved
Hide resolved
galcyurio
requested changes
May 14, 2023
Comment on lines
17
to
26
| private var page = Counter(1) | ||
| private val products: Products = Products() | ||
| override fun initCart() { | ||
| setProducts() | ||
| view.setPage(page.value) | ||
| val cartProducts = products.getProductsInRange((page.value - 1) * 5, 5) | ||
| view.initCartProductModels(cartProducts.toPresentation()) | ||
| checkRightPageAble() | ||
| checkLeftPageAble() | ||
| } |
There was a problem hiding this comment.
여전히 페이징 처리에 관한 코드들이 Presenter의 이곳 저곳에 흩어져 있습니다.
page가 쓰이는 곳을 검색하고 해당 코드들을 최대한 하나의 객체로 모아보세요.
Comment on lines
25
to
29
| every { view.initCartItems(any()) } just runs | ||
| every { view.setCartItems(any()) } just runs | ||
| every { view.setPage(any()) } just runs | ||
| every { view.setLeftPageState(any()) } just runs | ||
| every { view.setRightPageState(any()) } just runs |
There was a problem hiding this comment.
relaxed mock에 대해서 알아보고 적용해본 뒤 불필요한 stubbing 코드들을 제거해보세요.
galcyurio
approved these changes
May 15, 2023
Comment on lines
+10
to
+11
| private val cartRepository: CartRepository, | ||
| private val productRepository: ProductRepository, |
There was a problem hiding this comment.
이 Paging 객체를 페이지가 있는 다른 화면에서도 쓸 수 있도록 추상화시켜보세요.
offset 방식이므로 OffsetPaging과 같은 이름으로 변경해보고 해당 기능만 남겨보면 어떨까요?
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.
안녕하세요 리뷰어님! 스캇입니다!
첫 미션 이후로 오랜만에 뵙네요! 반갑습니다!
항상 좋은 리뷰 남겨주셔서 감사드립니다!!
이번 미션 잘 부탁드립니다!!