[쇼핑 장바구니] 알송 1, 2단계 미션 제출합니다. #58
Conversation
- 상품 삭제 시 한 페이지에 5개의 상품이 유지되지 않는 버그 수정
| @Test | ||
| fun `화면이_띄워지면_장바구니에_담긴_상품명이_보인다`() { | ||
| onView(withId(R.id.rv_cart)) | ||
| .perform(RecyclerViewActions.scrollToPosition<CartViewHolder>(0)) | ||
| .check(matches(hasDescendant(allOf(withText("맥북"), isDisplayed())))) | ||
| } | ||
|
|
||
| @Test | ||
| fun `화면이_띄워지면_장바구니에_담긴_상품의_가격이_보인다`() { | ||
| onView(withId(R.id.rv_cart)) | ||
| .perform(RecyclerViewActions.scrollToPosition<CartViewHolder>(0)) | ||
| .check(matches(hasDescendant(allOf(withText("100원"), isDisplayed())))) | ||
| } |
There was a problem hiding this comment.
만약 화면 디자인은 바뀌지 않았지만 RecyclerView가 아닌 다른 View로 바뀌게 된다면, 이 테스트는 사용 불가능한 형태가 될 걸로 예상돼요.
UI에서 테스트하고자 하는 것이 정말 "리사이클러뷰"라는 제약이 따라야하는지 고민해보세요!
There was a problem hiding this comment.
죄송합니다.. RecyclerViewActions를 사용하지 않고 리사이클러뷰의 스크롤을 테스트하는 법을 찾지 못하겠네요.. 혹시 현업에서 리사이클러뷰가 아닌 다른 뷰를 쓰는 일이 많이 있는지 여쭤보고 싶습니다..!
There was a problem hiding this comment.
화면에 리사이클러뷰가 있고, 그 안에 텍스트가 있음!
이란 테스트 보다는,
화면에 텍스트가 있음!
이런 테스트가 UI 테스트에서는 좀 더 범용적이지 않을까 하는 관점을 보여드리고 싶었어요!
현업에서는 UI 테스트를 잘 진행하지 않고 있어요. 비용적인 문제 때문에요 😥
|
|
||
| class CartPageManager { |
There was a problem hiding this comment.
이 객체는 어떤 역할을 담당하는지 설명해주실 수 있나요 ?_?
또, 만약 다른 링크나 다른 곳에서 들어온 카트에서는 보여줄 수 있는 페이지의 수가 변경될 수 있다면, 유연하게 대체할 수 있는 형태일까요?
There was a problem hiding this comment.
카트 액티비티에서 페이지를 전환하는 버튼은 마지막 페이지일 경우나, 첫 번째 페이지일 경우 색깔이 바뀌고 눌리지 않게 해야 하는데, 그것을 관리하는 객체입니다!
다른 곳에서 들어온 카트라는 것이 무슨 뜻인지 이해가 가지 않는데요.. 좀 더 자세히 설명을 부탁드려도 될까요..?
There was a problem hiding this comment.
액티비티는 앱을 구성하는 요소이지요!
앱을 구성하다보면 특정 액티비티는 특정 이벤트가 일어나지 않으면 결코 나타날 수 없게 될 수도 있겠지만, 해당 액티비티 자체는 언제든 다른 로직들부터 호출될 수 있지요!
그렇게 진입 점이 다른곳에서는 조금은 다른 뷰를 보여주어야하는 요구사항이 있을수도 있으니, 이런 경우에도 유연하게 대처할 수 있는 형태인지 고민해보시라는 의미었어요 :)
| private fun changeNextButtonEnabled() { | ||
| viewModel.canMoveNextPage.observe(this) { | ||
| binding.btnNext.isClickable = it | ||
| binding.btnNext.isEnabled = it | ||
| } | ||
| } | ||
|
|
||
| private fun changePreviousButtonEnabled() { | ||
| viewModel.canMovePreviousPage.observe(this) { | ||
| binding.btnPrevious.isClickable = it | ||
| binding.btnPrevious.isEnabled = it | ||
| } | ||
| } |
There was a problem hiding this comment.
데이터바인딩을 활용해서 VIewModel의 값을 직접 구독해 실시간으로 뷰의 값을 변경시키려면 어떻게 구성해볼 수 있을까요?
There was a problem hiding this comment.
xml의 뷰들이 뷰모델의 라이브데이터를 직접 참조하고 observe함수에서는 뷰모델을 갱신해주는 것으로 수정 가능했습니다! 😊
| class CartAdapter( | ||
| private val itemRemoveClickListener: (Long) -> Unit, |
There was a problem hiding this comment.
버튼을 클릭했을 때의 콜백 이름은 View.OnClickListener interface를 잘 참고해보시고 이름을 지어보세요 :)
There was a problem hiding this comment.
OnClickListener interface에 onClick()이라는 이름으로 메서드가 선언되어있음을 알 수 있었습니다. onClick()이라는 네이밍은 "클릭 시" 라는 의미의 네이밍이라고 생각이 되기 때문에 onRemoveButtonClick() 정도로 지으면 될 것 같습니다!
| Glide.with(binding.root) | ||
| .load(product.imageUrl) | ||
| .into(binding.ivProductImage) | ||
| } |
There was a problem hiding this comment.
데이터바인딩의 기능을 활용해서 이렇게 직접 세팅해주는 코드를 줄여내볼 수 있겠어요!
| private val cartPageManager by lazy { CartPageManager() } | ||
| private val _pageNumber: MutableLiveData<Int> = MutableLiveData() |
There was a problem hiding this comment.
_pageNumber 는 loadCartItems를 호출하지 않으면 갱신할 방법이 없겠어요.
이 Viewmodel을 활용하는 곳에서 이 사실을 알지 못하면 다른 개발자가 기능을 개발할 때 버그가 100% 발생할것으로 예상되어요.
또, 늘 loadCartItems()를 호출해주어야 대부분의 뷰가 갱신하도록 구성되어있다면, 이것은 MVP 아키텍처와 큰 차이가 있는지 고민해보셔도 좋겠어요.
There was a problem hiding this comment.
확실히 그렇네요! 뷰모델을 처음 쓰다보니 아직 익숙치가 않네요.. 😅 수정했습니다!
|
|
||
| binding = DataBindingUtil.setContentView(this, R.layout.activity_product_detail) |
There was a problem hiding this comment.
DataBindingUtil과 ~~Binding을 활용해서 inflate하는 것은 각각 어떤 차이점이 있을까요?
There was a problem hiding this comment.
binding = ActivityProductDetailBinding.inflate(layoutInflater)
setContentView(binding.root)
과 같이 사용할 수도 있습니다! 제가 기존에 사용한 DataBindingUtil을 사용하는 방법은 layout의 id를 모르는 경우에만 사용하라고 공식문서의 DataBindingUtil 부분에도 설명이 되어 있네요.. https://developer.android.com/reference/android/databinding/DataBindingUtil
T inflate ([LayoutInflater] inflater,
int layoutId,
[ViewGroup] parent,
boolean attachToParent,
[DataBindingComponent] bindingComponent)
Inflates a binding layout and returns the newly-created binding for that layout.
Use this version only if layoutId is unknown in advance. Otherwise, use the generated Binding's inflate method to ensure type-safe inflation.
사실 '레이아웃id를 모를 때' 가 무엇을 의미하는지 정확히 모르겠습니다. 어플리케이션의 실행 도중에 어떤 레이아웃이 선택될지가 동적으로 결정될 경우를 말하는 것일까요?
There was a problem hiding this comment.
어플리케이션의 실행 도중에 어떤 레이아웃이 선택될지가 동적으로 결정될 경우를 말하는 것일까요?
저도 이 경우가 생각이 나네요!
액티비티 처럼 너무나 명확하게 레이아웃 id에 대해 알고 있다면 DataBindingUtil를 사용할 필요성이 느껴지지 않아서요 :)
| runCatching { | ||
| viewModel.loadProduct(productId) | ||
| }.onSuccess { |
There was a problem hiding this comment.
viewModel.loadProduct(productId) 에서는 어떤 Exception을 처리하기 위해 runCatching을 사용하셨나요?
There was a problem hiding this comment.
현 로직상 예외가 던져지는 경우는 없긴 합니다만, 나중에 로직이 바뀔 경우 예외가 발생할 일이 있다고 생각했습니다. 만약 productId와 id가 일치하는 product가 저장소에 존재하지 않는 경우 예외 처리를 위해 사용했습니다.
| notifyDataSetChanged() | ||
| } |
There was a problem hiding this comment.
notifyDataSetChanged 메서드에 린트가 나타나는 이유는 무엇일까요?
There was a problem hiding this comment.
notifyDataSetChaged는 공식문서에도 설명하고 있지만 최후의 수단으로 사용해야 하는 함수이고, 가급적 다른 함수를 사용하도록 권장하고 있기 때문입니다. 😅 이 함수는 리사이클러뷰의 모든 상품을 지웠다가 다시 그리기 때문에 성능 상의 문제가 발생할 수 있습니다. notifyItemRangeInserted()를 사용하는 것으로 수정하였습니다!
| private fun setProductAdapter() { | ||
| binding.rvProducts.layoutManager = GridLayoutManager(this, SPAN_COUNT) |
There was a problem hiding this comment.
LayoutManager는 xml에서도 세팅해볼 수 있어요.
어떻게 세팅할 수 있을까요?
There was a problem hiding this comment.
app:layoutManager="androidx.recyclerview.widget.GridLayoutManager"
app:spanCount="2"
와 같은 방법으로 할 수 있었습니다!
Unit 테스트에서 ViewModelProvider를 통해 ViewModel를 생성하지 않고, 직접 ViewModel을 생성해서 해당 ViewModel에 대한 Unit Test를 진행했다는 의미이시지요? Activity에서 ViewModelFactory를 통해서 ViewModel을 생성해서 쓰는 이유는 무엇인가요? 레벨 1의 로또 미션을 되새겨볼까요? |
|
답변 감사합니다!! Activity에서 ViewModelFactory를 통해서 ViewModel을 생성해서 쓰는 이유는, 뷰모델에 외부 데이터를 주입해 사용하기 위함이었습니다. 실제 앱에서는 뷰모델에 데이터를 주입하게 되면 인스턴스가 제대로 생성되지 않기 때문이었습니다. 하지만 unit테스트에서의 뷰모델은 실제 앱처럼 동작하지 않고 특정 기능만 테스트하면 됩니다. 따라서 ViewModelProvider를 통하지 않고 인스턴스를 생성할 수 있다는 설명으로 이해하면 될까요? |
…참조하고 있다면 observe 함수를 쓰지 않도록 변경 뷰가 데이터바인딩으로 라이브데이터를 직접 참조하고 있으면 observe를 직접 하지 않아도 된다는 것을 뒤늦게 깨달아서 수정합니다 ^^;
|
|
||
| class CartPageManager { |
There was a problem hiding this comment.
액티비티는 앱을 구성하는 요소이지요!
앱을 구성하다보면 특정 액티비티는 특정 이벤트가 일어나지 않으면 결코 나타날 수 없게 될 수도 있겠지만, 해당 액티비티 자체는 언제든 다른 로직들부터 호출될 수 있지요!
그렇게 진입 점이 다른곳에서는 조금은 다른 뷰를 보여주어야하는 요구사항이 있을수도 있으니, 이런 경우에도 유연하게 대처할 수 있는 형태인지 고민해보시라는 의미었어요 :)
| @Test | ||
| fun `화면이_띄워지면_장바구니에_담긴_상품명이_보인다`() { | ||
| onView(withId(R.id.rv_cart)) | ||
| .perform(RecyclerViewActions.scrollToPosition<CartViewHolder>(0)) | ||
| .check(matches(hasDescendant(allOf(withText("맥북"), isDisplayed())))) | ||
| } | ||
|
|
||
| @Test | ||
| fun `화면이_띄워지면_장바구니에_담긴_상품의_가격이_보인다`() { | ||
| onView(withId(R.id.rv_cart)) | ||
| .perform(RecyclerViewActions.scrollToPosition<CartViewHolder>(0)) | ||
| .check(matches(hasDescendant(allOf(withText("100원"), isDisplayed())))) | ||
| } |
There was a problem hiding this comment.
화면에 리사이클러뷰가 있고, 그 안에 텍스트가 있음!
이란 테스트 보다는,
화면에 텍스트가 있음!
이런 테스트가 UI 테스트에서는 좀 더 범용적이지 않을까 하는 관점을 보여드리고 싶었어요!
현업에서는 UI 테스트를 잘 진행하지 않고 있어요. 비용적인 문제 때문에요 😥
| private fun setOnNextButtonClickListener() { | ||
| binding.btnNext.setOnClickListener { | ||
| viewModel.plusPageNum() | ||
| } | ||
| } | ||
|
|
||
| private fun setOnPreviousButtonClickListener() { | ||
| binding.btnPrevious.setOnClickListener { | ||
| viewModel.minusPageNum() | ||
| } | ||
| } |
There was a problem hiding this comment.
binidng 객체에 viewModel을 넘기고 있으니, 이런 클릭 이벤트들도 xml에 선언해보는 건 어떨까요?
| fun plusPageNum() { | ||
| cartPageManager.plusPageNum() | ||
| _pageNumber.value = cartPageManager.pageNum | ||
| _canMovePreviousPage.value = cartPageManager.canMovePreviousPage() | ||
| _canMoveNextPage.value = cartPageManager.canMoveNextPage(cartItems.size) | ||
| _cart.value = getProducts() | ||
| } | ||
|
|
||
| fun minusPageNum() { | ||
| cartPageManager.minusPageNum() | ||
| _pageNumber.value = cartPageManager.pageNum | ||
| _canMovePreviousPage.value = cartPageManager.canMovePreviousPage() | ||
| _canMoveNextPage.value = cartPageManager.canMoveNextPage(cartItems.size) | ||
| _cart.value = getProducts() | ||
| } |
There was a problem hiding this comment.
CartPageManager의 값을 전달해주기만 하는 느낌이 많이 드네요.
CartPage에 대한 정보를 LiveData 자체에 갖게 해보는 방법도 있겠어요 :)
그렇담 Manager라는 이름은 굉장히 어색해지겠어요! 여러 방법을 잘 찾아볼까요?
There was a problem hiding this comment.
페이지를 관리하는 CartPageManager라는 객체로는 라이브데이터로 갖도록 하는 것이 상당히 어색하군요! CartPage라는 새로운 객체로 변경하고, 이 객체를 라이브데이터로 갖도록 수정했습니다! 코드가 훨씬 간결해졌군요 😄
| notifyItemRangeInserted( | ||
| newProducts.size * currentOffset, | ||
| newProducts.size, | ||
| ) | ||
| currentOffset++ |
There was a problem hiding this comment.
currentOffset을 필드로 관리하는 건 조금 위험할 수 있어 보여요.
현 products와, 추가 하고 난 products의 사이즈, newProducts의 사이즈 등을 계산하는 로직으로 구성해보는 건 어떨까요?
There was a problem hiding this comment.
현 products의 사이즈와 newProducts의 사이즈만 가지고도 범위를 지정해줄 수 있었네요! currentOffset이 필요가 없었습니다 😆
| // then | ||
| assertThat(actual[0].name).isEqualTo("갤럭시북") | ||
| assertThat(actual[1].name).isEqualTo("맥북") | ||
| assertThat(actual[2].name).isEqualTo("그램") |
There was a problem hiding this comment.
assertAll로 묶는 것과 그렇지 않은 것은 어떤 차이가 있을까요?
There was a problem hiding this comment.
하나의 테스트 함수에서 여러개의 검증을 해야 할 경우, assertAll로 묶는 것이 좋다는 것을 알게 되었습니다!
assertAll(
"모든 상품을 확인",
{ assertThat(actual[0].name).isEqualTo("갤럭시북") },
{ assertThat(actual[1].name).isEqualTo("맥북") },
{ assertThat(actual[2].name).isEqualTo("그램") },
)
assertAll로 묶지 않고 각각 assertThat을 써주었을 경우, 위에서 실패한 단언문이 있을 경우 이후의 단언문이 실행되지 않고, assertAll로 묶을 경우에는 위에서 단언문이 실패하더라도 모든 단언문이 실행됨을 알 수 있었습니다. 따라서 지금같은 경우 assertAll로 묶어주었습니다. 만약 묶어주지 않을 경우, 여러 개의 단언문이 테스트에 실패할 경우에는 어떤 단언문이 실패하는지를 한 번에 알 수가 없게 되겠군요. 😄
| // then | ||
| assertEquals(CartsImpl.find(0), product.copy(id = 0)) | ||
| } |
여기에 앱 생명주기도 같이 끼워서 생각해보세요!
Provider로부터 생성하는 것은 결국 ViewModel 생성하는 것과 크게 다르지 않아보여요. |
안녕하세요 말리빈! 블랙잭 미션에서 만나뵀었는데 이번에 또 만나게 되었네요! 블랙잭 미션에서 많이 도와주셔서 크게 성장할 수 있었던 것 같습니다. 이번에도 잘부탁드립니다 🥰🥰
질문사항
ViewModel에 생성자로 데이터를 주입하기 위해 factory를 사용했습니다. 만약 factory를 사용하지 않으면 뷰모델의 인스턴스가 제대로 만들어지지 않는다고 하더라구요. 그런데 이 과정에서 사용한 ViewModelProvider는 안드로이드에 의존하고 있는 객체이기 때문에 unit 테스트에서는 사용하지 못한다는 것을 알게 되었습니다. 그런데 unit테스트에서는 ViewModelProvider를 사용하지 않아도 에러가 발생하지 않는다는 것을 알게 되어서, ViewModelProvider와 Factory를 사용하지 않고 바로 인스턴스를 만들었습니다. 이렇게 해도 괜찮은 것일까요?