[하티] 1, 2단계 쇼핑 장바구니 제출합니다. #19

sujin9 · 2023-05-11T08:50:56Z

안녕하세요, 토리! :)
이번 안드로이드 장바구니 미션 피드백 받게 된 하티라고 합니다! 😄
다시 뵙게 되어서 반가워요!!

이번 미션을 진행하면서는 시간 부족을 많이 느꼈어요..
그래서 아마도 구현하지 못한 기능이나 버그들이 많이 남아있을 것 같아요 :(
그리고 CartRepositoryImpl에서 CartDbHelpe가 제대로 create되지 않아 오류가 발생하는 경우가 있었는데,
CartDbHelperTest를 실행하고 다시 app을 실행하면 제대로 동작하더라구요.. 원인은 아직 찾지 못했지만, 혹시 몰라 말씀드려요!

이번 미션에서는 시간도 부족했고, 여러모로 아쉬움이 많이 남는 것 같습니다.
그럼에도 리뷰 많이 주시면 열심히 반영하면서 개선해볼게요!
미리 리뷰에 감사드립니다 ;>

- glide 추가

- 아이템이 없으면 영역 자체가 보이지 않게 된다

galcyurio

offset 기반의 페이징 처리와 MVP 패턴으로 잘 구현해주셨네요 👏
이번 미션에서도 다른 미션과 마찬가지로 테스트 코드를 작성해보세요.
현재 발생하는 오류에 대해서는 질문을 남겨두었으니 최대한 질문에 답해보고 마지막으로 힌트를 열어보세요.

galcyurio · 2023-05-11T23:12:11Z

app/src/main/java/woowacourse/shopping/database/cart/CartDbHelper.kt

+    override fun onCreate(db: SQLiteDatabase?) {
+        db?.execSQL(SQL_CREATE_ENTRIES)
+    }


그리고 CartRepositoryImpl에서 CartDbHelpe가 제대로 create되지 않아 오류가 발생하는 경우가 있었는데,
CartDbHelperTest를 실행하고 다시 app을 실행하면 제대로 동작하더라구요.. 원인은 아직 찾지 못했지만, 혹시 몰라 말씀드려요!

각 질문에 대해 스스로 답변해보세요.

onCreate() 함수는 언제 호출되는걸까요?

RecentlyViewedProductDbHelper, CartDbHelper의 onCreate() 함수가 둘 다 호출되나요?

하나의 DbHelper의 onUpgrade()에서 버전이 하나 올라갔으면 다른 하나의 onUpgrade()에서는 어떻게 될까요?

힌트

onCreate()와 onUpgrade()를 하나로 합쳐볼까요?

추가로 Robolectric을 이용해서 DB가 제대로 생성되었는지 확인하는 테스트코드를 작성해볼까요?
Robolectric 대신 androidTest 디렉토리에서 진행하셔도 좋습니다.

onCreate() 함수는 언제 호출되는걸까요?

readableDatabase 혹은 writableDatabase 가 실행될 때, 데이터베이스가 존재하지 않는 경우에만 open 됩니다. 즉, 시스템 안에 database가 없는 경우 딱 한 번만 호출됩니다.

RecentlyViewedProductDbHelper, CartDbHelper의 onCreate() 함수가 둘 다 호출되나요?

에뮬레이터에서 데이터를 지우고 처음부터 로그를 찍어보았는데, 앱을 실행하면 RecentlyViewedProductDbHelper는 onCreate() 되었지만 CartDbHelper의 onCreate()는 호출되지 않았어요.
저는 이미 RecentlyViewedProductDbHelper에서 db가 생성되었기 때문에 실행이 안 되는 것 같다고 이해하고 있는데, 이 이유가 맞을까요? 🤔

하나의 DbHelper의 onUpgrade()에서 버전이 하나 올라갔으면 다른 하나의 onUpgrade()에서는 어떻게 될까요?

역시나 다른 하나의 onUpgrade() 역시 호출되지 않을 것 같아요. 버전이 올라가면 그에 맞게 처음 (이 코드상에서는 RecentlyViewedProductDbHelper에서 db를 오픈할 때)에 onUpgrade가 실행이 될 것이고, 그 이후에 open을 할 때는 이미 버전이 업그레이드 된 db가 존재하기 때문에 호출되지 않을 것 같아요.

하나의 프로젝트에서 하나의 데이터베이스만 만들고 관리할 수 있도록 프로젝트 내에 DBHelper 파일을 하나만 만들었고,
그 안에서 여러 개의 테이블을 한 번에 모두 생성하도록 수정해보았습니다! 커밋 내역
이렇게 하는 방법이 맞을까요??

의도대로 잘 해주셨습니다 👍️
이번에 SQLite에서 DB가 어떻게 생성되고 업데이트되는지에 대해서는 확실하게 이해하셨네요!

app/src/main/java/woowacourse/shopping/ui/cart/CartPresenter.kt

...in/java/woowacourse/shopping/database/recentlyviewedproduct/RecentlyViewedProductDbHelper.kt

galcyurio · 2023-05-11T23:19:46Z

app/src/main/java/woowacourse/shopping/ui/productdetail/ProductDetailPresenter.kt

+) : ProductDetailContract.Presenter {
+    override fun loadProduct(productId: Long) {
+        productRepository.findById(productId)?.run {
+            view.setProduct(ProductDetailUIState.from(this))


UiState를 분리했을 때 어떤 이점이 있을까요?

사실 지금까지 UiState를 분리하는 것의 이점을 크게 느끼지는 못했던 것 같아요..! 현재 미션에서 느끼기에는, 우선 뷰에 그려질 때 필요한 상태들이 무엇인지 직관적으로 알 수 있다는 장점이 있는 것 같아요. 또한 상태들을 view에 전달할 때 readable하게 전달하여 부수효과를 줄일 수 있지 않을까 하는 생각이 듭니다!

공식문서나 블로그 등을 찾아보면서 느낀 점은 MVVM 패턴에서 관찰 가능한 데이터와 함께 주로 쓰이는 것 같고, 화면의 상태를 나타내는 1개만의 스트림을 둠으로써 ui가 변경되는 지점을 명확히 하고 가독성을 높여준다는 것 같았어요!

아직 잘 와닿지는 않지만, 이러한 내용들이 제가 잘 이해하고 있는 게 맞을까요? :) ㅠㅠ

말씀하신 것들 모두 맞습니다!
개인적으로 UiState가 유용했던 경험은 복잡한 형태의 UI를 만들 때 였습니다.
2~3개의 domain DTO를 받아서 UiState를 만들면 테스트도 쉽고 가독성도 좋았거든요 ㅎㅎ

app/src/main/java/woowacourse/shopping/ui/products/ProductListPresenter.kt

galcyurio · 2023-05-11T23:22:01Z

app/src/main/java/woowacourse/shopping/ui/products/ProductListActivity.kt

+        presenter.loadProducts(PAGE_SIZE, offset)
+        offset += PAGE_SIZE
+    }
+
+    private fun initLoadingButton() {
+        binding.btnLoading.setOnClickListener {
+            presenter.loadProducts(PAGE_SIZE, offset)
+            offset += PAGE_SIZE


페이징 처리와 관련된 코드들을 별도 클래스나 함수로 분리해볼까요?

피드백이 반영되지 않았습니다~

app/src/main/res/layout/activity_product_list.xml

- 하나의 DB는 DB Helper 파일을 하나만 갖도록 수정

galcyurio

1, 2 단계 미션 고생하셨습니다 👏👏
질문해주신 부분들에 대한 답변과 몇 가지 추가로 코멘트를 남겨두었습니다.
이번 피드백은 다음 단계에 같이 반영해주세요!

galcyurio · 2023-05-17T13:12:15Z

app/src/main/java/woowacourse/shopping/database/cart/CartDbHelper.kt

+    override fun onCreate(db: SQLiteDatabase?) {
+        db?.execSQL(SQL_CREATE_ENTRIES)
+    }


의도대로 잘 해주셨습니다 👍️
이번에 SQLite에서 DB가 어떻게 생성되고 업데이트되는지에 대해서는 확실하게 이해하셨네요!

galcyurio · 2023-05-17T13:13:02Z

app/src/test/java/woowacourse/shopping/ui/cart/CartPresenterTest.kt

+    }
+
+    @Test
+    fun 장바구니에_담은_상품을_삭제하면_장바구니_목록에_해당_상품이_삭제된다() {


@Test fun `이렇게 할 수도 있어요`() { }

galcyurio · 2023-05-17T13:13:41Z

app/src/test/java/woowacourse/shopping/ui/cart/CartPresenterTest.kt

+
+    @Test
+    fun 장바구니에_담은_상품을_삭제하면_장바구니_목록에_해당_상품이_삭제된다() {
+        every { view.setCartItems(any()) } answers { }


relaxed mock에 대해서 공부해보고 불필요한 stubbing 코드들을 제거해볼까요?

relaxed mock을 사용하게 되면 메소드들이 0, false, "" 와 같은 값을 반환하게 되어서 굳이 번거롭게 every를 사용할 필요가 없어집니다! 다음 단계에 반영하겠습니다 :)

galcyurio · 2023-05-17T13:14:45Z

app/src/test/java/woowacourse/shopping/ui/cart/CartPresenterTest.kt

+        presenter.deleteCartItem(2)
+        val product = FakeProductRepository.findById(2)
+
+        Assert.assertEquals(false, FakeCartRepository.findAll().contains(product))


테스트 코드에서는 가독성이 중요합니다.
조금 더 가독성을 향상시켜주는 assertion library를 활용해보시면 어떨까요?
개인적으로는 Truth를 추천드립니다.

테스트 코드에서는 assert문을 작성할 때, 보통 static import 기능을 활용해요!

galcyurio · 2023-05-17T13:18:12Z

app/src/main/java/woowacourse/shopping/ui/productdetail/ProductDetailPresenter.kt

+) : ProductDetailContract.Presenter {
+    override fun loadProduct(productId: Long) {
+        productRepository.findById(productId)?.run {
+            view.setProduct(ProductDetailUIState.from(this))


말씀하신 것들 모두 맞습니다!
개인적으로 UiState가 유용했던 경험은 복잡한 형태의 UI를 만들 때 였습니다.
2~3개의 domain DTO를 받아서 UiState를 만들면 테스트도 쉽고 가독성도 좋았거든요 ㅎㅎ

galcyurio · 2023-05-17T13:19:43Z

app/src/main/java/woowacourse/shopping/ui/products/ProductListActivity.kt

+        presenter.loadProducts(PAGE_SIZE, offset)
+        offset += PAGE_SIZE
+    }
+
+    private fun initLoadingButton() {
+        binding.btnLoading.setOnClickListener {
+            presenter.loadProducts(PAGE_SIZE, offset)
+            offset += PAGE_SIZE


피드백이 반영되지 않았습니다~

galcyurio · 2023-05-17T13:22:47Z

app/src/test/java/woowacourse/shopping/database/FakeCartRepository.kt

+import woowacourse.shopping.domain.Product
+import woowacourse.shopping.repository.CartRepository
+
+object FakeCartRepository : CartRepository {


mock 객체 대신에 Fake 구현체를 활용하셨네요 👍️

sujin9 added 23 commits May 9, 2023 14:27

docs: 기능 목록 초안 작성

048c578

feat: 도메인 모듈 및 상품 클래스 추가

ae77603

feat: 필요한 repository들의 interface 추가

a38fab3

feat: 상품에 관한 database 및 repository 구현

6c6e33f

feat: 장바구니에 관한 database 및 repository 구현

21872ff

feat: 최근 본 상품에 관한 database 및 repository 구현

35e643f

refactor: 필요 없는 데이터베이스 제거 후 로컬 데이터 이용하도록 구현

37cb40f

feat: 상품 리스트를 그리드 리사이클러뷰로 보이도록 구현

3976e44

feat: 상품 리사이클러뷰 내부 아이템 이미지 및 이름, 가격 보이도록 구현

1e0a4ef

- glide 추가

feat: 상품 상세 페이지 구현 및 상품 클릭->상품 상세 페이지 이동 기능 구현

a748268

feat: 장바구니 상품 담기 및 상품 상세 페이지->장바구니 페이지 이동 기능 구현

9677a70

feat: 장바구니 페이지 리사이클러뷰 구현 및 아이템 삭제 기능 구현

f4429a6

feat: 최근 본 상품 리사이클러뷰 레이아웃 구현

e28f2bf

feat: 최근 본 상품 리사이클러뷰 기능 구현

119945b

- 아이템이 없으면 영역 자체가 보이지 않게 된다

feat: main의 App Bar 색상 및 장바구니 이동 버튼 추가

940982e

feat: 장바구니와 상품 상세 페이지 App Bar 커스텀 구현

f9e11af

fix: 색상으로 인해 보이지 않는 아이콘 색 추가

5d1d032

refactor: 패키지 변경

0a95a3e

feat: 최근 본 상품이 db에는 계속 저장되고, 불러올 때만 최신 10개를 가져오도록 구현

29901a9

feat: 장바구니 아이템이 페이지마다 5개씩 보이는 기능 구현

7bb1b02

feat: 상품 리스트의 아이템이 더보기 버튼을 누를 때마다 20개씩 더 보이도록 구현

1180713

fix: 처음 실행했을 때 빈 리스트가 전달되는 로직 오류 수정

57de1d3

test: Cart Table create test 추가

691cdc2

galcyurio requested changes May 11, 2023

View reviewed changes

sujin9 added 6 commits May 13, 2023 17:17

feat: NestedScrollView 적용

f512de9

fix: 테이블이 생성되지 않는 문제 해결

c50e677

- 하나의 DB는 DB Helper 파일을 하나만 갖도록 수정

test: db 및 테이블이 생성되었는지 테스트

667ee96

test: presenter test 추가

3a50cba

refactor: multiline string 적용

e7a39e6

fix: item이 더이상 없을 때 더보기 버튼을 누르면 종료되는 문제 해결

b5bbd50

sujin9 added 6 commits May 17, 2023 14:23

refactor: 중복으로 사용되는 가격 포맷을 별도의 파일로 분리

27f9ad0

refactor: 함수 분리

fc7dc5e

refactor: 에러 문구 string resource 에 추가

d371dd7

refactor: 함수 분리

c227973

fix: 앱 첫 실행 시 최근 본 항목이 제대로 뜨지 않는 문제 해결

f2ba08f

refactor: 매직 넘버 상수화

a18e8dd

galcyurio approved these changes May 17, 2023

View reviewed changes

galcyurio merged commit 49171ab into woowacourse:sujin9 May 17, 2023

[하티] 1, 2단계 쇼핑 장바구니 제출합니다. #19

[하티] 1, 2단계 쇼핑 장바구니 제출합니다. #19

Uh oh!

Conversation

sujin9 commented May 11, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

galcyurio left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

힌트

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sujin9 May 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sujin9 May 17, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

galcyurio left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sujin9 May 19, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

sujin9 commented May 11, 2023 •

edited

Loading

sujin9 May 13, 2023 •

edited

Loading

sujin9 May 17, 2023 •

edited

Loading

sujin9 May 19, 2023 •

edited

Loading