-
Notifications
You must be signed in to change notification settings - Fork 71
[쇼핑 주문 1, 2 단계] 채넛 미션 제출합니다. #99
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
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Reviews pausedUse the following commands to manage reviews:
Summary by CodeRabbit
Walkthrough이 변경사항은 안드로이드 쇼핑 애플리케이션의 핵심 구조와 기능을 대폭 확장하는 대규모 초기 구현입니다. 데이터 계층에서는 Room 기반 로컬 데이터베이스, Retrofit 기반 네트워크 통신, 다양한 데이터 소스 및 리포지토리 패턴이 도입되었습니다. 도메인 계층에는 장바구니, 상품, 최근 본 상품 등의 모델과 관련 인터페이스가 추가되었습니다. 프레젠테이션 계층에서는 MVVM 패턴 기반의 ViewModel, LiveData, 데이터 바인딩, 커스텀 어댑터, 액티비티, UI 상태 관리, 바인딩 어댑터 등이 구현되었습니다. 또한, 장바구니, 상품 목록, 상품 상세, 추천 상품 등 주요 화면의 레이아웃과 리소스(드로어블, 컬러, 벡터 아이콘 등)가 추가되었습니다. 테스트를 위한 Fixture, Dao 테스트, UI 테스트 코드도 포함되어 있습니다. Manifest, Gradle 설정, README 등 프로젝트의 전반적인 환경 구성도 함께 이루어졌습니다. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Actionable comments posted: 74
🧹 Nitpick comments (115)
app/src/main/res/drawable/ic_previous.xml (1)
1-9
: 벡터 드로어블 색상 및 헤더 선언 일관성 개선 제안
android:fillColor="#ffffff"
처럼 색상을 하드코딩하는 대신,@android:color/white
나 프로젝트 색상 리소스(@color/…
)를 참조하도록 변경해 보세요.
또한 XML 버전 선언(<?xml version="1.0" encoding="utf-8"?>
)을 추가하면 다른 리소스 파일과 일관성을 유지할 수 있습니다.app/src/main/res/drawable/ic_add.xml (1)
1-21
: 색상 및 투명도 리소스 참조 사용 권장
android:fillColor="#ffffff"
,android:fillColor="#00000000"
등 하드코딩된 색상 대신@android:color/white
,@android:color/transparent
또는 프로젝트 색상 리소스(@color/…
)를 사용해보세요.android:strokeColor="#555555"
도 마찬가지로 리소스에서 관리하면 유지보수가 용이해집니다.app/src/main/res/drawable/bg_cart_quantity_box.xml (1)
5-7
: 배경 색상 리소스 일관성 검토
<solid android:color="@android:color/white" />
대신 프로젝트 색상 리소스(@color/white
)를 사용하거나, 디자인 시스템에서 정의한 테마 속성을 참조하는게 좋습니다.app/src/main/res/drawable/ic_exit.xml (1)
1-9
: 색상 하드코딩 대신 리소스/테마 속성 사용 제안
android:fillColor="#ffffff"
를 하드코딩하는 대신@android:color/white
또는 프로젝트 색상 리소스를 이용하면 테마 변경 시 유지보수가 수월해집니다.app/src/main/res/drawable/round_btn_cart_next.xml (2)
2-2
:shape
타입 명시 권장
현재<shape>
태그에android:shape
속성이 없으므로 기본값에 의존합니다. 명시적으로android:shape="rectangle"
을 추가하면 가독성과 유지보수성이 향상됩니다.- <shape xmlns:android="http://schemas.android.com/apk/res/android"> + <shape xmlns:android="http://schemas.android.com/apk/res/android" + android:shape="rectangle">
4-8
: 코너 반경 값 상수화 제안
여러 곳에서 반복 사용되는4dp
값을dimens.xml
에 정의(@dimen/corner_radius_small
등)하여 중앙에서 관리하면 일관성과 재사용성이 높아집니다.- android:topRightRadius="4dp" - android:bottomRightRadius="4dp" + android:topRightRadius="@dimen/corner_radius_small" + android:bottomRightRadius="@dimen/corner_radius_small"app/src/main/res/drawable/round_btn_cart_previous.xml (2)
2-2
:shape
타입 명시 권장
<shape>
태그에android:shape="rectangle"
을 추가하여 기본값 의존을 줄이고 명확하게 타입을 지정하는 것이 좋습니다.- <shape xmlns:android="http://schemas.android.com/apk/res/android"> + <shape xmlns:android="http://schemas.android.com/apk/res/android" + android:shape="rectangle">
4-8
: 코너 반경 값 상수화 제안
4dp
값을 직접 하드코딩하기보다 공통된@dimen
리소스로 분리하면 유지보수와 확장에 유리합니다.- android:topLeftRadius="4dp" - android:bottomLeftRadius="4dp" + android:topLeftRadius="@dimen/corner_radius_small" + android:bottomLeftRadius="@dimen/corner_radius_small"app/src/main/res/drawable/ic_delete.xml (2)
1-1
: XML 프로로그 추가 고려
다른 리소스 파일과의 일관성을 위해 파일 상단에<?xml version="1.0" encoding="utf-8"?>
프로로그를 추가하는 것을 권장합니다.+ <?xml version="1.0" encoding="utf-8"?> <vector xmlns:android="http://schemas.android.com/apk/res/android"
7-7
: 테마 속성 사용 제안
현재android:fillColor="@color/gray5"
로 고정된 색상을 사용 중인데,?attr/colorOnSurface
등 테마 속성을 활용하면 다크 모드나 테마 변경 시 유연하게 대응할 수 있습니다.app/src/main/res/color/checkbox_selector.xml (1)
3-6
: 체크되지 않은 상태 명시 권장
기본 아이템으로 unchecked 상태를 처리하지만,android:state_checked="false"
를 명시하거나state_enabled="false"
항목을 추가하여 비활성화 등 예외 상황을 명확히 표시하는 것이 좋습니다.<selector xmlns:android="http://schemas.android.com/apk/res/android"> + <!-- 체크 안됨을 명시적으로 정의 --> + <item android:color="@color/baemin" android:state_checked="false"/>app/src/main/res/drawable/ic_back.xml (2)
1-1
: XML 프로로그 추가 건의
벡터 드로어블 상단에<?xml version="1.0" encoding="utf-8"?>
를 추가하면 다른 리소스 파일과 형식을 일치시킬 수 있습니다.+ <?xml version="1.0" encoding="utf-8"?> <vector xmlns:android="http://schemas.android.com/apk/res/android"
5-11
: 색상 지정 중복 제거
android:tint="@color/white"
와<path android:fillColor="@android:color/white">
가 같은 값을 중복 지정하고 있습니다. 둘 중 하나로 통일하여 유지보수성을 높이세요.- android:tint="@color/white" ... - <path - android:fillColor="@android:color/white" + <path + android:fillColor="@color/white"app/src/main/java/woowacourse/shopping/domain/model/Price.kt (1)
3-10
: 도메인 모델링을 위한 value class 설계가 잘 되어 있습니다!
@JvmInline
을 사용한 value class 선택이 좋네요. 몇 가지 생각해볼 점들이 있습니다:
비즈니스 규칙 검증: 현재 0 이상의 값만 허용하고 있는데, 실제 쇼핑몰에서는 어떤 추가적인 제약사항이 있을까요? 예를 들어 최대 가격 제한이나 특정 단위(예: 10원 단위) 제약은 어떨까요?
에러 메시지: 한국어 메시지를 사용하셨는데, 국제화(i18n)를 고려한다면 어떤 방식으로 접근할 수 있을까요?
확장성: 향후 가격과 관련된 연산(할인, 세금 계산 등)이 필요하다면 이 클래스에 어떤 메서드들을 추가할 수 있을까요?
도메인 주도 설계(DDD)의 Value Object 패턴을 잘 적용하신 것 같습니다. 이런 타입 안전성이 어떤 장점을 가져다주는지 생각해보세요.
app/src/main/res/drawable/ic_cart.xml (1)
1-9
: 벡터 드로어블 리소스 설계에 대해 생각해볼 점들이 있습니다아이콘 디자인을 직접 구현하셨네요. 몇 가지 고려사항들을 생각해보시면 좋을 것 같습니다:
크기 고정 vs 유연성: 현재 22dp x 17dp로 고정되어 있는데, 다양한 화면에서 사용할 때 어떤 문제가 생길 수 있을까요?
wrap_content
나 다른 크기로 사용해야 할 상황은 없을까요?색상 접근성: 흰색(#ffffff) 고정색을 사용하고 있는데, 다크모드나 다양한 배경에서는 어떻게 보일까요? 색상을 더 유연하게 처리할 수 있는 방법은 무엇일까요?
복잡도 vs 유지보수성: path 데이터가 매우 복잡한데, Material Design Icons나 다른 아이콘 라이브러리를 사용하는 것과 비교했을 때의 장단점은 무엇일까요?
일관성: 다른 아이콘들(
ic_loading.xml
,ic_back.xml
등)과 스타일이나 크기 체계에서 일관성을 유지하고 있는지 확인해보세요.UI 리소스 설계 원칙에 대해 생각해보시고, Android Design Guidelines를 참고해보시는 것도 좋겠습니다.
app/src/main/res/drawable/ic_loading.xml (1)
1-13
: 로딩 인디케이터 디자인이 깔끔하네요!벡터 기반 로딩 아이콘을 잘 구현하셨습니다. 사용자 경험 관점에서 몇 가지 생각해볼 점들이 있습니다:
애니메이션: 현재는 정적 아이콘인데, 실제 로딩 상태를 표현하려면 어떤 애니메이션이 필요할까요?
AnimationDrawable
이나AnimatedVectorDrawable
을 고려해보셨나요?테마 적용: 검은색 stroke를 사용하고 있는데, 라이트/다크 테마에서 어떻게 보일까요?
?attr/colorOnSurface
같은 테마 색상을 사용하는 방법은 어떨까요?접근성: 스크린 리더 사용자들에게 로딩 상태를 어떻게 전달할 수 있을까요? 이 아이콘을 사용하는 View에서 고려해야 할 접근성 속성들은 무엇일까요?
UX 패턴: 이 로딩 아이콘이 사용될 컨텍스트(전체 화면 로딩 vs 부분 로딩)에 따라 크기나 스타일을 다르게 해야 할 상황은 없을까요?
Android의 로딩 UX 패턴과 Material Design 가이드라인을 참고해보시면 도움이 될 것 같습니다.
app/src/main/java/woowacourse/shopping/presentation/Extra.kt (1)
3-8
: 상수 관리를 체계적으로 하고 계시네요!인텐트 키들을 중앙집중식으로 관리하는 접근법이 좋습니다. 더 나은 설계를 위해 생각해볼 점들이 있습니다:
타입 안전성: 현재는 String 키만 정의되어 있는데, 실제 데이터를 주고받을 때 타입 실수를 방지할 수 있는 방법은 없을까요? 예를 들어,
putExtra
와getExtra
를 감싸는 타입 안전한 wrapper는 어떨까요?네임스페이스: 앱이 커지면서 다른 종류의 키들(preference keys, bundle keys 등)이 추가될 때를 대비해서 어떻게 구조화할 수 있을까요?
문서화: 각 키가 어떤 타입의 데이터를 담는지, 어느 액티비티 간에 사용되는지 주석으로 명시하면 어떨까요?
리팩토링 안전성: IDE의 리팩토링 기능을 활용할 때, 이런 String 상수들의 한계는 무엇이고 어떻게 보완할 수 있을까요?
Kotlin의 sealed class나 enum을 활용한 더 타입 안전한 접근법도 고려해보시면 좋겠습니다.
app/src/main/res/drawable/ic_next.xml (1)
1-9
: 접근성 강화를 위한 사용 시 contentDescription 설정
벡터 자체에는contentDescription
속성이 없으므로, 이 아이콘을 사용하는ImageView
또는ActionBar
등에 반드시 적절한android:contentDescription
을 지정해 주세요.app/src/main/java/woowacourse/shopping/presentation/cart/CartCounterClickListener.kt (1)
1-7
: 인터페이스에 KDoc 추가 고려
CartCounterClickListener
와 각 메서드에 간단한 KDoc을 달아 역할과 파라미터(id)의 의미를 명확히 표현하면 유지보수성이 향상됩니다.app/src/main/java/woowacourse/shopping/data/dto/cart/SortX.kt (1)
1-14
: DTO 클래스명 명확성 검토
SortX
라는 이름이 다소 모호할 수 있습니다. API 사양에 맞춰SortInfo
,SortDto
등 좀 더 직관적인 이름으로 변경하는 방안을 고려해 보세요.app/src/main/java/woowacourse/shopping/presentation/product/ItemClickListener.kt (1)
1-9
: 인터페이스에 KDoc 추가 고려
ItemClickListener
의 메서드 동작과 파라미터(productId
,cartItem
)의 역할을 설명하는 KDoc을 추가하면, 협업 시 이해도를 높일 수 있습니다.app/src/main/java/woowacourse/shopping/data/dto/product/SortX.kt (1)
7-14
: 클래스 설계에서 개선할 점들을 찾아보세요.현재 구현에서 몇 가지 생각해볼 점들이 있습니다:
SortX
라는 클래스명이 무엇을 나타내는지 명확하지 않습니다.sorted
와unsorted
필드가 동시에 존재하는 이유를 생각해보세요.질문:
- 이 두 불린 값이 서로 반대의 의미라면, 하나만 사용해도 되지 않을까요?
- 클래스명을 더 의미있게 만들 수 있는 방법이 있을까요?
힌트: 실제 API 응답 구조를 확인해보고, 각 필드의 역할을 명확히 이해한 후 최적의 설계를 고민해보세요.
app/src/main/java/woowacourse/shopping/data/dto/cart/UpdateCartRequest.kt (2)
3-3
: 코드 포맷팅을 깔끔하게 정리해보세요.불필요한 빈 줄이 있습니다. 일관된 코드 스타일을 유지하는 것이 좋습니다.
8-11
: 데이터 유효성에 대해 생각해보세요.현재
quantity
가Int
타입으로 되어있는데, 장바구니 수량에 대해 고려해야 할 제약사항들이 있을까요?생각해볼 점들:
- 음수 수량이 가능한가요?
- 0은 어떤 의미인가요?
- 매우 큰 수량은 제한해야 하나요?
힌트: DTO 레벨에서 검증할지, 다른 레이어에서 검증할지도 함께 고민해보세요. 각 레이어의 책임을 명확히 하는 것이 중요합니다.
app/src/main/java/woowacourse/shopping/domain/model/CartItem.kt (1)
4-4
: 기본값 설정에 대해 신중히 생각해보세요.
cartId
에 기본값0L
이 설정되어 있습니다.질문들:
- 0이 유효한 ID 값인가요?
- 새로운 카트 아이템을 생성할 때 ID가 없는 상황을 어떻게 처리해야 할까요?
- 데이터베이스에서 0이 실제 ID로 사용될 가능성이 있나요?
힌트: 도메인 모델에서 ID의 의미와 라이프사이클을 고려해보세요.
app/src/main/java/woowacourse/shopping/data/dto/cart/CartItemRequest.kt (1)
7-12
: 데이터 유효성 검증에 대해 생각해보세요.현재 DTO가 깔끔하게 구현되어 있네요! 다만 한 가지 고민해볼 점이 있어요:
quantity
나productId
에 대한 유효성 검증이 필요할까요? 예를 들어:
quantity
가 0 이하의 값이 올 수 있을까요?productId
가 음수가 될 수 있을까요?이런 검증을 DTO에서 해야 할지, 아니면 다른 계층(예: Repository나 UseCase)에서 처리하는 게 좋을지 생각해보세요. 각 접근 방식의 장단점은 무엇일까요?
app/src/androidTest/java/woowacourse/shopping/dao/CartDaoTest.kt (1)
30-68
: 테스트 케이스를 더 견고하게 만들어보세요.현재 기본적인 CRUD 기능을 잘 테스트하고 있어요! 추가로 고려해볼 테스트 시나리오들이 있을까요?
예를 들어:
- 존재하지 않는 상품 ID로 삭제를 시도하면?
- 페이지 크기가 0이거나 음수일 때는?
- 중복된 상품을 삽입하려고 하면?
어떤 예외 상황들이 실제 사용 중에 발생할 수 있을지 생각해보시고, 그런 케이스들도 테스트해볼 가치가 있는지 고민해보세요. 테스트의 목적은 무엇인가요?
app/src/main/java/woowacourse/shopping/data/dto/cart/CartItemCountResponse.kt (2)
1-3
: 코드 포맷팅을 확인해보세요.패키지 선언 다음에 빈 줄이 하나 더 있는 것 같아요. 프로젝트의 코딩 스타일 가이드에 따라 일관성을 맞춰보시는 게 어떨까요?
8-11
: API 응답 데이터의 유효성에 대해 생각해보세요.서버에서 받는
quantity
값이 항상 유효할 것이라고 가정할 수 있을까요?네트워크를 통해 받는 데이터의 경우:
- 예상치 못한 음수 값이 올 수 있을까요?
- 클라이언트에서 이런 값들을 어떻게 처리하는 게 좋을까요?
다른 DTO들(예:
CartItemRequest
)과 비교해서 검증 방식의 일관성도 고려해보세요.app/src/main/java/woowacourse/shopping/data/CartItemMapper.kt (1)
10-15
: 주석 처리된 미완성 코드를 정리해보세요.현재
toDomain
메서드가 주석 처리되어 있고 미완성 상태입니다.이런 상황에서 어떤 선택을 할 수 있을까요?
- 현재 필요하지 않다면 코드를 제거하는 것이 좋을까요?
- 나중에 구현할 예정이라면 어떤 방식으로 표시하는 것이 좋을까요?
코드베이스를 깔끔하게 유지하는 방법에 대해 생각해보세요.
app/src/main/java/woowacourse/shopping/data/dto/product/Pageable.kt (1)
6-20
: 데이터 클래스 설계가 잘 되어있습니다.페이지네이션 정보를 담는 DTO로서 적절한 구조를 가지고 있습니다.
한 가지 생각해볼 점이 있습니다:
SortX
라는 타입명을 보면 어떤 의미인지 직관적으로 알 수 있나요?- 네이밍 컨벤션 관점에서 더 명확한 이름이 있을까요?
또한 모든 프로퍼티가 nullable하지 않게 설계된 것도 좋은 선택입니다. API 응답의 특성을 고려했을 때 이런 설계가 적절한지 생각해보세요.
app/src/main/java/woowacourse/shopping/data/entity/RecentlyViewedProduct.kt (1)
6-14
: 엔티티 설계가 잘 되어있지만 테스트 관점에서 개선해보세요.Room 엔티티로서 필요한 정보들이 잘 정의되어 있습니다.
한 가지 고려해볼 점이 있습니다:
viewedAt
필드에서System.currentTimeMillis()
를 기본값으로 사용하고 있는데, 이것이 테스트 작성 시 어떤 어려움을 만들 수 있을까요?- 시간 값을 외부에서 주입받을 수 있는 방법은 없을까요?
또한 데이터 클래스에서 기본값을 가진 매개변수의 위치에 대해서도 생각해보세요. 일반적으로 어떤 순서로 배치하는 것이 좋을까요?
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewModelFactory.kt (1)
7-14
: 의존성 주입 패턴과 타입 안전성 개선을 고려해보세요.현재 구현에서 Service Locator 패턴을 사용하고 있는데, 다음과 같은 개선점을 생각해볼 수 있습니다:
- 타입 캐스팅
as T
가 안전한지 검증하는 방법은 무엇일까요?- RepositoryModule에 직접 의존하는 것 대신 다른 의존성 주입 방식을 고려해본 적이 있나요?
이런 구조에서 발생할 수 있는 잠재적 문제점들을 어떻게 해결할 수 있을지 생각해보세요.
app/src/androidTest/java/woowacourse/shopping/view/ProductDetailActivityTest.kt (1)
33-56
: 테스트 메서드 네이밍과 테스트 깊이를 개선해보세요.현재 테스트들이 UI 요소의 표시 여부만 확인하고 있는데:
- 한글 테스트 메서드명이 팀의 컨벤션에 맞는지 확인해보셨나요?
- 각 UI 요소가 올바른 데이터를 표시하는지도 검증해볼 필요가 있지 않을까요?
- 사용자 상호작용(버튼 클릭 등)에 대한 테스트도 고려해보셨나요?
더 포괄적인 테스트 시나리오를 설계해보시는 것을 추천합니다.
app/src/main/java/woowacourse/shopping/presentation/cart/CartViewModelFactory.kt (1)
7-12
: 팩토리 클래스들의 중복된 패턴을 개선해보세요.다른 ViewModelFactory들과 비교해보면 거의 동일한 구조를 가지고 있습니다:
- 이런 반복적인 팩토리 클래스들을 제네릭을 활용해서 통합할 수 있는 방법은 없을까요?
- 모든 팩토리에서 동일한 의존성 주입 패턴을 사용하는 것이 최선의 방법일까요?
코드 중복을 줄이고 유지보수성을 높이는 방향으로 리팩토링을 고려해보세요.
app/src/main/java/woowacourse/shopping/domain/model/Product.kt (1)
6-10
: 가격 정보 캡슐화에 대한 설계 고민
_price
를 private으로 숨기고price
프로퍼티를 통해 값만 노출하는 설계를 선택하셨네요.이런 캡슐화 패턴을 선택한 이유에 대해 생각해보셨나요? Price 객체의 어떤 부분을 숨기고 싶었던 건지, 그리고 이런 설계가 코드의 유지보수성에 어떤 영향을 미칠지 고민해보시면 좋을 것 같습니다.
힌트: Price 클래스의 구현을 살펴보고, 도메인 계층에서 데이터를 어떻게 보호하고 있는지 확인해보세요.
app/src/main/java/woowacourse/shopping/data/dto/cart/Pageable.kt (2)
14-19
: 페이지네이션 상태 모델링에 대한 고민
paged
와unpaged
두 개의 Boolean 프로퍼티가 동시에 존재하네요.이런 설계에서 두 값이 어떤 관계를 가져야 할지 생각해보셨나요? 예를 들어
paged=true, unpaged=true
인 상황이 발생할 수 있을까요?API 응답의 구조를 다시 한 번 확인해보시고, 이런 중복된 정보를 어떻게 더 명확하게 표현할 수 있을지 고민해보시면 좋겠습니다.
17-17
: 타입 네이밍 컨벤션 점검
SortX
라는 타입명을 사용하고 계시네요.일반적으로 클래스명에
X
접미사를 붙이는 것은 어떤 의미인지 생각해보셨나요? 이 타입이 어떤 역할을 하는지, 그리고 더 명확한 이름으로 표현할 수 있는지 고민해보시면 좋을 것 같습니다.힌트: 정렬 정보를 담는 클래스라면 그 역할이 드러나는 이름을 고려해보세요.
app/src/main/java/woowacourse/shopping/presentation/bindingadapter/ButtonBindingAdapter.kt (2)
9-16
: UI 상태 표현 방식에 대한 고민버튼의 활성화 상태를 색상으로만 구분하고 계시네요.
접근성 관점에서 생각해보면, 색상만으로 상태를 구분하는 것이 모든 사용자에게 충분할까요? 색각 장애가 있는 사용자들은 어떻게 이 상태를 구분할 수 있을지 고민해보셨나요?
Android의 접근성 가이드라인을 확인해보시고, 색상 외에 어떤 추가적인 시각적 단서를 제공할 수 있는지 생각해보시면 좋겠습니다.
9-12
: 네이밍 일관성 점검
@BindingAdapter("isEnabledCyanOrGray")
와 함수명setCyanOrGrayTint
를 보니 두 이름이 약간 다른 스타일로 되어 있네요.바인딩 어댑터의 속성명과 함수명 사이에 일관된 네이밍 컨벤션을 적용하는 것에 대해 어떻게 생각하시나요?
프로젝트 전체에서 바인딩 어댑터들의 네이밍 패턴을 한 번 정리해보시는 것은 어떨까요?
app/src/androidTest/java/woowacourse/shopping/dao/RecentProductDaoTest.kt (1)
38-38
: 테스트 가독성 개선 기회테스트에서
3
,0
,2
같은 매직 넘버들이 사용되고 있네요.이런 숫자들이 어디서 나온 건지, 그리고
Fixture.dummyRecentlyViewedProductList
의 구조와 어떤 관계가 있는지 명확하게 드러나고 있을까요?테스트 코드를 처음 보는 사람도 쉽게 이해할 수 있도록 하려면 어떤 방법들을 사용할 수 있을지 고민해보시면 좋겠습니다.
Also applies to: 51-51, 63-63, 73-73
app/src/main/java/woowacourse/shopping/presentation/recommend/ItemDiffUtil.kt (1)
6-16
: DiffUtil 구현의 성능 최적화를 고려해보세요현재 구현에서
areContentsTheSame
메서드가 객체 전체 비교(==
)를 사용하고 있네요. 이에 대해 몇 가지 질문을 해보겠습니다:
CartItemUiModel
에서equals()
메서드가 모든 필드를 비교하도록 구현되어 있나요?- UI 업데이트에 실제로 영향을 주는 필드들만 비교하면 더 효율적일까요?
- 추천 화면에서 자주 변경되는 필드와 그렇지 않은 필드를 구분해볼 수 있을까요?
힌트: RecyclerView의 성능을 위해서는 실제로 UI에 영향을 주는 변경사항만 감지하는 것이 좋습니다.
app/src/androidTest/java/woowacourse/shopping/view/CartActivityTest.kt (1)
20-23
: Intents 설정의 목적을 명확히 해보세요
@Before
와@After
에서Intents.init()
과Intents.release()
를 설정했는데:
- 현재 테스트에서 Intent 관련 검증을 하고 있나요?
- 만약 Intent 테스트가 없다면, 이 설정이 필요한 이유는 무엇일까요?
불필요한 설정은 테스트 실행 시간을 늘릴 수 있으니, 실제 필요성을 검토해보세요.
app/src/main/java/woowacourse/shopping/presentation/ResultState.kt (1)
3-13
: 잘 설계된 상태 관리 클래스입니다!sealed class를 활용한 상태 표현이 타입 안전성을 보장하며, covariance(
out T
) 사용도 적절합니다.다만 한 가지 고려해볼 점이 있습니다:
Failure
상태에서throwable
이 nullable인 이유가 무엇인지 생각해보세요. 어떤 상황에서 에러 정보 없이 실패 상태를 표현하게 될까요? 이런 경우가 실제로 필요한지, 아니면 더 구체적인 에러 정보를 항상 제공하는 것이 좋을지 팀과 논의해보시면 어떨까요?app/src/main/java/woowacourse/shopping/presentation/cart/CartPageClickListener.kt (1)
5-13
: 깔끔하고 목적이 명확한 인터페이스 설계입니다!각 메서드의 역할이 분명하고 네이밍도 직관적입니다.
한 가지 더 생각해볼 점은 인터페이스 분리 원칙(ISP)입니다. 현재 4개의 서로 다른 기능이 하나의 인터페이스에 있는데, 이들을 더 작은 단위로 분리하는 것이 좋을지 고려해보세요. 예를 들어
onClickRecommend()
는 다른 기능들과 성격이 다를 수 있겠네요. 어떻게 생각하시나요?README.md (2)
3-3
: 문서 구조를 개선해보세요.마크다운 표준에 따르면 헤딩 레벨은 순차적으로 증가해야 합니다. 현재
# android-shopping-order
다음에 바로### 1단계 기능 목록
이 오는데, 이 부분을 어떻게 개선할 수 있을까요?마크다운 헤딩 레벨에 대한 모범 사례를 찾아보시고, 문서의 계층 구조를 다시 생각해보세요.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3(MD001, heading-increment)
14-16
: 리스트 들여쓰기를 일관성 있게 맞춰보세요.중첩된 리스트의 들여쓰기가 표준과 맞지 않습니다. 마크다운에서 중첩 리스트는 보통 2칸 들여쓰기를 사용하는데, 현재는 4칸으로 되어 있네요.
마크다운 리스트 작성 규칙을 확인해보시고, 일관된 스타일로 정리해보시면 어떨까요?
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
14-14: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
app/src/main/java/woowacourse/shopping/data/remote/ProductService.kt (2)
10-22
: 비동기 처리 패턴에 대해 생각해보세요현재
Call<T>
반환 타입을 사용하고 있는데, 이것이 최선의 선택일까요?다음 질문들을 스스로에게 해보세요:
- 코틀린의 코루틴과
suspend
함수를 사용하면 어떤 장점이 있을까요?- 네트워크 에러나 파싱 에러가 발생했을 때 현재 구조에서는 어떻게 처리하게 될까요?
Call<T>
와suspend
함수 중 어느 것이 더 현대적이고 테스트하기 쉬운 방식일까요?코루틴 공식 문서를 참고해서 비동기 처리의 다른 방법들을 살펴보시길 권합니다.
10-22
: API 인터페이스 설계 개선 고려사항현재
Call<T>
방식으로 구현되어 있는데, 최신 Android 개발 트렌드를 고려할 때 어떤 대안들이 있을까요?다음 질문들을 생각해보세요:
- 코루틴을 사용한다면 반환 타입을 어떻게 개선할 수 있을까요?
- 네트워크 에러나 파싱 에러를 어떻게 일관성 있게 처리할 수 있을까요?
- Retrofit의
suspend
함수를 사용하면 어떤 장점이 있을까요?힌트:
suspend
키워드와Result<T>
또는 sealed class 활용을 검토해보세요.app/src/main/java/woowacourse/shopping/di/DatabaseModule.kt (1)
16-16
: 싱글톤 패턴 구현을 다시 살펴보세요현재 데이터베이스 인스턴스 관리 방식에서 개선할 점이 있을까요?
질문해보세요:
CartDatabase.getInstance()
가 이미 싱글톤이라면, 여기서 추가로 캐싱하는 이유는 무엇일까요?database
변수에 값을 할당하는also
의 목적은 무엇이고, 이것이 필요한가요?- 더 간단하고 명확한 방법은 없을까요?
Room 데이터베이스의 인스턴스 관리 방식을 문서에서 확인해보고, 중복된 싱글톤 관리가 필요한지 판단해보세요.
app/src/main/java/woowacourse/shopping/domain/repository/RecentProductRepository.kt (2)
8-8
: 메서드 이름의 의도를 명확히 해보세요
getMostRecentProduct
가Product?
를 반환하는데, 메서드 이름만으로 null이 반환될 수 있다는 것을 알 수 있을까요?고민해보세요:
- 언제 null이 반환될까요? (최근 상품이 없을 때?)
- 메서드 이름으로 이런 상황을 더 명확하게 표현할 수 있을까요?
- 도메인 언어로 이 동작을 어떻게 설명하는 것이 더 좋을까요?
도메인 주도 설계(DDD)의 관점에서 메서드 이름이 비즈니스 의도를 명확하게 전달하는지 생각해보세요.
5-14
: 비동기 패턴과 네이밍 일관성 개선 고려잘 설계된 리포지토리 인터페이스입니다! 다음 관점에서 한 번 더 검토해보세요:
비동기 패턴:
- 현재 콜백 패턴을 사용하고 있는데, 코루틴 Flow나 suspend 함수를 사용하면 어떤 장점이 있을까요?
- 테스트 작성 시 콜백 검증과 코루틴 테스트 중 어느 것이 더 용이할까요?
네이밍 일관성:
getRecentProducts
와insertRecentProduct
에서 단수/복수 형태가 다른 이유가 있나요?getMostRecentProduct
의 네이밍이 메서드의 의도를 명확히 표현하고 있나요?함수 시그니처:
insertRecentProduct
에서 성공/실패만 알면 되는 경우Result<Unit>
대신 다른 방식은 없을까요?이러한 관점들을 바탕으로 API 일관성을 높일 수 있는 방법을 생각해보세요.
app/src/main/java/woowacourse/shopping/data/datasource/remote/ProductDataSource.kt (1)
6-11
: 매개변수 순서와 명명을 검토해보세요함수의 매개변수 순서가 사용성 측면에서 적절한지 생각해보세요.
고려사항들:
- 필수 매개변수와 옵셔널 매개변수의 순서가 논리적인가요?
onResult
콜백이 마지막에 오는 것이 코틀린의 관례와 맞나요?- 매개변수 이름들이 의미를 명확하게 전달하고 있나요?
코틀린 코딩 컨벤션과 함수형 프로그래밍의 관례를 참고해서 더 나은 시그니처를 고민해보세요.
app/src/main/res/layout-land/activity_detail_product.xml (1)
93-106
: 버튼 스타일링과 사용성을 검토해보세요.현재 버튼에
android:background
와android:backgroundTint
모두 설정되어 있습니다. 이 두 속성이 어떻게 상호작용하는지 이해하고 계신가요?또한 버튼의 터치 영역과 접근성을 고려할 때, 현재 높이
52dp
가 충분한지 Android 가이드라인을 참고해 생각해보세요.생각해볼 점들:
- Material Design 버튼 컴포넌트를 사용하는 것은 어떨까요?
- 버튼 상태(pressed, disabled 등)에 따른 시각적 피드백은 어떻게 제공할 수 있을까요?
app/src/main/java/woowacourse/shopping/presentation/SingleLiveData.kt (1)
27-29
: call() 메서드의 용도를 명확히 해보세요.
call()
메서드가value = null
을 설정하는데, 이것이 어떤 상황에서 유용할지 생각해보세요.null 값으로 이벤트를 트리거하는 것과 실제 데이터와 함께 이벤트를 트리거하는 것의 차이점은 무엇일까요?
고민해볼 점들:
- 메서드 이름이 의도를 명확히 표현하고 있나요?
- null 안전성 측면에서 Observer에서는 어떻게 처리해야 할까요?
app/src/main/java/woowacourse/shopping/data/database/CartDatabase.kt (1)
12-12
: 데이터베이스 설계와 명명에 대해 고민해보세요현재 데이터베이스 이름이 "cart"인데 장바구니 데이터뿐만 아니라 최근 본 상품(
RecentlyViewedProduct
)도 포함하고 있습니다.데이터베이스나 클래스의 이름이 그 역할을 명확하게 표현하는 것이 왜 중요한지 생각해보세요. 다른 개발자가 이 코드를 봤을 때 어떤 혼란이 있을 수 있을까요?
app/src/main/java/woowacourse/shopping/data/dto/cart/CartResponse.kt (1)
22-29
: 도메인 매핑 함수의 설계를 검토해보세요확장 함수를 사용해 DTO를 도메인 모델로 변환하는 방식이 깔끔합니다.
하지만 매핑 로직에서 생각해볼 점이 있습니다:
Price(this.price)
- 가격이 음수이거나 0인 경우는 어떻게 처리해야 할까요?- 이미지 URL이나 다른 필드들의 유효성 검증은 어느 계층에서 해야 할까요?
도메인 계층의 역할과 데이터 유효성 검증 책임에 대해 어떻게 생각하시나요?
app/src/main/java/woowacourse/shopping/data/dto/product/ProductsResponse.kt (1)
6-30
: 페이지네이션 응답 구조에 대해 생각해보세요Spring의 Page 인터페이스와 유사한 구조로 보입니다. 많은 필드들이 있는데, 실제로 앱에서 모든 필드를 사용하고 있나요?
페이지네이션을 구현할 때 고려사항들:
- 어떤 필드들이 실제로 UI에서 필요한가요? (
totalPages
,last
,first
등)- 불필요한 필드들이 네트워크 대역폭과 파싱 시간에 미치는 영향은?
- 백엔드 API 응답을 그대로 모델링하는 것과 앱에 필요한 데이터만 선별하는 것 중 어떤 접근이 더 좋을까요?
사용하지 않는 필드들을 확인하고 정말 필요한 데이터만 남기는 것을 고려해보세요.
app/src/main/java/woowacourse/shopping/data/dto/product/ProductContent.kt (2)
8-20
: 다른 DTO와의 일관성을 확인해보세요
ProductContent
와CartResponse
의 구조가 매우 유사합니다.코드 중복을 발견했을 때 어떻게 접근해야 할까요?
- 공통 인터페이스나 기본 클래스를 만들 수 있을까요?
- 아니면 각각 다른 목적이므로 분리하는 것이 더 좋을까요?
- 향후 각 DTO가 독립적으로 변경될 가능성은 어떨까요?
DRY(Don't Repeat Yourself) 원칙과 응집도 사이의 균형점을 찾아보세요.
22-29
: 명명된 매개변수 사용이 좋습니다
CartResponse
의toDomain()
과 달리 여기서는 명명된 매개변수를 사용했습니다.두 가지 접근 방식의 차이점을 생각해보세요:
- 명명된 매개변수의 장점은 무엇인가요?
- 일관성 측면에서 어떤 방식을 팀 전체에서 사용하는 것이 좋을까요?
_price
매개변수 이름에서 밑줄의 의미는 무엇일까요?코드 일관성을 위해 프로젝트 전체에서 하나의 스타일로 통일하는 것을 고려해보세요.
app/src/androidTest/java/woowacourse/shopping/Fixture.kt (2)
8-67
: 테스트 픽스처 설계에 대해 생각해볼 점들이 있어 보입니다.현재 픽스처가 잘 구성되어 있지만, 몇 가지 질문을 던져보고 싶습니다:
imageUrl
과category
필드에 "aa", "bb", "cc" 같은 placeholder 값을 사용하는 것이 테스트에 미치는 영향은 무엇일까요?테스트 간 독립성을 위해 각 테스트마다 다른 픽스처를 사용하는 것과 공통 픽스처를 사용하는 것의 장단점은 무엇인지 생각해보셨나요?
viewedAt
같은 시간 관련 필드에 고정값(1234, 5555, 9999)을 사용할 때 발생할 수 있는 문제점은 없을까요?이런 부분들을 개선하기 위해 어떤 방법들을 고려해볼 수 있을지 한번 생각해보시기 바랍니다.
8-67
: 테스트 데이터의 품질과 유지보수성에 대해 고민해보셨나요?현재 Fixture 객체가 잘 구성되어 있지만, 테스트 데이터를 더욱 효과적으로 관리할 수 있는 방법들을 생각해보시면 좋겠습니다.
몇 가지 고려해볼 점들:
- 하드코딩된 값들(
"aa"
,"bb"
,1234
등)이 테스트의 의도를 명확히 전달하고 있는지- 중복되는 데이터 구조를 어떻게 더 효율적으로 관리할 수 있을지
- 테스트별로 다른 시나리오가 필요할 때는 어떻게 대응할 수 있을지
테스트 데이터 설계 패턴에 대해 어떤 접근 방식을 고려해보셨는지 궁금합니다.
app/src/main/java/woowacourse/shopping/domain/model/Cart.kt (3)
21-25
: delete 메서드의 예외 처리 방식에 대해 생각해보세요현재
delete
메서드는 존재하지 않는 상품을 삭제하려 할 때NoSuchElementException
을 던집니다. 이런 상황에서 예외를 던지는 것이 적절한지, 아니면 다른 방식(예: 결과를 나타내는 sealed class나 nullable 반환)이 더 나은지 고민해보세요.어떤 상황에서 예외를 사용하는 것이 좋고, 언제 다른 방식을 사용하는 것이 더 적절할까요?
6-7
: 매번 Map을 다시 생성하는 것에 대해 생각해보세요
cartMapByProductId
가 매번associateBy
로 재생성됩니다. Cart에 많은 아이템이 있을 때 성능에 미치는 영향을 고려해보세요.이런 상황에서 성능과 불변성 사이의 균형을 어떻게 맞출 수 있을까요? 다른 자료구조나 패턴을 활용할 수 있는 방법이 있을까요?
9-9
: private 생성자 패턴의 목적을 명확히 해보세요private 생성자를 사용한 이유가 무엇인지 생각해보세요. 이 패턴이 클래스의 불변성과 어떤 관련이 있는지, 그리고 이런 설계가 코드를 읽는 다른 개발자에게 어떤 의미를 전달하는지 고민해보세요.
코드 주석이나 문서화를 통해 이런 설계 의도를 더 명확히 표현할 수 있는 방법이 있을까요?
app/src/main/java/woowacourse/shopping/data/remote/OkHttpClientProvider.kt (1)
33-35
: 하드코딩된 태그명과 로깅 구조를 개선해보세요"PrettyLogger"라는 태그가 하드코딩되어 있습니다. 이런 방식이 유지보수에 어떤 영향을 미칠 수 있는지 생각해보세요.
상수로 분리하거나 다른 방식으로 관리할 수 있는 방법이 있을까요?
app/src/main/java/woowacourse/shopping/di/DataSourceModule.kt (1)
21-37
: 반복되는 패턴을 개선해보세요세 개의 provide 함수가 비슷한 패턴을 가지고 있습니다. 이런 중복을 줄이면서도 타입 안전성을 보장할 수 있는 방법이 있을까요?
또한, Dagger Hilt나 Koin 같은 DI 프레임워크를 사용했을 때의 장단점과 현재 방식의 장단점을 비교해보세요.
app/src/main/java/woowacourse/shopping/data/dto/cart/CartsResponse.kt (2)
25-25
: 클래스 네이밍에 대해 생각해보세요
SortX
라는 네이밍이 조금 특이합니다. 이 이름이 의미하는 바가 명확한지, 그리고 다른 개발자들이 이해하기 쉬운 이름인지 생각해보세요.API 응답에서 "sort" 필드가 어떤 구조를 가지고 있는지 확인하고, 더 명확한 이름을 지을 수 있을까요?
7-30
: 페이지네이션 로직 활용 계획을 세워보세요이 DTO는 페이지네이션을 위한 많은 정보를 포함하고 있습니다. 실제로 UI에서 이 모든 정보를 어떻게 활용할 계획인지 생각해보세요.
어떤 필드들이 실제로 필요하고, 어떤 필드들은 미래를 위한 것인지 구분해보세요. 또한 이 DTO를 도메인 모델로 변환할 때 어떤 로직이 필요할지 고민해보세요.
app/src/androidTest/java/woowacourse/shopping/view/ProductActivityTest.kt (1)
42-61
: 상품 아이템 UI 요소 테스트가 잘 구성되어 있습니다.각 UI 요소(이미지, 이름, 가격)에 대해 개별 테스트를 작성한 것이 좋습니다.
추가로 고려해볼 점들이 있습니다:
- 현재는 첫 번째 아이템(position 0)만 테스트하고 있는데, 다양한 상품 데이터에 대한 테스트도 필요할까요?
- 상품 목록이 비어있는 경우는 어떻게 처리되는지 확인해보셨나요?
- 네트워크 오류나 로딩 상태에 대한 UI 테스트도 필요한지 생각해보세요.
이런 케이스들을 어떻게 테스트할 수 있을지 먼저 생각해보시고, 필요하다면 추가 테스트 작성을 고려해보세요.
app/src/main/java/woowacourse/shopping/presentation/ProductUiModel.kt (1)
14-30
: 도메인-프레젠테이션 변환 함수가 잘 구현되어 있습니다.양방향 변환 함수를 extension function으로 구현한 것이 코드 가독성을 높여줍니다.
변환 과정에서 데이터 무결성을 어떻게 보장할 수 있을지 생각해보세요:
Price
객체 생성 시 음수나 유효하지 않은 값이 들어올 수 있는 상황은 없을까요?- 변환 과정에서 데이터 손실이 발생할 가능성은 없는지 확인해보셨나요?
이런 엣지 케이스들을 어떻게 처리하면 좋을지 먼저 생각해보시고, 필요하다면 유효성 검사나 예외 처리를 고려해보세요.
app/src/androidTest/java/woowacourse/shopping/ViewInteractionExtensions.kt (1)
13-24
: 테스트 유틸리티 함수들이 매우 잘 설계되어 있습니다!Espresso 코드의 가독성을 크게 향상시키는 확장 함수들입니다. 각 함수가 하나의 명확한 역할을 하고 있어 좋습니다.
추가로 확장을 고려해볼 만한 영역들이 있습니다:
- 텍스트 입력이나 스와이프 같은 다른 일반적인 UI 작업도 필요할까요?
- 에러 상황(예: 뷰를 찾을 수 없는 경우)에 대한 더 명확한 메시지를 제공하는 함수는 어떨까요?
- 복잡한 RecyclerView 조작(특정 조건의 아이템 찾기 등)을 위한 함수도 유용할 수 있을까요?
실제 테스트를 작성하면서 반복적으로 사용하는 패턴들을 찾아서 점진적으로 확장해나가는 것을 추천합니다.
app/src/main/AndroidManifest.xml (1)
20-36
: 액티비티 구조를 분석해보세요새로운 액티비티들이 추가되었습니다. 각 액티비티의 역할과 네비게이션 구조에 대해 생각해보세요.
다음과 같은 관점에서 현재 구조를 검토해보시기 바랍니다:
- 각 액티비티가 단일 책임 원칙을 따르고 있나요?
- 액티비티 간 데이터 전달 방식이 일관성 있나요?
- Fragment 기반 구조 vs Activity 기반 구조의 장단점을 고려해보셨나요?
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewHolder.kt (1)
22-24
: 데이터 변환에 대해 생각해보세요
CartItemUiModel
을toDomain()
으로 변환하여 바인딩하고 있습니다. 이런 변환이 ViewHolder에서 일어나는 것이 적절한지 생각해보세요.다음 관점에서 현재 구현을 검토해보시기 바랍니다:
- ViewHolder의 책임 범위는 어디까지여야 할까요?
- 데이터 변환 로직을 다른 계층으로 이동하는 것이 더 좋을까요?
- Clean Architecture 관점에서 UI 모델과 도메인 모델의 변환은 어느 계층에서 담당해야 할까요?
app/src/main/res/layout/activity_product.xml (3)
6-8
: 데이터 바인딩 섹션이 비어있는 이유를 생각해보세요.현재
<data>
섹션이 비어있습니다. 이 레이아웃에서 데이터 바인딩을 사용할 계획이신가요? 만약 사용하지 않는다면<layout>
태그를 제거하는 것을 고려해보세요.질문: 데이터 바인딩과 일반 뷰 바인딩의 차이점과 각각 언제 사용하는 것이 좋을까요?
58-58
: RecyclerView 높이를 고정값으로 설정한 이유를 다시 생각해보세요.최근 상품 목록의 높이가
160dp
로 고정되어 있습니다.힌트: 다양한 화면 크기에서 어떻게 보일지 고려해보세요.
wrap_content
나 비율 기반 높이를 사용하는 방법도 있습니다.질문: 가로 스크롤 RecyclerView에서 높이를 결정하는 다른 방법들은 무엇이 있을까요?
96-114
: ShimmerFrameLayout 내부 구조를 개선할 수 있는 방법을 생각해보세요.현재
GridLayout
을 사용하여 shimmer 아이템들을 배치하고 있습니다.힌트: RecyclerView의 GridLayoutManager와 일치하는 구조로 만드는 것이 좋습니다. 실제 상품 목록과 동일한 레이아웃 매니저를 사용하면 어떨까요?
질문: 로딩 UI와 실제 UI가 다르게 보일 때 사용자 경험에 어떤 영향을 줄까요?
app/src/main/java/woowacourse/shopping/presentation/product/ProductAdapter.kt (2)
64-78
: ViewHolder 클래스들을 별도 파일로 분리하는 것을 고려해보세요.현재 ViewHolder들이 어댑터 내부에 정의되어 있습니다.
질문: 클래스가 커질 때 어떤 문제점들이 생길까요? 단일 책임 원칙(SRP) 관점에서 생각해보세요.
힌트: ViewHolder를 별도 파일로 분리하면 테스트하기 쉬워지고 재사용성도 높아집니다.
ProductViewHolder.kt
같은 파일로 분리해보는 것은 어떨까요?
89-91
: 상수 관리 방식을 개선해보세요.에러 메시지가 companion object에 하드코딩되어 있습니다.
힌트: 문자열 리소스(
strings.xml
)를 사용하면 어떤 장점이 있을까요? 특히 다국어 지원과 일관된 에러 메시지 관리 측면에서요.질문: 개발 중 디버깅용 메시지와 사용자에게 보여줄 메시지를 어떻게 구분해서 관리하면 좋을까요?
app/src/main/java/woowacourse/shopping/di/RepositoryModule.kt (1)
11-43
: 테스트 시 모킹이 어려운 구조입니다.현재 구조에서 단위 테스트를 작성할 때 어떤 어려움이 있을지 생각해보세요.
질문: 이 모듈을 사용하는 클래스를 테스트할 때 가짜(mock) 구현체를 어떻게 주입할 수 있을까요?
힌트: 인터페이스 기반 DI나 Hilt/Dagger 같은 DI 프레임워크를 사용하면 어떤 장점이 있을지 알아보세요.
참고: 의존성 주입의 테스트 용이성 장점에 대해 학습해보시길 권합니다.
app/src/main/res/layout/activity_recommend.xml (1)
29-34
: 가이드라인 비율값들의 의미를 명확하게 해보세요.가이드라인이 25%와 66% 위치에 설정되어 있습니다.
질문: 이 비율들이 어떤 디자인 의도를 가지고 있을까요? 다양한 화면 크기에서 적절할까요?
힌트: 디자인 시스템의 그리드나 황금 비율 같은 디자인 원칙을 고려해보세요. 또는 이 비율들을 상수로 정의하여 재사용하는 것은 어떨까요?
Also applies to: 94-99
app/build.gradle.kts (2)
49-59
: 자격증명 처리 방식에 대해 한 번 더 생각해보세요현재 local.properties에서 ID와 비밀번호를 로드하여 BuildConfig로 노출하고 있습니다.
몇 가지 고려해볼 점들이 있습니다:
- local.properties 파일이 .gitignore에 포함되어 있나요?
- 실제 프로덕션 환경에서는 어떤 방식으로 민감한 정보를 관리하는 것이 좋을까요?
- BuildConfig에 평문으로 저장하는 것이 안전한 방법일까요?
Android에서 민감한 정보를 안전하게 관리하는 다른 방법들을 조사해보시는 것을 추천합니다.
67-93
: 의존성 관리에 대해 생각해보세요필요한 라이브러리들이 적절히 추가되었습니다.
몇 가지 고민해볼 점들:
- 각 라이브러리가 정말 필요한지 검토해보셨나요?
- 라이브러리 버전들이 서로 호환되는지 확인하셨나요?
- 앞으로 라이브러리 버전을 어떻게 관리할 계획인가요?
특히 Room, Retrofit, Lifecycle 등의 핵심 아키텍처 라이브러리들의 최신 권장사항을 확인해보시는 것을 추천합니다.
app/src/main/java/woowacourse/shopping/presentation/product/RecentAdapter.kt (2)
14-17
: 리스트 업데이트 방식을 개선해보세요현재
notifyDataSetChanged()
를 사용하고 있는데, 이 방식에 대해 어떻게 생각하시나요?고민해볼 점들:
- 리스트가 크거나 자주 업데이트되는 경우 성능에 어떤 영향을 줄까요?
- RecyclerView에서 더 효율적인 업데이트 방법이 있을까요?
- Android에서 권장하는
DiffUtil
에 대해 알아보셨나요?리스트 업데이트 성능 최적화에 대한 Android 공식 문서를 참고해보시는 것을 추천합니다.
37-50
: ViewHolder 구현이 깔끔합니다데이터 바인딩을 활용한 ViewHolder가 잘 구현되었습니다.
작은 개선 포인트:
executePendingBindings()
의 역할과 언제 사용해야 하는지 알고 계신가요?- 성능상 이 메서드가 항상 필요한지 생각해보세요.
데이터 바인딩의 실행 타이밍에 대한 Android 문서를 참고해보시면 도움이 될 것입니다.
app/src/main/java/woowacourse/shopping/presentation/CartItemUiModel.kt (2)
13-18
: 도메인 변환 로직이 명확합니다UI 모델에서 도메인 모델로의 변환이 잘 구현되었습니다.
한 가지 생각해볼 점:
- 매핑 과정에서 데이터 유효성 검증이 필요한 경우는 없을까요?
- 예를 들어, quantity가 음수인 경우 어떻게 처리해야 할까요?
매핑 로직의 안전성에 대해 한번 더 고민해보세요.
20-26
: 프레젠테이션 변환 로직이 적절합니다도메인 모델에서 UI 모델로의 변환이 잘 구현되었습니다.
고민해볼 점:
isSelected
가 항상 기본값(false
)으로 초기화되는 것이 맞을까요?- 특정 상황에서는 다른 초기값이 필요할 수도 있지 않을까요?
현재 비즈니스 요구사항에 맞는지 확인해보세요.
app/src/main/res/layout/item_cart_product.xml (2)
29-39
: 체크박스 구현이 적절합니다데이터 바인딩을 통한 상태 관리와 이벤트 처리가 잘 구현되어 있습니다.
접근성을 한번 더 고려해보세요:
- 체크박스에 적절한 contentDescription이 있나요?
- 시각적 장애가 있는 사용자가 이 체크박스의 용도를 알 수 있을까요?
Android 접근성 가이드라인을 참고해보시는 것을 추천합니다.
79-101
: 수량 조절과 가격 표시 구조가 좋습니다모듈화된 레이아웃과 적절한 제약조건으로 잘 구성되어 있습니다.
고민해볼 점들:
- 가격 형식화를 string 리소스에서 처리하는 것이 최선일까요?
- 다국가 통화나 천 단위 구분자 등을 고려했을 때 더 유연한 방법이 있을까요?
- NumberFormat이나 다른 형식화 방법을 사용하는 것은 어떨까요?
국제화와 지역화에 대해 한번 생각해보세요.
app/src/main/res/layout/activity_cart.xml (2)
45-53
: 레이아웃 ID명과 실제 역할 불일치 검토 필요
layout_cart_total_price
라는 ID를 가진 요소가 TextView로 정의되어 있지만, 실제로는 배경 컨테이너 역할을 하고 있습니다. 이는 코드 가독성을 떨어뜨릴 수 있습니다.다음과 같이 수정을 고려해보세요:
- <TextView - android:id="@+id/layout_cart_total_price" + <View + android:id="@+id/container_cart_total_price"
129-137
: 하드코딩된 shimmer 아이템 개수 개선 고려현재 5개의 shimmer 아이템이 하드코딩되어 있습니다. 화면 크기나 아이템 높이에 따라 동적으로 조정하는 것을 고려해보세요.
더 나은 사용자 경험을 위해 화면 높이에 따라 동적으로 shimmer 아이템 개수를 계산하거나, 최소한 상수로 분리하는 것을 고려해보세요.
app/src/main/java/woowacourse/shopping/data/remote/CartItemService.kt (2)
18-22
: 페이지네이션 매개변수 설계 검토
size
매개변수가 nullable로 정의되어 있는데, 이는 서버에서 기본값을 처리한다는 의미로 보입니다. 하지만 클라이언트에서 명시적으로 기본값을 제공하는 것이 더 명확할 수 있습니다.다음과 같이 기본값을 명시적으로 설정하는 것을 고려해보세요:
- @Query("size") size: Int?, + @Query("size") size: Int = 20,또는 서버 측 기본값 처리를 유지하고 싶다면 주석으로 의도를 명확히 해주세요.
40-41
: API 엔드포인트 URL 일관성 검토cart-items의 개수를 조회하는 엔드포인트가
/cart-items/counts
로 되어 있습니다. RESTful 규칙에 따르면/cart-items/count
(단수형)가 더 적절할 수 있습니다.서버 API와 협의하여 다음과 같은 URL을 고려해보세요:
- @GET("/cart-items/counts") + @GET("/cart-items/count")app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendActivity.kt (1)
21-24
: ViewModel 캐스팅 방식 개선 고려ViewModel을 여러 인터페이스로 캐스팅하는 방식이 반복되고 있습니다. 이는 타입 안전성을 해칠 수 있습니다.
RecommendViewModel에서 인터페이스들을 직접 구현하는 대신, 컴포지션 패턴을 사용하거나 어댑터에 개별 람다를 전달하는 방식을 고려해보세요:
private val recommendAdapter: RecommendAdapter by lazy { RecommendAdapter( onItemClick = viewModel::onItemClick, onCartCounterClick = viewModel::onCartCounterClick ) }app/src/main/res/layout/activity_detail_product.xml (1)
82-92
: 포함된 레이아웃의 데이터 바인딩을 살펴보세요.
layout_cart_quantity_box
에 전달하는 속성들(clickListener
,id
,quantity
)을 확인해보세요. 이 컴포넌트가 재사용 가능하도록 설계되었는지 생각해보세요. 어떤 다른 화면에서도 이 컴포넌트를 사용할 수 있을까요?app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailActivity.kt (3)
24-28
: 액티비티의 아키텍처 패턴을 분석해보세요.ItemClickListener 인터페이스를 구현하고 있습니다. 이 패턴을 선택한 이유가 무엇일까요? 다른 방법으로도 클릭 이벤트를 처리할 수 있는데, 이 방식의 장점은 무엇인지 생각해보세요.
83-95
: ViewModel 관찰 패턴을 분석해보세요.두 가지 다른 LiveData를 관찰하고 있습니다. 이들의 역할이 중복되지는 않나요? 언제 어떤 토스트가 표시되는지 시나리오를 그려보세요.
121-122
: 빈 메서드의 목적을 생각해보세요.인터페이스를 구현하면서 빈 메서드를 남겨두었습니다. 이 메서드가 어떤 역할을 해야 하는지, 왜 현재는 비어있는지 생각해보세요. 코드의 의도를 명확히 하려면 어떻게 해야 할까요?
정적 분석 도구에서도 이 부분을 지적했습니다. 어떤 해결책이 있을지 고민해보세요.
🧰 Tools
🪛 detekt (1.23.8)
[warning] 121-122: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
app/src/main/java/woowacourse/shopping/domain/repository/CartRepository.kt (2)
6-14
: 비동기 처리 패턴을 분석해보세요.콜백 기반의 비동기 처리를 사용하고 있습니다. 이 방식의 장단점을 생각해보세요. 최근 안드로이드 개발에서는 어떤 다른 비동기 패턴들이 권장되고 있을까요?
23-34
: 유사한 메서드들의 역할을 구분해보세요.
insertProduct
와updateProduct
메서드가 별도로 있고,insertOrUpdate
도 있습니다. 각각 언제 사용되는지, 이런 분리가 필요한 이유를 생각해보세요.app/src/main/java/woowacourse/shopping/presentation/cart/CartActivity.kt (1)
148-156
: 화면 간 데이터 전달 방식을 분석해보세요.RecommendActivity로 데이터를 전달할 때 ViewModel에서 값을 가져오고 있습니다. null 체크를 하고 있지만, 이런 상황이 발생할 수 있는 시나리오를 생각해보세요.
app/src/main/java/woowacourse/shopping/data/repository/remote/ProductRepositoryImpl.kt (2)
19-29
: 상품을 장바구니 아이템으로 매핑하는 로직을 살펴보세요.현재 구현에서 각 상품을 장바구니 아이템으로 변환할 때
cartRepository.getCartItemById()
를 호출하고 있습니다.이 접근 방식에 대해 생각해보세요:
- 상품 목록을 조회할 때마다 각 상품에 대해 개별적으로 장바구니 조회를 하는 것이 효율적일까요?
- 대량의 상품이 있을 때 성능에 미치는 영향은 어떨까요?
더 효율적인 방법을 고민해보세요.
41-45
: 사용하지 않는 코드를 정리해주세요.
toCartItems()
확장 함수가 정의되어 있지만 사용되지 않고 있으며, 주석 처리된 코드도 포함되어 있습니다.사용하지 않는 코드를 유지하는 이유가 있나요? 만약 향후 사용할 계획이라면 TODO 주석으로 명시하거나, 그렇지 않다면 제거하는 것을 고려해보세요.
app/src/main/java/woowacourse/shopping/presentation/product/ProductActivity.kt (3)
25-42
: Activity의 책임이 많아 보입니다.현재
ProductActivity
가 여러 인터페이스를 구현하고 많은 책임을 가지고 있습니다:
- Cart counter 관리
- Item click 처리
- 어댑터 초기화
- 네비게이션 처리
이런 책임들을 어떻게 더 잘 분리할 수 있을지 생각해보세요. 예를 들어:
- 일부 로직을 별도 클래스로 분리할 수 있을까요?
- 인터페이스들을 하나로 통합할 수 있을까요?
127-168
: ViewModel 관찰 로직이 복잡합니다.
observeViewModel()
메서드에서 많은 LiveData를 관찰하고 있습니다.이 구조에 대해 생각해보세요:
- 각 관찰자가 명확한 단일 책임을 가지고 있나요?
- UI 상태 업데이트 로직을 더 간단하게 만들 수 있는 방법이 있을까요?
- 일부 관찰자들을 하나로 통합할 수 있을까요?
코드의 가독성을 높이는 방법을 고민해보세요.
144-147
: 동일한 어댑터 업데이트 로직이 중복됩니다.
products
와showLoadMore
옵저버에서 동일한productAdapter.setData()
로직이 중복되고 있습니다.이 중복을 어떻게 제거할 수 있을지 생각해보세요:
- 두 데이터를 결합하는 방법이 있을까요?
- ViewModel에서 이미 결합된 상태를 제공할 수 있을까요?
Also applies to: 160-163
app/src/main/java/woowacourse/shopping/data/repository/local/RecentProductRepositoryImpl.kt (1)
72-73
: 타임스탬프 설정 방식을 고려해보세요.현재
System.currentTimeMillis()
를 사용하여 조회 시간을 설정하고 있습니다.이 방식에 대해 생각해보세요:
- 시스템 시간이 변경되거나 시간대가 다른 경우에 문제가 없을까요?
- 더 안정적인 시간 측정 방법이 있을까요?
- 테스트 시에는 어떻게 시간을 제어할 수 있을까요?
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailViewModel.kt (1)
34-45
: 비동기 작업 처리 방식을 개선할 수 있을까요?현재 콜백 방식으로 비동기 작업을 처리하고 있는데, 코틀린에서 제공하는 다른 비동기 처리 방법들은 어떤 것들이 있는지 알아보셨나요? 특히
Coroutines
을 사용하면 어떤 이점이 있을지 생각해보시면 어떨까요?app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewModel.kt (1)
67-68
: 빈 메서드 구현이 의도된 것인가요?정적 분석 도구에서도 지적했듯이 빈 메서드가 있습니다. 이것이 의도된 구현인지, 아니면 아직 구현되지 않은 기능인지 명확하게 할 필요가 있습니다. 인터페이스 분리 원칙 관점에서 불필요한 메서드를 구현하지 않아도 되는 방법은 없을까요?
🧰 Tools
🪛 detekt (1.23.8)
[warning] 67-68: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
app/src/main/java/woowacourse/shopping/data/repository/remote/CartRepositoryImpl.kt (1)
119-119
: 프로덕션 코드에 디버그 로그가 있어요프로덕션 환경에서도 이 로그가 출력되어야 하나요? 보안상 민감한 정보가 로그에 남을 수 있는지 고려해보셨나요? 디버그 로그를 관리하는 더 좋은 방법은 없을까요?
app/src/main/java/woowacourse/shopping/presentation/cart/CartViewModel.kt (1)
115-120
: 체크박스 상태 업데이트 로직의 효율성을 개선할 수 있을까요?
allCheckOrUnchecked
메서드에서 리스트를 두 번 순회하고 있습니다:
- 한 번은 모든 항목이 선택되었는지 확인
- 또 한 번은 모든 항목이 선택 해제되었는지 확인
이 로직을 어떻게 최적화할 수 있을까요? 한 번의 순회로 같은 결과를 얻을 수 있는 방법을 생각해보세요.
app/src/main/java/woowacourse/shopping/data/datasource/remote/CartDataSourceImpl.kt (1)
144-145
: 확장 함수의 안전성을 높일 수 있는 방법이 있을까요?
toIdOrNull
확장 함수에서:headers()["LOCATION"]?.substringAfterLast("/")?.toLongOrNull()이 구현에 대해 생각해보세요:
- "LOCATION" 헤더가 예상한 형식이 아니라면?
- 헤더 이름의 대소문자는 문제없을까요?
- URL 형식이 바뀌면 어떻게 될까요?
더 견고한 파싱 방법을 고민해보세요.
app/src/main/java/woowacourse/shopping/presentation/bindingadapter/TextViewBindingAdapter.kt (1)
7-16
: 가격 계산 로직을 더 견고하게 만들어보세요.기본 구현은 잘 되어 있습니다! 하지만 몇 가지 상황을 더 고려해보면 어떨까요?
price * count
에서 오버플로우가 발생할 수 있는 상황은 없을까요?- 음수 가격이나 수량이 들어올 가능성은 없나요?
- 0원이나 0개일 때는 어떻게 표시되어야 할까요?
다음을 고려해보세요:
- 입력값 검증 로직 추가
- 큰 숫자 처리를 위한 Long 타입 사용 검토
- 예외 상황에 대한 적절한 메시지 표시
현재 요구사항에서는 이 정도로 충분할 수 있지만, 실제 서비스에서는 어떤 추가 검증이 필요할지 생각해보세요.
app/src/main/java/woowacourse/shopping/domain/repository/ProductRepository.kt (1)
6-18
: 비동기 처리 방식에 대한 설계 결정을 어떻게 내리셨나요?현재 콜백 기반의 비동기 처리를 사용하고 계시는데, 이런 설계 선택에는 분명한 이유가 있을 것 같습니다.
고려해볼 점들:
- 콜백 방식과 다른 비동기 처리 방법들(예: Coroutines, RxJava)의 장단점은 무엇인가요?
- Repository 인터페이스에서
Result<T>
타입을 사용한 이유는 무엇인가요?- nullable 파라미터들(
page
,pageSize
,category
)이 API 설계에 어떤 유연성을 제공하나요?이런 설계 결정들이 전체 아키텍처에 어떤 영향을 미치는지 생각해보시면 도움이 될 것 같습니다.
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailViewModelFactory.kt (1)
7-18
: ViewModelFactory의 의존성 관리 방식에 대해 어떻게 생각하시나요?현재 구현이 작동하지만, Factory 패턴에서 의존성을 관리하는 방식에 대해 함께 생각해보면 좋겠습니다.
고려해볼 점들:
- RepositoryModule에서 직접 의존성을 가져오는 것과 생성자로 주입받는 것의 차이점은 무엇일까요?
as T
캐스팅이 필요한 이유와 이것이 타입 안전성에 미치는 영향은?- 여러 ViewModel이 생겨날 때 이런 Factory 구조의 확장성은 어떨까요?
- 테스트할 때 이런 구조가 어떤 장단점을 가질까요?
의존성 주입과 Factory 패턴에 대한 다른 접근 방식들도 찾아보시면 도움이 될 것 같습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Knowledge Base: Disabled due to Reviews > Disable Knowledge Base setting
📒 Files selected for processing (107)
README.md
(1 hunks)app/build.gradle.kts
(3 hunks)app/src/androidTest/java/woowacourse/shopping/Fixture.kt
(1 hunks)app/src/androidTest/java/woowacourse/shopping/ViewInteractionExtensions.kt
(1 hunks)app/src/androidTest/java/woowacourse/shopping/dao/CartDaoTest.kt
(1 hunks)app/src/androidTest/java/woowacourse/shopping/dao/RecentProductDaoTest.kt
(1 hunks)app/src/androidTest/java/woowacourse/shopping/view/CartActivityTest.kt
(1 hunks)app/src/androidTest/java/woowacourse/shopping/view/ProductActivityTest.kt
(1 hunks)app/src/androidTest/java/woowacourse/shopping/view/ProductDetailActivityTest.kt
(1 hunks)app/src/main/AndroidManifest.xml
(2 hunks)app/src/main/java/woowacourse/shopping/ShoppingApplication.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/CartItemMapper.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/DummyProducts.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/ThreadUtils.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dao/CartDao.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dao/RecentlyProductDao.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/database/CartDatabase.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/datasource/local/RecentProductDataSource.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/datasource/local/RecentProductDataSourceImpl.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/datasource/local/UserPreference.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/datasource/remote/CartDataSource.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/datasource/remote/CartDataSourceImpl.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/datasource/remote/ProductDataSource.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/datasource/remote/ProductDataSourceImpl.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/cart/CartContent.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/cart/CartItemCountResponse.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/cart/CartItemRequest.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/cart/CartResponse.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/cart/CartsResponse.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/cart/Pageable.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/cart/SortX.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/cart/UpdateCartRequest.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/product/Pageable.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/product/ProductContent.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/product/ProductsResponse.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/dto/product/SortX.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/entity/CartEntity.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/entity/RecentlyViewedProduct.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/remote/CartItemService.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/remote/OkHttpClientProvider.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/remote/ProductService.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/repository/local/RecentProductRepositoryImpl.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/repository/remote/CartRepositoryImpl.kt
(1 hunks)app/src/main/java/woowacourse/shopping/data/repository/remote/ProductRepositoryImpl.kt
(1 hunks)app/src/main/java/woowacourse/shopping/di/DataSourceModule.kt
(1 hunks)app/src/main/java/woowacourse/shopping/di/DatabaseModule.kt
(1 hunks)app/src/main/java/woowacourse/shopping/di/NetworkModule.kt
(1 hunks)app/src/main/java/woowacourse/shopping/di/RepositoryModule.kt
(1 hunks)app/src/main/java/woowacourse/shopping/domain/model/Cart.kt
(1 hunks)app/src/main/java/woowacourse/shopping/domain/model/CartItem.kt
(1 hunks)app/src/main/java/woowacourse/shopping/domain/model/Price.kt
(1 hunks)app/src/main/java/woowacourse/shopping/domain/model/Product.kt
(1 hunks)app/src/main/java/woowacourse/shopping/domain/repository/CartRepository.kt
(1 hunks)app/src/main/java/woowacourse/shopping/domain/repository/ProductRepository.kt
(1 hunks)app/src/main/java/woowacourse/shopping/domain/repository/RecentProductRepository.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/CartItemUiModel.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/Extra.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/IntentsExtensions.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/ProductUiModel.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/ResultState.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/SingleLiveData.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/bindingadapter/ButtonBindingAdapter.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/bindingadapter/ImageViewBindingAdapter.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/bindingadapter/LayoutBindingAdapter.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/bindingadapter/TextViewBindingAdapter.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/cart/CartActivity.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/cart/CartAdapter.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/cart/CartCounterClickListener.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/cart/CartPageClickListener.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/cart/CartViewModel.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/cart/CartViewModelFactory.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/product/ItemClickListener.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/product/ProductActivity.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/product/ProductAdapter.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/product/ProductViewModel.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/product/ProductViewModelFactory.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/product/RecentAdapter.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailActivity.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailViewModel.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailViewModelFactory.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/recommend/ItemDiffUtil.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendActivity.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendAdapter.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewHolder.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewModel.kt
(1 hunks)app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewModelFactory.kt
(1 hunks)app/src/main/res/color/checkbox_selector.xml
(1 hunks)app/src/main/res/drawable/bg_cart_badge.xml
(1 hunks)app/src/main/res/drawable/bg_cart_item.xml
(1 hunks)app/src/main/res/drawable/bg_cart_quantity_box.xml
(1 hunks)app/src/main/res/drawable/ic_add.xml
(1 hunks)app/src/main/res/drawable/ic_back.xml
(1 hunks)app/src/main/res/drawable/ic_cart.xml
(1 hunks)app/src/main/res/drawable/ic_delete.xml
(1 hunks)app/src/main/res/drawable/ic_exit.xml
(1 hunks)app/src/main/res/drawable/ic_loading.xml
(1 hunks)app/src/main/res/drawable/ic_next.xml
(1 hunks)app/src/main/res/drawable/ic_previous.xml
(1 hunks)app/src/main/res/drawable/round_btn_cart_next.xml
(1 hunks)app/src/main/res/drawable/round_btn_cart_previous.xml
(1 hunks)app/src/main/res/layout-land/activity_detail_product.xml
(1 hunks)app/src/main/res/layout/activity_cart.xml
(1 hunks)app/src/main/res/layout/activity_detail_product.xml
(1 hunks)app/src/main/res/layout/activity_main.xml
(0 hunks)app/src/main/res/layout/activity_product.xml
(1 hunks)app/src/main/res/layout/activity_recommend.xml
(1 hunks)app/src/main/res/layout/item_cart_product.xml
(1 hunks)
⛔ Files not processed due to max files limit (30)
- app/src/main/res/layout/item_load_more.xml
- app/src/main/res/layout/item_product.xml
- app/src/main/res/layout/item_recent_product.xml
- app/src/main/res/layout/item_recommend_product.xml
- app/src/main/res/layout/layout_cart_product_shimmer.xml
- app/src/main/res/layout/layout_cart_quantity_box.xml
- app/src/main/res/layout/layout_product_shimmer.xml
- app/src/main/res/layout/layout_recent_product.xml
- app/src/main/res/layout/view_cart_action.xml
- app/src/main/res/menu/menu_product.xml
- app/src/main/res/menu/menu_product_detail.xml
- app/src/main/res/values-night/themes.xml
- app/src/main/res/values/colors.xml
- app/src/main/res/values/dimens.xml
- app/src/main/res/values/strings.xml
- app/src/main/res/values/themes.xml
- app/src/test/java/woowacourse/shopping/JsonObjectTest.kt
- app/src/test/java/woowacourse/shopping/domain/PriceTest.kt
- app/src/test/java/woowacourse/shopping/domain/ProductTest.kt
- app/src/test/java/woowacourse/shopping/fixture/FakeCartRepository.kt
- app/src/test/java/woowacourse/shopping/fixture/FakeProductRepository.kt
- app/src/test/java/woowacourse/shopping/fixture/FakeRecentProductRepository.kt
- app/src/test/java/woowacourse/shopping/fixture/ProductsFixture.kt
- app/src/test/java/woowacourse/shopping/presentation/CartViewModelTest.kt
- app/src/test/java/woowacourse/shopping/presentation/InstantTaskExecutorExtension.kt
- app/src/test/java/woowacourse/shopping/presentation/ProductDetailViewModelTest.kt
- app/src/test/java/woowacourse/shopping/presentation/ProductViewModelTest.kt
- app/src/test/java/woowacourse/shopping/presentation/getOrAwaitValue.kt
- build.gradle.kts
- gradle/libs.versions.toml
💤 Files with no reviewable changes (1)
- app/src/main/res/layout/activity_main.xml
🧰 Additional context used
📓 Path-based instructions (1)
`**/*.kt`: You MUST follow these guidelines: - Do not provide complete solutions unless explicitly requested. - Guide the programmer in problem-solving instead of providing direct ...
**/*.kt
: You MUST follow these guidelines:
- Do not provide complete solutions unless explicitly requested.
- Guide the programmer in problem-solving instead of providing direct answers.
- When asked about programming concepts (e.g., "What is a hook?"), give direct and clear explanations.
- Break problems into smaller, manageable steps to help think through them logically.
- Ask leading questions and provide hints instead of just telling the answer.
- Encourage independent debugging before offering suggestions.
- Refer to relevant documentation instead of providing ready-made solutions.
- Promote modular thinking—breaking problems into reusable components.
- Remind the programmer to reflect on lessons learned after solving an issue.
- If explicitly asked for code (e.g., "Give me the code"), then provide it.
app/src/main/java/woowacourse/shopping/domain/model/Price.kt
app/src/main/java/woowacourse/shopping/presentation/bindingadapter/LayoutBindingAdapter.kt
app/src/main/java/woowacourse/shopping/presentation/cart/CartCounterClickListener.kt
app/src/main/java/woowacourse/shopping/data/dto/cart/UpdateCartRequest.kt
app/src/main/java/woowacourse/shopping/presentation/bindingadapter/TextViewBindingAdapter.kt
app/src/main/java/woowacourse/shopping/data/dto/cart/SortX.kt
app/src/main/java/woowacourse/shopping/presentation/product/ItemClickListener.kt
app/src/main/java/woowacourse/shopping/data/dto/cart/CartItemCountResponse.kt
app/src/main/java/woowacourse/shopping/ShoppingApplication.kt
app/src/main/java/woowacourse/shopping/data/entity/CartEntity.kt
app/src/main/java/woowacourse/shopping/presentation/Extra.kt
app/src/main/java/woowacourse/shopping/data/dto/cart/CartItemRequest.kt
app/src/androidTest/java/woowacourse/shopping/dao/CartDaoTest.kt
app/src/main/java/woowacourse/shopping/data/dto/product/SortX.kt
app/src/main/java/woowacourse/shopping/data/CartItemMapper.kt
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewModelFactory.kt
app/src/main/java/woowacourse/shopping/presentation/product/ProductViewModelFactory.kt
app/src/main/java/woowacourse/shopping/data/DummyProducts.kt
app/src/main/java/woowacourse/shopping/data/dao/RecentlyProductDao.kt
app/src/main/java/woowacourse/shopping/domain/model/CartItem.kt
app/src/main/java/woowacourse/shopping/data/remote/ProductService.kt
app/src/main/java/woowacourse/shopping/di/DatabaseModule.kt
app/src/main/java/woowacourse/shopping/presentation/bindingadapter/ImageViewBindingAdapter.kt
app/src/main/java/woowacourse/shopping/presentation/bindingadapter/ButtonBindingAdapter.kt
app/src/main/java/woowacourse/shopping/presentation/cart/CartViewModelFactory.kt
app/src/main/java/woowacourse/shopping/data/entity/RecentlyViewedProduct.kt
app/src/main/java/woowacourse/shopping/data/database/CartDatabase.kt
app/src/androidTest/java/woowacourse/shopping/view/ProductDetailActivityTest.kt
app/src/main/java/woowacourse/shopping/di/NetworkModule.kt
app/src/main/java/woowacourse/shopping/data/dto/product/ProductContent.kt
app/src/androidTest/java/woowacourse/shopping/Fixture.kt
app/src/main/java/woowacourse/shopping/presentation/IntentsExtensions.kt
app/src/main/java/woowacourse/shopping/presentation/recommend/ItemDiffUtil.kt
app/src/main/java/woowacourse/shopping/data/datasource/remote/ProductDataSource.kt
app/src/androidTest/java/woowacourse/shopping/dao/RecentProductDaoTest.kt
app/src/main/java/woowacourse/shopping/presentation/cart/CartPageClickListener.kt
app/src/main/java/woowacourse/shopping/data/datasource/local/UserPreference.kt
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailViewModelFactory.kt
app/src/main/java/woowacourse/shopping/data/remote/OkHttpClientProvider.kt
app/src/main/java/woowacourse/shopping/presentation/SingleLiveData.kt
app/src/main/java/woowacourse/shopping/domain/model/Product.kt
app/src/main/java/woowacourse/shopping/data/datasource/local/RecentProductDataSource.kt
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendActivity.kt
app/src/main/java/woowacourse/shopping/domain/repository/RecentProductRepository.kt
app/src/main/java/woowacourse/shopping/data/dto/cart/Pageable.kt
app/src/main/java/woowacourse/shopping/presentation/ProductUiModel.kt
app/src/main/java/woowacourse/shopping/data/dto/product/Pageable.kt
app/src/main/java/woowacourse/shopping/presentation/ResultState.kt
app/src/main/java/woowacourse/shopping/data/dto/product/ProductsResponse.kt
app/src/main/java/woowacourse/shopping/presentation/product/ProductAdapter.kt
app/src/main/java/woowacourse/shopping/data/dto/cart/CartsResponse.kt
app/src/main/java/woowacourse/shopping/data/dto/cart/CartContent.kt
app/src/main/java/woowacourse/shopping/domain/repository/ProductRepository.kt
app/src/main/java/woowacourse/shopping/di/DataSourceModule.kt
app/src/main/java/woowacourse/shopping/data/dto/cart/CartResponse.kt
app/src/main/java/woowacourse/shopping/data/datasource/local/RecentProductDataSourceImpl.kt
app/src/main/java/woowacourse/shopping/data/ThreadUtils.kt
app/src/androidTest/java/woowacourse/shopping/view/CartActivityTest.kt
app/src/main/java/woowacourse/shopping/di/RepositoryModule.kt
app/src/main/java/woowacourse/shopping/data/datasource/remote/CartDataSource.kt
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewHolder.kt
app/src/main/java/woowacourse/shopping/presentation/cart/CartActivity.kt
app/src/androidTest/java/woowacourse/shopping/view/ProductActivityTest.kt
app/src/main/java/woowacourse/shopping/domain/repository/CartRepository.kt
app/src/main/java/woowacourse/shopping/domain/model/Cart.kt
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailActivity.kt
app/src/main/java/woowacourse/shopping/data/remote/CartItemService.kt
app/src/androidTest/java/woowacourse/shopping/ViewInteractionExtensions.kt
app/src/main/java/woowacourse/shopping/data/repository/local/RecentProductRepositoryImpl.kt
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendAdapter.kt
app/src/main/java/woowacourse/shopping/presentation/product/RecentAdapter.kt
app/src/main/java/woowacourse/shopping/data/dao/CartDao.kt
app/src/main/java/woowacourse/shopping/presentation/cart/CartAdapter.kt
app/src/main/java/woowacourse/shopping/data/datasource/remote/ProductDataSourceImpl.kt
app/src/main/java/woowacourse/shopping/data/repository/remote/ProductRepositoryImpl.kt
app/src/main/java/woowacourse/shopping/presentation/CartItemUiModel.kt
app/src/main/java/woowacourse/shopping/data/repository/remote/CartRepositoryImpl.kt
app/src/main/java/woowacourse/shopping/presentation/product/ProductViewModel.kt
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewModel.kt
app/src/main/java/woowacourse/shopping/presentation/cart/CartViewModel.kt
app/src/main/java/woowacourse/shopping/data/datasource/remote/CartDataSourceImpl.kt
app/src/main/java/woowacourse/shopping/presentation/product/ProductActivity.kt
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailViewModel.kt
🧬 Code Graph Analysis (5)
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendActivity.kt (1)
app/src/main/java/woowacourse/shopping/presentation/cart/CartActivity.kt (4)
initInsets
(57-63)setupToolbar
(72-81)initAdapter
(65-70)observeViewModel
(83-111)
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailActivity.kt (1)
app/src/main/java/woowacourse/shopping/presentation/product/ProductActivity.kt (4)
initInsets
(90-96)setupToolbar
(98-100)observeViewModel
(127-168)showToast
(186-190)
app/src/main/java/woowacourse/shopping/data/repository/local/RecentProductRepositoryImpl.kt (1)
app/src/main/java/woowacourse/shopping/data/ThreadUtils.kt (1)
runThread
(5-12)
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewModel.kt (1)
app/src/main/java/woowacourse/shopping/presentation/product/ProductViewModel.kt (2)
fetchData
(43-65)updateQuantity
(131-141)
app/src/main/java/woowacourse/shopping/presentation/product/ProductActivity.kt (2)
app/src/main/java/woowacourse/shopping/presentation/cart/CartActivity.kt (4)
showSkeleton
(113-120)initInsets
(57-63)observeViewModel
(83-111)showToast
(122-126)app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailActivity.kt (3)
initInsets
(64-70)observeViewModel
(83-95)showToast
(97-101)
🪛 markdownlint-cli2 (0.17.2)
README.md
3-3: Heading levels should only increment by one level at a time
Expected: h2; Actual: h3
(MD001, heading-increment)
14-14: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 2; Actual: 4
(MD007, ul-indent)
🪛 detekt (1.23.8)
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailActivity.kt
[warning] 121-122: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
app/src/main/java/woowacourse/shopping/data/datasource/remote/ProductDataSourceImpl.kt
[warning] 65-66: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewModel.kt
[warning] 67-68: This empty block of code can be removed.
(detekt.empty-blocks.EmptyFunctionBlock)
🔇 Additional comments (58)
app/src/main/res/drawable/bg_cart_badge.xml (1)
4-5
: @color/red 리소스 존재 여부 확인 요청
<solid android:color="@color/red" />
에서 참조하는@color/red
가colors.xml
에 정의되어 있는지 확인해주세요.app/src/main/res/drawable/bg_cart_item.xml (1)
1-12
: 다른 리소스와 색상 참조 일관성 확인
프로젝트 전체에서@android:color/white
대신 커스텀 컬러 리소스(@color/white
)를 사용 중인지 검토하세요. 일관된 방식으로 색상을 관리하면 테마 변경 시 유지보수가 쉬워집니다.app/src/main/java/woowacourse/shopping/data/dto/product/SortX.kt (1)
1-1
: 패키지 위치에 대해 다시 한번 생각해보세요.현재 파일이
product
패키지에 있는데, AI 요약에서는cart
패키지라고 언급되어 있습니다. 이 클래스가 정말 상품(product) 관련 정렬 정보를 나타내는 것인지, 아니면 다른 도메인에 속해야 하는지 검토해보시기 바랍니다.힌트: 이 클래스가 어디서 사용되는지 확인해보고, 사용 맥락에 맞는 패키지 구조를 고민해보세요.
Likely an incorrect or invalid review comment.
app/src/main/java/woowacourse/shopping/domain/model/CartItem.kt (1)
8-8
: 계산 속성의 안전성을 검토해보세요.
totalPrice
계산이 단순해 보이지만, 몇 가지 시나리오를 고려해보세요:생각해볼 점들:
quantity
가 음수라면?- 곱셈 결과가 Int 범위를 초과한다면?
product.price
가 음수라면?질문: 이런 예외 상황들을 어느 레이어에서 처리하는 것이 적절할까요? 도메인 모델의 책임 범위는 어디까지일까요?
app/src/main/java/woowacourse/shopping/data/entity/CartEntity.kt (1)
9-10
: 컬럼 매핑 전략이 좋습니다.
@ColumnInfo
를 명시적으로 사용해서 데이터베이스 컬럼명을 명확히 정의한 것이 좋은 접근입니다. 이렇게 하면 코드가 변경되어도 데이터베이스 스키마의 안정성을 유지할 수 있습니다.잘한 점: 스네이크 케이스(
product_id
,quantity
) 사용으로 데이터베이스 네이밍 컨벤션을 따르고 있습니다.app/src/main/java/woowacourse/shopping/presentation/bindingadapter/LayoutBindingAdapter.kt (2)
6-12
: 깔끔하고 재사용 가능한 바인딩 어댑터 구현입니다.이 구현은 단일 책임 원칙을 잘 따르고 있고, 코드도 이해하기 쉽습니다.
한 가지 생각해볼 점이 있다면: 현재는
View.VISIBLE
와View.GONE
만 사용하고 있는데, 어떤 상황에서는View.INVISIBLE
이 필요할 수도 있습니다. 이 둘의 차이점을 알고 계시나요?현재 요구사항에서는
GONE
이 적절한 선택인지 한 번 더 생각해보세요. 레이아웃에서 해당 뷰가 차지하는 공간을 완전히 제거하는 것이 맞는지, 아니면 공간은 유지하되 보이지만 않게 하는 것이 더 나은지 고려해보시면 좋겠습니다.
6-12
: 깔끔한 구현입니다!가시성 제어 로직이 명확하고 간단하게 구현되어 있습니다.
한 가지 생각해볼 점: 현재는
VISIBLE
과GONE
만 사용하고 있는데, 향후INVISIBLE
옵션도 필요할 수 있을까요? 지금은 이 구현으로 충분하지만, 나중에 확장이 필요하다면 어떤 방식으로 접근하시겠어요?README.md (1)
5-17
: 기능 목록이 매우 명확하고 잘 정리되어 있습니다!각 단계별로 완료된 기능들이 체크리스트 형태로 잘 정리되어 있고, 특히 추천 알고리즘의 세부 사항까지 상세히 문서화되어 있어 좋습니다. 이런 상세한 문서화는 팀 협업에 매우 도움이 됩니다.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
14-14: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
15-15: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
16-16: Unordered list indentation
Expected: 2; Actual: 4(MD007, ul-indent)
app/src/main/java/woowacourse/shopping/data/datasource/local/RecentProductDataSource.kt (2)
10-10
: 잠재적인 예외 상황을 고려해보세요.
getOldestProduct()
가RecentlyViewedProduct
를 non-nullable로 반환하는데, 데이터베이스가 비어있을 때는 어떻게 될까요?이런 상황에서 발생할 수 있는 문제들을 어떻게 해결할 수 있을지 생각해보세요.
getMostRecentProduct()
는 nullable로 설계된 이유와 비교해보면 어떨까요?
5-17
: 인터페이스 설계가 깔끔하고 목적에 맞게 잘 구성되어 있습니다!메서드명이 직관적이고, 최근 본 상품 관리에 필요한 기본적인 CRUD 연산들이 잘 정의되어 있습니다. 인터페이스를 통한 추상화로 테스트 가능성도 좋아 보입니다.
app/src/main/java/woowacourse/shopping/data/datasource/remote/ProductDataSource.kt (1)
5-17
: 에러 처리 전략을 생각해보세요데이터소스 레이어에서 콜백만 사용하고 있는데, 네트워크 에러나 파싱 에러가 발생했을 때는 어떻게 처리할 계획인가요?
비교해보세요:
RecentProductRepository
는Result<T>
를 사용해서 성공/실패를 구분하고 있습니다.- 하지만 여기서는 성공 케이스만 콜백으로 처리하고 있네요.
- 구현체에서 에러가 발생하면 어떻게 호출자에게 알릴 수 있을까요?
레이어별 책임과 에러 처리 방식의 일관성을 고려해서, 어떤 방식이 더 적절한지 판단해보세요.
app/src/main/java/woowacourse/shopping/data/database/CartDatabase.kt (2)
21-27
: 싱글톤 패턴의 스레드 안전성이 잘 구현되어 있습니다
@Volatile
과synchronized
를 사용한 double-checked locking 패턴을 올바르게 구현했습니다.이 패턴이 왜 필요한지, 그리고 각 키워드가 어떤 역할을 하는지 설명할 수 있나요? 특히
@Volatile
이 없다면 어떤 문제가 발생할 수 있는지 생각해보세요.
29-35
: 애플리케이션 컨텍스트 사용이 좋습니다
context.applicationContext
를 사용하여 메모리 누수를 방지한 것이 좋습니다.Activity Context와 Application Context의 차이점과 언제 어떤 것을 사용해야 하는지 이해하고 계신가요? 데이터베이스처럼 앱 전체에서 사용되는 싱글톤에서는 왜 Application Context를 사용해야 할까요?
app/src/main/java/woowacourse/shopping/data/dto/cart/CartResponse.kt (1)
8-20
: 직렬화 어노테이션 사용이 적절합니다
@SerialName
어노테이션을 모든 필드에 명시적으로 사용한 것이 좋습니다.API 응답의 필드명과 코틀린 필드명이 동일한 경우에도
@SerialName
을 명시하는 것이 왜 좋은 습관인지 생각해보세요. 향후 API가 변경되거나 코드 리팩토링 시 어떤 이점이 있을까요?app/src/main/java/woowacourse/shopping/domain/repository/ProductRepository.kt (1)
6-18
: 리포지토리 인터페이스 설계에 대해 몇 가지 고민해볼 점들이 있습니다.잘 정의된 인터페이스지만 설계 관점에서 생각해볼 부분들이 있어요:
fetchPagingProducts
는CartItem
리스트를,fetchProductById
는Product
를 반환하는데, 이런 차이가 있는 이유는 무엇인가요? 클라이언트 관점에서 일관성 있는 API가 더 좋을까요?콜백 기반 비동기 처리를 선택한 특별한 이유가 있나요? 코루틴과 비교했을 때의 장단점을 어떻게 평가하셨는지 궁금합니다.
page
와pageSize
가 nullable인데, null일 때의 동작은 어떻게 정의되어야 할까요? 이런 부분이 문서화되어 있나요?현재 안드로이드 개발에서 일반적으로 사용되는 비동기 패턴들과 비교해서 이 방식을 선택한 근거를 정리해보시면 좋을 것 같습니다.
app/src/main/java/woowacourse/shopping/presentation/IntentsExtensions.kt (2)
7-17
: 확장 함수 설계가 깔끔하지만 에러 처리 방식에 대해 생각해볼 점이 있습니다.API 호환성을 잘 처리하신 좋은 확장 함수네요! 몇 가지 궁금한 점들이 있어요:
error()
함수를 사용해서 예외를 던지는 방식을 선택하신 이유는 무엇인가요?IllegalArgumentException
이나 커스텀 예외를 사용하는 것과 비교해서 어떤 장점이 있을까요?에러 메시지가 하드코딩되어 있는데, 다국어 지원이나 일관된 에러 메시지 관리 측면에서는 어떻게 생각하시나요?
이 확장 함수를 사용하는 액티비티들에서 예외가 발생했을 때 어떻게 처리되는지 확인해보셨나요?
안드로이드에서 Intent 데이터 전달 시 발생할 수 있는 다양한 오류 상황들을 어떻게 처리하는 것이 좋은지 한번 정리해보시면 도움이 될 것 같습니다.
7-17
: API 호환성 처리와 타입 안전성을 어떻게 균형있게 구현하셨는지 설명해주실 수 있나요?이 확장 함수는 Android API 호환성 문제를 잘 해결하고 있습니다. 구현 방식에 대해 몇 가지 질문이 있습니다:
reified
키워드를 사용한 이유와 그 장점은 무엇인가요?- API 33 전후로 다른 처리 방식을 사용하는 이유를 설명해주실 수 있나요?
@Suppress("DEPRECATION")
을 사용한 부분에서 다른 대안은 없었을까요?- 에러 메시지를 상수로 분리한 설계 의도는 무엇인가요?
이런 구현 패턴이 프로젝트 전반에서 어떻게 활용되고 있는지도 궁금합니다.
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailViewModelFactory.kt (1)
7-18
: ViewModel 팩토리 구현이 표준적이지만 의존성 주입 방식에 대해 고민해볼 점들이 있습니다.팩토리 패턴을 올바르게 구현하셨지만 몇 가지 생각해볼 부분들이 있어요:
RepositoryModule
에 직접 의존하는 방식의 장단점은 무엇일까요? 특히 테스트 작성 시 어떤 어려움이 있을 수 있을까요?팩토리에서 세 개의 리포지토리를 주입받는데, 이것이 Single Responsibility Principle 관점에서는 어떻게 평가할 수 있을까요?
만약 나중에 리포지토리 구현을 변경하거나 목 객체로 교체해야 한다면 현재 구조에서는 어떤 방법들이 있을까요?
Hilt나 다른 DI 프레임워크와 비교했을 때 현재 방식의 장점과 단점을 정리해보시고, 프로젝트 규모가 커질 때 어떤 문제가 생길 수 있는지 예상해보시면 좋겠습니다.
app/src/main/java/woowacourse/shopping/data/remote/OkHttpClientProvider.kt (1)
12-16
: 인증 정보 로딩 시점과 오류 처리를 고민해보세요현재 object 초기화 시점에 UserPreference에서 인증 정보를 가져오고 있습니다. 만약 이 시점에 인증 정보가 없거나 잘못된 경우 어떤 일이 일어날까요?
인증 정보를 언제 로드하는 것이 더 안전하고 효율적일지, 그리고 오류 상황을 어떻게 처리할지 생각해보세요.
app/src/main/java/woowacourse/shopping/data/dto/cart/CartsResponse.kt (1)
21-25
:✅ Verification successful
의존하는 클래스들이 정의되어 있는지 확인해보세요
Pageable
과SortX
클래스를 참조하고 있는데, 이 클래스들이 어디에 정의되어 있는지 확인해보세요.만약 이 클래스들이 아직 정의되지 않았다면, 어떤 구조로 만들어야 할지 생각해보세요. API 응답 구조를 보고 필요한 필드들을 파악할 수 있나요?
다음 스크립트를 실행해서 Pageable과 SortX 클래스가 정의되어 있는지 확인해보세요:
🏁 Script executed:
#!/bin/bash # Pageable과 SortX 클래스 정의 확인 echo "Pageable 클래스 검색:" fd "Pageable.kt" echo "SortX 클래스 검색:" fd "SortX.kt" echo "관련 클래스들 검색:" rg -A 5 "class Pageable|data class Pageable|class SortX|data class SortX"Length of output: 2644
의존 클래스 정의 확인 완료
Pageable
및SortX
클래스가 아래 경로에 정의되어 있어CartsResponse
에서의 참조가 유효함을 확인했습니다.
- app/src/main/java/woowacourse/shopping/data/dto/cart/Pageable.kt
- app/src/main/java/woowacourse/shopping/data/dto/cart/SortX.kt
app/src/androidTest/java/woowacourse/shopping/view/ProductActivityTest.kt (1)
18-26
: 테스트 클래스 구조가 잘 정리되어 있습니다!Intents 초기화와 해제가 적절히 처리되어 있어 좋습니다.
한 가지 고려해볼 점이 있는데, 현재 각 테스트가 독립적으로 실행되면서 매번 Activity가 새로 생성됩니다. 테스트 성능을 향상시키거나 테스트 간 상태 공유가 필요한 경우가 있는지 생각해보세요. 어떤 상황에서 테스트 간 상태를 공유해야 할까요?
app/src/main/java/woowacourse/shopping/presentation/ProductUiModel.kt (1)
6-12
: UI 모델 설계가 깔끔합니다!도메인과 프레젠테이션 레이어를 분리한 접근이 좋습니다.
category
필드에 기본값으로 빈 문자열을 사용하신 이유가 있나요? 이런 기본값 설정이 어떤 상황에서 유용할지, 그리고 다른 대안(nullable, sealed class 등)과 비교했을 때의 장단점을 생각해보세요.또한
price
를Int
로 표현하는 것이 모든 상황에 적합한지도 고려해보면 좋겠습니다. 다양한 통화나 소수점이 있는 가격을 어떻게 처리할 수 있을까요?app/src/main/java/woowacourse/shopping/data/datasource/remote/CartDataSource.kt (1)
5-30
: 데이터 소스 인터페이스가 깔끔하게 설계되어 있습니다!콜백 기반의 비동기 처리와
Result
래퍼를 사용한 에러 핸들링이 일관성 있게 적용되어 있어 좋습니다.설계에 대해 몇 가지 생각해볼 점들이 있습니다:
- 현재 콜백 패턴을 사용하고 있는데, 코루틴이나 RxJava 같은 다른 비동기 패턴과 비교했을 때의 장단점은 무엇일까요?
getPagedCartItems
만Result
를 사용하지 않는 이유가 있나요? 이 메서드에서도 에러가 발생할 수 있지 않을까요?- 메서드별로 다른 에러 타입이 필요한 상황이 있을까요?
이런 설계 결정들의 근거를 한 번 정리해보시고, 일관성을 유지하는 것이 중요한지 고민해보세요.
app/src/main/AndroidManifest.xml (3)
5-5
: 네트워크 권한 추가가 적절합니다INTERNET 권한이 추가되어 원격 데이터 통신이 가능해졌습니다. 이 권한이 어떤 네트워크 작업을 위해 필요한지 생각해보세요.
8-8
: Application 클래스 설정을 확인해보세요ShoppingApplication 클래스가 등록되었습니다. 이 클래스에서 어떤 초기화 작업들을 수행하고 있는지, 그리고 의존성 주입이나 전역 설정이 올바르게 이루어지고 있는지 확인해보시기 바랍니다.
ShoppingApplication 클래스의 구현을 검토하여 다음을 확인해보세요:
- 데이터베이스 초기화가 적절한가?
- Repository 모듈들이 올바르게 설정되었는가?
- 메모리 누수 가능성은 없는가?
17-17
: 보안 설정에 대해 고민해보세요
usesCleartextTraffic="true"
설정이 추가되었습니다. 이 설정이 필요한 이유와 보안상 고려사항이 무엇인지 생각해보세요.다음 질문들을 스스로 답해보시기 바랍니다:
- 현재 API 서버가 HTTPS를 지원하나요?
- 프로덕션 환경에서도 이 설정이 필요한가요?
- 대안적인 해결 방법은 없을까요?
app/src/main/java/woowacourse/shopping/data/datasource/local/RecentProductDataSourceImpl.kt (2)
6-8
: 데이터 소스 구조에 대해 생각해보세요RecentProductDataSource 인터페이스를 구현하여 DAO를 래핑하고 있습니다. 이런 추상화 계층이 왜 필요한지, 어떤 장점을 제공하는지 생각해보세요.
다음 질문들을 스스로 답해보시기 바랍니다:
- Repository 패턴에서 이 계층의 역할은 무엇인가요?
- 향후 원격 데이터 소스를 추가할 때 어떤 도움이 될까요?
13-13
: 예외 처리에 대해 생각해보세요
getOldestProduct()
메서드가 nullable이 아닌 타입을 반환합니다. 데이터가 없을 때 어떤 일이 발생할지 생각해보세요.다음을 확인해보시기 바랍니다:
- DAO 구현에서 데이터가 없을 때 어떻게 처리하나요?
- 이 메서드를 호출하는 곳에서 예외 처리가 적절히 되어 있나요?
- null-safety를 보장하는 더 나은 방법은 없을까요?
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendViewHolder.kt (2)
12-20
: ViewHolder 구조를 분석해보세요두 개의 서로 다른 리스너를 받는 구조로 설계되었습니다. 이런 설계 결정의 이유에 대해 생각해보세요.
다음 질문들을 고민해보시기 바랍니다:
- ItemClickListener와 CartCounterClickListener를 분리한 이유는?
- 단일 책임 원칙 관점에서 이 구조의 장점은?
- 리스너를 init 블록에서 설정하는 것의 장단점은?
26-42
: 팩토리 패턴 활용을 칭찬합니다companion object의 create 메서드를 통해 ViewHolder 생성을 캡슐화한 것이 좋습니다.
이 패턴의 장점들을 스스로 정리해보세요:
- 객체 생성 로직의 캡슐화
- 어댑터에서의 사용 편의성
- 향후 생성자 변경 시의 유연성
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendAdapter.kt (3)
9-14
: ListAdapter 사용이 적절합니다
ListAdapter
와ItemDiffUtil
을 사용하여 효율적인 리스트 업데이트를 구현했습니다.이 선택의 장점들을 생각해보세요:
- DiffUtil을 통한 성능 최적화
- 자동적인 애니메이션 처리
- 메모리 효율성
15-18
: 객체 생성 패턴을 확인해보세요ViewHolder 생성을
RecommendViewHolder.create()
에 위임하고 있습니다. 이런 패턴의 장점에 대해 생각해보세요.다음 질문을 스스로 답해보시기 바랍니다:
- 직접 생성자를 호출하는 것 대신 factory 메서드를 사용하는 이유는?
- 코드의 가독성과 유지보수성 측면에서 어떤 장점이 있을까요?
20-26
: 어댑터의 역할에 대해 생각해보세요현재 어댑터가 매우 간단하게 구현되어 있습니다. 어댑터의 핵심 책임이 무엇인지, 그리고 현재 구현이 그 책임을 잘 수행하고 있는지 생각해보세요.
다음 관점에서 현재 구현을 평가해보시기 바랍니다:
- 단일 책임 원칙을 잘 따르고 있나요?
- 향후 기능 추가 시 확장하기 쉬운 구조인가요?
- 테스트하기 쉬운 구조인가요?
app/build.gradle.kts (4)
7-8
: 플러그인 설정이 적절합니다Room 데이터베이스와 Kotlin 직렬화를 위한 필수 플러그인들이 올바르게 추가되었습니다.
23-24
: JUnit 5 설정이 올바릅니다Android에서 JUnit 5를 사용하기 위한 runner 설정이 적절히 구성되었습니다.
60-63
: 빌드 기능 설정이 적절합니다데이터 바인딩과 빌드 설정 기능이 올바르게 활성화되었습니다.
97-99
: 테스트 의존성이 잘 구성되었습니다테스트를 위한 필요한 라이브러리들이 적절히 추가되었습니다. 테스트 작성에 도움이 될 것입니다.
app/src/main/java/woowacourse/shopping/presentation/product/RecentAdapter.kt (2)
9-12
: 어댑터 구조가 잘 설계되었습니다ItemClickListener를 통한 클릭 이벤트 처리와 초기화가 적절히 구현되었습니다.
19-35
: ViewHolder 관련 메서드들이 적절히 구현되었습니다표준적인 RecyclerView 어댑터 패턴을 잘 따르고 있습니다.
app/src/main/java/woowacourse/shopping/presentation/CartItemUiModel.kt (1)
5-11
: UI 모델 설계가 훌륭합니다도메인 모델과 UI 모델을 분리하고, UI에만 필요한
isSelected
속성을 추가한 것이 좋은 설계입니다. 레이어 간 관심사 분리가 잘 되어 있네요.app/src/main/res/layout/item_cart_product.xml (2)
6-19
: 데이터 바인딩 변수들이 잘 정의되었습니다필요한 데이터와 리스너들이 명확하게 정의되어 있어 레이아웃에서 효과적으로 사용할 수 있겠네요.
41-53
: 상품명 표시가 깔끔합니다텍스트 스타일링과 레이아웃 제약조건이 적절히 설정되어 있습니다.
app/src/main/res/layout/activity_cart.xml (2)
6-15
: 데이터 바인딩 변수 선언이 적절합니다ViewModel과 ClickListener를 데이터 바인딩 변수로 선언한 구조가 MVVM 패턴에 잘 맞습니다. 변수명도 명확하게 정의되어 있어 좋습니다.
62-63
:✅ Verification successful
데이터 바인딩 표현식 검증 필요
isCheckAll
속성과onClickCheckAll()
메서드가 ViewModel과 ClickListener에 올바르게 구현되어 있는지 확인이 필요합니다.다음 스크립트로 해당 메서드들이 정의되어 있는지 확인해보세요:
🏁 Script executed:
#!/bin/bash # ViewModel의 isCheckAll 속성과 ClickListener의 onClickCheckAll 메서드 확인 rg -A 3 "isCheckAll" --type kotlin rg -A 3 "onClickCheckAll" --type kotlinLength of output: 2819
데이터 바인딩 표현식 검증 완료
CartViewModel.kt
에val isCheckAll: LiveData<Boolean>
(내부_isCheckAll
포함)이 정의되어 있습니다.CartPageClickListener.kt
에fun onClickCheckAll()
이 선언되어 있고,CartActivity.kt
에서 해당 메서드가 구현되어 호출되고 있습니다.위 확인 결과, XML의
android:checked="@{vm.isCheckAll}" android:onClick="@{() -> clickListener.onClickCheckAll()}"바인딩 표현식은 문제없이 작동합니다.
app/src/main/java/woowacourse/shopping/data/remote/CartItemService.kt (1)
29-33
:✅ Verification successful
RESTful API 규칙 준수 확인
PATCH 메서드를 사용하여 부분 업데이트를 수행하는 것은 적절합니다. 하지만
UpdateCartRequest
가 실제로 부분 업데이트를 지원하는지 확인이 필요합니다.다음 스크립트로
UpdateCartRequest
DTO의 구조를 확인해보세요:
🏁 Script executed:
#!/bin/bash # UpdateCartRequest DTO 구조 확인 rg -A 10 "class UpdateCartRequest" --type kotlinLength of output: 428
RESTful API 부분 업데이트 지원 확인
UpdateCartRequest
가 오직 갱신 대상인quantity
필드만을 포함하고 있어, PATCH를 사용한 부분 업데이트 요구사항을 충족합니다. 추가 수정은 필요하지 않습니다.app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendActivity.kt (1)
76-84
: companion object의 newIntent 메서드 구현 우수Intent 생성을 위한 정적 팩토리 메서드가 잘 구현되어 있습니다. 이는 Activity 시작을 위한 표준적인 패턴입니다.
app/src/main/res/layout/activity_detail_product.xml (2)
6-17
: 데이터 바인딩 구조가 잘 설계되어 있습니다.ViewModel과 ItemClickListener를 분리하여 관심사를 명확히 구분한 점이 좋습니다. 이런 설계 패턴을 선택한 이유를 생각해보세요. 다른 액티비티에서도 비슷한 패턴을 사용하고 있나요?
94-105
: 최근 본 상품 영역의 가시성 로직을 점검해보세요.
app:visibleIf="@{!vm.isRecentProduct}"
에서 부정 연산자(!
)를 사용하고 있습니다. 이 로직이 의도한 대로 동작하는지 확인해보세요.isRecentProduct
가 true일 때와 false일 때 각각 어떤 UI 상태를 원하는 건가요?app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailActivity.kt (1)
41-45
: Intent에서 데이터를 안전하게 가져오는 방법을 고민해보세요.productId를 0L로 기본값 설정했는데, 만약 실제로 0이 전달되면 어떤 일이 일어날까요? 이런 예외 상황을 어떻게 처리하면 좋을지 생각해보세요.
app/src/main/java/woowacourse/shopping/presentation/cart/CartActivity.kt (2)
22-33
: 어댑터 초기화 패턴을 분석해보세요.
lazy
키워드를 사용해서 어댑터를 초기화했습니다. 이 방식의 장점이 무엇인지, 언제 어댑터가 실제로 생성되는지 생각해보세요.
51-55
: 액티비티 생명주기를 고려해보세요.
onRestart()
에서 데이터를 다시 로드하고 있습니다. 이것이 필요한 시나리오가 무엇인지 생각해보세요. 다른 생명주기 메서드(onResume
등)를 사용할 수도 있는데 왜onRestart
를 선택했을까요?app/src/main/java/woowacourse/shopping/presentation/product/ProductActivity.kt (1)
58-62
: onRestart에서 데이터를 다시 로딩하는 이유를 고려해보세요.
onRestart()
에서fetchData()
와fetchCartItemCount()
를 호출하고 있습니다.이 접근 방식에 대해 생각해보세요:
- 매번 다시 시작할 때마다 데이터를 새로 가져오는 것이 필요한가요?
- 사용자 경험 측면에서 어떤 영향이 있을까요?
- 더 효율적인 데이터 동기화 방법이 있을까요?
app/src/main/java/woowacourse/shopping/data/repository/local/RecentProductRepositoryImpl.kt (1)
49-50
: 비동기 작업에서 Result 처리 패턴을 확인해보세요.
runThread
블록 내에서Result.success(Unit)
를 반환하고 있는데, 이것이runThread
의runCatching
과 어떻게 상호작용하는지 생각해보세요.
app/src/main/java/woowacourse/shopping/data/ThreadUtils.kt
의runThread
구현을 보면 이미runCatching
으로 감싸고 있습니다. 명시적으로Result.success()
를 반환할 필요가 있을까요?app/src/main/java/woowacourse/shopping/presentation/cart/CartAdapter.kt (1)
50-66
: ViewHolder 패턴 구현이 잘 되어 있네요!데이터 바인딩을 사용한 ViewHolder 구현이 깔끔합니다.
executePendingBindings()
호출도 적절하게 되어 있어서 바인딩 성능 최적화가 잘 되어 있습니다.app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailViewModel.kt (1)
74-80
: 최근 상품 삽입 실패를 어떻게 처리해야 할까요?최근 상품 삽입이 실패했을 때 단순히 토스트 메시지만 보여주는 것이 적절한지 생각해보셨나요? 사용자 경험 관점에서 이 기능이 실패했을 때 어떤 대안적인 처리 방법이 있을까요?
app/src/main/java/woowacourse/shopping/data/repository/remote/CartRepositoryImpl.kt (1)
13-13
: 캐시 초기화 전략을 고민해보셨나요?캐시를 빈
Cart()
로 초기화하고 있는데, 애플리케이션 시작 시 캐시를 어떻게 초기화할지에 대한 전략이 있나요? 사용자가 앱을 처음 열었을 때 캐시와 실제 데이터가 동기화되지 않을 수 있는 상황을 어떻게 처리할 계획인가요?app/src/main/java/woowacourse/shopping/presentation/product/ProductViewModel.kt (1)
119-129
: 비즈니스 로직의 일관성을 확인해보세요
addToCart
메서드에서 수량을 1로 고정하고 있습니다:cartRepository.insertProduct(cartItem.product, 1) { result ->그런데 123번 줄에서는:
updateQuantity(productId = cartItem.product.productId, 1)
- 이미 장바구니에 있는 상품을 추가할 때 어떤 일이 일어날까요?
- 서버에서 어떻게 처리되는지 확인해보셨나요?
- 사용자 경험 관점에서 이 동작이 직관적일까요?
다른 쇼핑 앱들의 동작과 비교해보세요.
app/src/main/java/woowacourse/shopping/data/DummyProducts.kt (1)
3-253
: 주석 처리된 코드의 목적과 중복 데이터를 검토해보세요.전체 파일이 주석 처리되어 있는데, 몇 가지 질문이 있습니다:
- 이 더미 데이터는 어떤 목적으로 만들어졌나요? (개발용? 테스트용?)
- ID 0-19번과 20-39번 상품들을 보면 이름과 가격이 동일한 중복 데이터가 있습니다. 이것이 의도된 것인가요?
- 주석 처리된 채로 코드베이스에 남겨둘 필요가 있을까요?
다음을 고려해보세요:
- 실제로 사용할 계획이 있다면 주석을 해제하고 중복 데이터를 정리
- 사용하지 않는다면 파일을 삭제
- 테스트용이라면 test 디렉토리로 이동
현재 프로젝트에서 이 데이터를 어떻게 활용할 계획인지 팀과 논의해보세요.
@Entity(tableName = "cart") | ||
data class CartEntity( | ||
@PrimaryKey @ColumnInfo(name = "product_id") val productId: Long, | ||
@ColumnInfo(name = "quantity") val quantity: Int, | ||
) |
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.
🛠️ Refactor suggestion
데이터베이스 설계 결정에 대해 생각해보세요.
현재 productId
를 기본 키로 사용하고 있습니다.
이 설계의 의미를 생각해보세요:
- 한 사용자의 장바구니에 같은 상품이 여러 번 들어갈 수 있나요?
- 다른 사용자들은 어떻게 구분하나요?
- 장바구니 아이템의 고유성을 어떻게 보장하나요?
질문:
- 현재 설계가 비즈니스 요구사항에 맞나요?
- 사용자별 장바구니 구분이 필요하다면 어떻게 개선할 수 있을까요?
힌트: 실제 앱의 사용자 관리 방식과 장바구니 요구사항을 함께 고려해보세요.
🤖 Prompt for AI Agents
In app/src/main/java/woowacourse/shopping/data/entity/CartEntity.kt around lines
7 to 11, the current design uses productId as the primary key, which does not
distinguish between different users' carts or allow multiple quantities of the
same product per user. To fix this, modify the entity to include a user
identifier (e.g., userId) and use a composite primary key consisting of userId
and productId to uniquely identify each cart item per user, ensuring proper
separation and uniqueness of cart entries.
@BindingAdapter("imageUrl") | ||
fun setLoadImage( | ||
view: ImageView, | ||
url: String?, | ||
) { | ||
Glide | ||
.with(view) | ||
.load(url) | ||
.fallback(R.drawable.ic_delete) | ||
.error(R.drawable.ic_delete) | ||
.into(view) | ||
} |
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.
🛠️ Refactor suggestion
이미지 로딩 처리에 대해 생각해볼 점들이 있습니다.
현재 구현이 기본적으로는 잘 작동하지만, 사용자 경험 측면에서 개선할 수 있는 부분들이 있습니다:
-
fallback
과error
에 모두ic_delete
를 사용하고 있는데, 이게 의도된 것인가요? 이 두 상황의 차이점은 무엇일까요? -
이미지 로딩 중에 사용자가 보게 될 화면은 어떨까요?
-
ic_delete
아이콘이 이미지 로딩 실패나 대체 이미지로 적절한 선택일까요?
이런 질문들을 통해 더 나은 사용자 경험을 제공할 수 있는 방법을 생각해보세요. Glide 공식 문서에서 placeholder()
, fallback()
, error()
메서드들의 차이점을 확인해보시면 도움이 될 것입니다.
🤖 Prompt for AI Agents
In
app/src/main/java/woowacourse/shopping/presentation/bindingadapter/ImageViewBindingAdapter.kt
around lines 8 to 19, improve user experience by differentiating the images used
for placeholder, fallback, and error states. Replace the current use of
ic_delete for both fallback and error with more appropriate images: use
placeholder() to show a temporary image while loading, fallback() for when the
URL is null, and error() for when loading fails. Choose images that clearly
communicate each state without confusing or alarming the user.
// "https://sitem.ssgcdn.com/44/58/17/item/1000555175844_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 25, | ||
// "노브랜드 코코넛 커피", | ||
// Price(3000), | ||
// "https://sitem.ssgcdn.com/24/94/44/item/1000595449424_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 26, | ||
// "스타벅스 미디엄 로스트 아메리카노", | ||
// Price(12300), | ||
// "https://sitem.ssgcdn.com/10/22/20/item/1000602202210_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 27, | ||
// "맥심 아라비카 100", | ||
// Price(3000), | ||
// "https://sitem.ssgcdn.com/82/57/69/item/0000007695782_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 28, | ||
// "콜드브루 미니컵 커피", | ||
// Price(9000), | ||
// "https://sitem.ssgcdn.com/23/32/44/item/1000641443223_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 29, | ||
// "아라비카 시그니처 블렌디드", | ||
// Price(17000), | ||
// "https://sitem.ssgcdn.com/45/77/23/item/1000646237745_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 30, | ||
// "카누 에스프레스 쇼콜라 라떼", | ||
// Price(11000), | ||
// "https://sitem.ssgcdn.com/66/68/49/item/1000632496866_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 31, | ||
// "이디야 트리플 아메리카노", | ||
// Price(13200), | ||
// "https://sitem.ssgcdn.com/14/47/79/item/1000626794714_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 32, | ||
// "테이스터스 초이 오리지날", | ||
// Price(8800), | ||
// "https://sitem.ssgcdn.com/30/03/11/item/1000563110330_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 33, | ||
// "카누 에스프레스 말차 라떼", | ||
// Price(9100), | ||
// "https://sitem.ssgcdn.com/04/25/36/item/1000567362504_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 34, | ||
// "맥심 카누 돌체라떼 8입", | ||
// Price(4550), | ||
// "https://sitem.ssgcdn.com/64/88/40/item/1000105408864_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 35, | ||
// "노브랜드 핸드드립 콜롬비아", | ||
// Price(12500), | ||
// "https://sitem.ssgcdn.com/38/81/38/item/1000057388138_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 36, | ||
// "콜롬비아 블렌드 원두", | ||
// Price(15900), | ||
// "https://sitem.ssgcdn.com/78/64/42/item/1000036426478_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 37, | ||
// "노브랜드 헤이즐넛향 드립백 원두커피", | ||
// Price(7500), | ||
// "https://sitem.ssgcdn.com/58/17/14/item/1000428141758_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 38, | ||
// "네스카페 수프리모 아메리카노 블랙", | ||
// Price(98000), | ||
// "https://sitem.ssgcdn.com/07/86/28/item/1000011288607_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 39, | ||
// "이디야 스틱 커피 오리지날 아메리카노", | ||
// Price(99000), | ||
// "https://sitem.ssgcdn.com/36/98/10/item/1000007109836_i1_464.jpg", | ||
// ), | ||
// Product( | ||
// 40, | ||
// "이디야 스틱 커피 오리지날 아메리카노2", | ||
// Price(1000), | ||
// "https://sitem.ssgcdn.com/36/98/10/item/1000007109836_i1_464.jpg", | ||
// ), | ||
// ) | ||
// } |
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.
🛠️ Refactor suggestion
주석 처리된 코드의 목적과 관리 방안을 고려해보세요.
현재 전체 DummyProducts
객체가 주석 처리되어 있는데, 몇 가지 질문을 해보겠습니다:
-
이 코드가 현재 프로젝트에서 어떤 역할을 해야 할까요? 개발용 데이터인가요, 테스트용인가요?
-
데이터를 자세히 보시면 ID 0-19번 상품들이 20-39번으로 중복되어 있습니다. 이게 의도된 것일까요?
-
주석 처리된 코드를 프로덕션 코드베이스에 유지하는 것이 좋은 방법일까요?
만약 테스트를 위한 데이터가 필요하다면, 어떤 다른 접근 방식들이 있을지 생각해보세요. 예를 들어, 테스트 디렉토리에 별도로 관리하거나, 더 작고 명확한 목적을 가진 테스트 데이터를 만드는 방법은 어떨까요?
코드의 목적이 명확하지 않을 때는 동료 개발자나 멘토와 상의해보는 것도 좋은 방법입니다.
🤖 Prompt for AI Agents
In app/src/main/java/woowacourse/shopping/data/DummyProducts.kt around lines 3
to 253, the entire DummyProducts object is commented out and contains duplicated
product IDs from 0-19 and 20-39. First, clarify the purpose of this data—whether
it is for development, testing, or production. If it is for testing, move this
data to a dedicated test resource or test data file instead of keeping it in the
main codebase. Remove duplicated entries to avoid confusion and maintain a
smaller, clearer dataset. Finally, avoid keeping large commented-out blocks in
production code; either delete or relocate them appropriately after confirming
their necessity with your team.
@BindingAdapter("price", "count") | ||
fun setFormattedPrice( | ||
view: TextView, | ||
price: Int, | ||
count: Int, | ||
) { | ||
val total = price * count | ||
val context = view.context | ||
view.text = context.getString(R.string.product_detail_price, total) | ||
} |
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.
🛠️ Refactor suggestion
가격 계산 로직에 대해 몇 가지 고려해볼 점들이 있습니다.
문자열 리소스를 사용한 점과 컨텍스트를 적절히 활용한 점은 좋은 구현입니다.
하지만 가격 계산 부분에서 생각해볼 점들이 있습니다:
price
나count
가 음수일 때는 어떻게 처리해야 할까요?- 큰 수의 곱셈에서 오버플로우가 발생할 가능성은 없을까요?
- 실제 쇼핑 앱에서 가격 계산 시 고려해야 할 다른 요소들은 무엇이 있을까요? (예: 할인, 세금 등)
현재는 간단한 곱셈이지만, 실제 상용 애플리케이션에서는 어떤 검증이나 예외 처리가 필요할지 생각해보세요.
비즈니스 로직이 더 복잡해진다면, 이런 계산을 별도의 유틸리티 클래스나 도메인 객체로 분리하는 것도 고려해볼 만합니다.
🤖 Prompt for AI Agents
In
app/src/main/java/woowacourse/shopping/presentation/bindingadapter/TextViewBindingAdapter.kt
around lines 7 to 16, the price calculation does not handle negative values or
potential integer overflow. Add validation to ensure price and count are
non-negative before calculation, and consider using a larger numeric type like
Long to prevent overflow when multiplying large values. Also, think about
extending the logic to handle discounts, taxes, or other pricing rules, and
consider moving this calculation into a dedicated utility or domain class to
separate business logic from UI binding.
override fun onCreate() { | ||
super.onCreate() | ||
DatabaseModule.init(this) | ||
DataSourceModule.init(this) | ||
RepositoryModule.init(this) | ||
UserPreference.init(this) | ||
} |
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.
🛠️ Refactor suggestion
모듈 초기화 시 고려해볼 점들이 있습니다.
현재 구현에서 몇 가지 생각해볼 부분이 있어요:
- 모듈 초기화 순서가 중요할 수 있는데, 현재 순서가 의존성을 고려한 것인지 확인해보셨나요?
- 초기화 중 예외가 발생하면 어떻게 처리할 계획인가요?
- 모든 초기화가 메인 스레드에서 실행되는데, 앱 시작 시간에 영향을 줄 수 있을까요?
이런 상황들을 어떻게 처리하면 좋을지 생각해보시고, 필요하다면 초기화 실패 시나리오에 대한 대응 방안도 고민해보세요.
🤖 Prompt for AI Agents
In app/src/main/java/woowacourse/shopping/ShoppingApplication.kt around lines 10
to 16, review the module initialization order to ensure dependencies are
correctly handled. Add try-catch blocks around each module's init call to handle
potential exceptions gracefully, possibly logging errors or providing fallback
behavior. Consider moving heavy or non-critical initialization off the main
thread using background threads or asynchronous tasks to avoid slowing app
startup.
<androidx.recyclerview.widget.RecyclerView | ||
android:id="@+id/rv_recommend" | ||
android:layout_width="match_parent" | ||
android:layout_height="0dp" | ||
android:layout_marginTop="24dp" | ||
android:orientation="horizontal" | ||
app:layoutManager="androidx.recyclerview.widget.LinearLayoutManager" | ||
app:layout_constraintTop_toBottomOf="@id/tv_recommend_desc" | ||
tools:itemCount="4" | ||
tools:layout_editor_absoluteX="0dp" | ||
tools:listitem="@layout/item_product" /> |
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.
RecyclerView의 제약 조건을 더 명확하게 설정해보세요.
추천 상품 목록의 높이가 0dp
로 설정되어 있지만 bottom constraint가 없습니다.
질문: 이 RecyclerView의 실제 높이는 어떻게 결정될까요? 의도한 대로 동작할까요?
힌트: ConstraintLayout에서 0dp
를 사용할 때는 양쪽 제약 조건이 모두 필요합니다. 하단 제약 조건을 추가하거나 wrap_content
를 고려해보세요.
🤖 Prompt for AI Agents
In app/src/main/res/layout/activity_recommend.xml lines 58 to 68, the
RecyclerView has its height set to 0dp but lacks a bottom constraint, which
means its height cannot be properly determined in ConstraintLayout. To fix this,
add a bottom constraint to the RecyclerView to define its vertical boundaries,
or alternatively change the height to wrap_content if that fits the design
intent, ensuring the RecyclerView's height is correctly calculated and
displayed.
<TextView | ||
android:id="@+id/layout_cart_total_price" | ||
android:layout_width="0dp" | ||
android:layout_height="78dp" | ||
android:background="@color/gray5" | ||
app:layout_constraintBottom_toBottomOf="parent" | ||
app:layout_constraintEnd_toStartOf="@id/layout_cart_recommend" | ||
app:layout_constraintHorizontal_chainStyle="spread" | ||
app:layout_constraintStart_toStartOf="parent" /> |
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.
🛠️ Refactor suggestion
하단 영역의 접근성을 개선해보세요.
가격 표시와 주문 버튼 영역에 접근성 속성들이 빠져있습니다.
질문: 시각 장애인 사용자가 이 화면을 사용할 때 어떤 어려움이 있을까요?
힌트: android:contentDescription
, android:accessibilityLiveRegion
같은 속성들을 찾아보세요. 특히 동적으로 변하는 가격 정보에 대해서요.
참고: Android 접근성 가이드라인을 참고해보세요.
Also applies to: 101-114
🤖 Prompt for AI Agents
In app/src/main/res/layout/activity_recommend.xml around lines 70 to 78 and also
lines 101 to 114, the TextView displaying the total price and the order button
area lack accessibility attributes. To fix this, add android:contentDescription
to provide descriptive text for screen readers, and use
android:accessibilityLiveRegion="polite" on the price TextView to notify users
of dynamic price changes. This will improve usability for visually impaired
users by making the UI elements understandable and updates announced properly.
<ImageView | ||
android:id="@+id/iv_cart_product" | ||
android:layout_width="136dp" | ||
android:layout_height="72dp" | ||
android:layout_marginTop="20dp" | ||
android:contentDescription="@null" | ||
android:scaleType="center" | ||
app:imageUrl="@{cartItem.product.imageUrl}" | ||
app:layout_constraintStart_toStartOf="@id/checkbox_cart_item" | ||
app:layout_constraintTop_toBottomOf="@id/tv_cart_product_name" | ||
tools:src="@drawable/ic_launcher_background" /> |
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.
🛠️ Refactor suggestion
이미지 로딩 구현이 좋습니다
커스텀 바인딩 어댑터를 통한 이미지 로딩이 잘 구현되어 있습니다.
접근성 개선을 고려해보세요:
contentDescription
이null
로 설정되어 있는데, 상품 이미지에 대한 설명이 필요하지 않을까요?- 예를 들어, 상품명을 contentDescription으로 사용하는 것은 어떨까요?
시각적 장애가 있는 사용자의 앱 사용 경험에 대해 생각해보세요.
🤖 Prompt for AI Agents
In app/src/main/res/layout/item_cart_product.xml around lines 55 to 65, the
ImageView's contentDescription is set to null, which reduces accessibility for
visually impaired users. Update the contentDescription attribute to dynamically
use the product name (e.g., cartItem.product.name) so screen readers can
describe the image meaningfully. This improves the app's accessibility by
providing context for the product image.
<ImageButton | ||
android:id="@+id/ib_cart_product_delete" | ||
android:layout_width="48dp" | ||
android:layout_height="48dp" | ||
android:backgroundTint="@color/white" | ||
android:contentDescription="@null" | ||
android:onClick="@{() -> cartPageClickListener.onClickDelete(cartItem)}" | ||
android:src="@drawable/ic_delete" | ||
app:layout_constraintBottom_toBottomOf="@id/tv_cart_product_name" | ||
app:layout_constraintEnd_toEndOf="parent" | ||
app:layout_constraintTop_toTopOf="@id/tv_cart_product_name" /> |
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.
🛠️ Refactor suggestion
삭제 버튼 기능은 잘 구현되었지만 접근성을 개선해야 합니다
클릭 이벤트 처리는 적절히 구현되어 있습니다.
하지만 중요한 개선점이 있습니다:
- 삭제 버튼에
contentDescription
이 없어서 스크린 리더 사용자가 버튼의 용도를 알 수 없습니다. - "상품 삭제" 또는 "장바구니에서 제거" 같은 명확한 설명이 필요합니다.
접근성은 모든 사용자를 위한 중요한 요소입니다. Android 접근성 가이드라인을 확인해보세요.
🤖 Prompt for AI Agents
In app/src/main/res/layout/item_cart_product.xml between lines 67 and 77, the
ImageButton for deleting a cart product lacks a meaningful contentDescription,
which is essential for accessibility. Replace android:contentDescription="@null"
with a descriptive string such as "상품 삭제" or "장바구니에서 제거" to clearly convey the
button's purpose to screen reader users. This improves accessibility by
providing context for visually impaired users.
@Query("UPDATE cart SET quantity = quantity + :quantity WHERE product_id = :productId") | ||
fun increaseQuantity( | ||
productId: Long, | ||
quantity: Int, | ||
) |
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.
🛠️ Refactor suggestion
수량 증가 시 음수 값 검증 필요
increaseQuantity
메서드에서 음수 값이 전달될 경우 의도하지 않은 결과가 발생할 수 있습니다.
Repository 레이어에서 매개변수 검증을 추가하거나, DAO 레벨에서 CHECK 제약조건을 고려해보세요.
비즈니스 로직은 DAO보다는 Repository나 UseCase에서 처리하는 것이 더 적절합니다.
🤖 Prompt for AI Agents
In app/src/main/java/woowacourse/shopping/data/dao/CartDao.kt around lines 34 to
38, the increaseQuantity method lacks validation for negative quantity values,
which can cause unintended behavior. Add validation in the repository layer or
use case to ensure the quantity parameter is positive before calling this DAO
method. Optionally, consider adding a CHECK constraint at the database level to
prevent negative quantities. Avoid placing business logic in the DAO; keep
validation in higher layers.
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.
Review continued from previous batch...
@Query("UPDATE cart SET quantity = quantity + :quantity WHERE product_id = :productId") | ||
fun increaseQuantity( | ||
productId: Long, | ||
quantity: Int, | ||
) | ||
|
||
@Query("UPDATE cart SET quantity = quantity - 1 WHERE product_id = :productId") | ||
fun decreaseQuantity(productId: Long) |
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.
🛠️ Refactor suggestion
quantity 업데이트 메서드의 매개변수 일관성 부족
increaseQuantity
는 증가량을 매개변수로 받지만, decreaseQuantity
는 항상 1씩만 감소시킵니다. 이는 API의 일관성을 해칩니다.
다음과 같이 일관성을 맞춰보세요:
- @Query("UPDATE cart SET quantity = quantity - 1 WHERE product_id = :productId")
- fun decreaseQuantity(productId: Long)
+ @Query("UPDATE cart SET quantity = quantity - :quantity WHERE product_id = :productId")
+ fun decreaseQuantity(productId: Long, quantity: Int = 1)
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
@Query("UPDATE cart SET quantity = quantity + :quantity WHERE product_id = :productId") | |
fun increaseQuantity( | |
productId: Long, | |
quantity: Int, | |
) | |
@Query("UPDATE cart SET quantity = quantity - 1 WHERE product_id = :productId") | |
fun decreaseQuantity(productId: Long) | |
@Query("UPDATE cart SET quantity = quantity + :quantity WHERE product_id = :productId") | |
fun increaseQuantity( | |
productId: Long, | |
quantity: Int, | |
) | |
@Query("UPDATE cart SET quantity = quantity - :quantity WHERE product_id = :productId") | |
fun decreaseQuantity(productId: Long, quantity: Int = 1) |
🤖 Prompt for AI Agents
In app/src/main/java/woowacourse/shopping/data/dao/CartDao.kt around lines 34 to
41, the increaseQuantity method accepts a quantity parameter to specify the
increment amount, but decreaseQuantity always decreases by 1 without a
parameter, causing inconsistency. To fix this, modify decreaseQuantity to accept
a quantity parameter like increaseQuantity and update the query to subtract the
given quantity, ensuring both methods have consistent parameter usage.
@Query("SELECT quantity FROM cart WHERE product_id = :productId") | ||
fun getQuantityById(productId: Long): Int | ||
|
||
@Query("SELECT SUM(quantity) FROM cart") | ||
fun getTotalQuantity(): Int? |
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.
🛠️ Refactor suggestion
일관성 없는 nullable 반환 타입 검토
getQuantityById
는 Int
를 반환하지만, getTotalQuantity
는 Int?
를 반환합니다. 두 메서드 모두 데이터가 없을 가능성이 있으므로 일관성을 위해 동일한 방식으로 처리해야 합니다.
다음과 같이 수정을 고려해보세요:
- fun getQuantityById(productId: Long): Int
+ fun getQuantityById(productId: Long): Int?
또는 기본값 0을 반환하도록 쿼리를 수정할 수 있습니다:
- @Query("SELECT quantity FROM cart WHERE product_id = :productId")
+ @Query("SELECT COALESCE(quantity, 0) FROM cart WHERE product_id = :productId")
fun getQuantityById(productId: Long): Int
🤖 Prompt for AI Agents
In app/src/main/java/woowacourse/shopping/data/dao/CartDao.kt around lines 19 to
23, the methods getQuantityById and getTotalQuantity have inconsistent nullable
return types; getQuantityById returns Int while getTotalQuantity returns Int?.
To fix this, make both methods consistent by either changing getQuantityById to
return Int? or modifying the queries to return 0 as a default value when no data
exists, ensuring both methods handle absence of data uniformly.
private fun observeViewModel() { | ||
viewModel.recommendProducts.observe(this) { | ||
recommendAdapter.submitList(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.
🛠️ Refactor suggestion
ViewModel 관찰 패턴 불완전
CartActivity의 observeViewModel()
과 비교했을 때, 에러 상태나 로딩 상태에 대한 처리가 누락되어 있습니다.
CartActivity를 참고하여 다음과 같은 관찰을 추가해보세요:
private fun observeViewModel() {
viewModel.recommendProducts.observe(this) {
recommendAdapter.submitList(it)
}
// 추가 필요한 관찰들
viewModel.uiState.observe(this) { state ->
// 로딩/성공/실패 상태 처리
}
viewModel.toastMessage.observe(this) { message ->
// 토스트 메시지 처리
}
}
ViewModel에 이러한 LiveData들이 있는지 확인하고 적절히 처리해주세요.
🤖 Prompt for AI Agents
In
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendActivity.kt
around lines 69 to 73, the observeViewModel() function only observes
recommendProducts and lacks handling for error and loading states. To fix this,
add observers for uiState and toastMessage LiveData from the ViewModel, similar
to CartActivity. Confirm these LiveData exist in the ViewModel, then implement
appropriate UI updates for loading, success, failure states, and toast message
display within these observers.
val price = intent.getIntExtra(Extra.KEY_SELECT_PRICE, 0) | ||
val count = intent.getIntExtra(Extra.KEY_SELECT_COUNT, 0) | ||
viewModel.fetchSelectedInfo(price, count) |
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.
🛠️ Refactor suggestion
Intent extras 검증 누락
Intent에서 가져온 값들에 대한 유효성 검증이 없습니다. 잘못된 데이터가 전달될 경우 예상치 못한 동작이 발생할 수 있습니다.
다음과 같이 입력값 검증을 추가해보세요:
val price = intent.getIntExtra(Extra.KEY_SELECT_PRICE, 0)
val count = intent.getIntExtra(Extra.KEY_SELECT_COUNT, 0)
+
+if (price < 0 || count < 0) {
+ finish()
+ return
+}
+
viewModel.fetchSelectedInfo(price, count)
🤖 Prompt for AI Agents
In
app/src/main/java/woowacourse/shopping/presentation/recommend/RecommendActivity.kt
around lines 33 to 35, the intent extras retrieved for price and count lack
validation, which may cause unexpected behavior if invalid data is passed. Add
validation checks to ensure that price and count are within expected ranges or
not default values before calling viewModel.fetchSelectedInfo. If validation
fails, handle the error appropriately, such as showing a message or using
default safe values.
private fun initInsets() { | ||
ViewCompat.setOnApplyWindowInsetsListener(binding.root) { v, insets -> | ||
val systemBars = insets.getInsets(WindowInsetsCompat.Type.systemBars()) | ||
v.setPadding(systemBars.left, systemBars.top, systemBars.right, systemBars.bottom) | ||
insets | ||
} | ||
} |
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.
🛠️ Refactor suggestion
시스템 인셋 처리 방식을 다른 액티비티와 비교해보세요.
ProductActivity.kt
의 initInsets()
메서드와 비교해보세요. 어떤 차이점이 있나요? 일관성을 위해 어떤 개선이 가능할까요?
🤖 Prompt for AI Agents
In
app/src/main/java/woowacourse/shopping/presentation/productdetail/ProductDetailActivity.kt
around lines 64 to 70, the initInsets() method sets padding using systemBars
insets but does not consume the insets afterward. Compare this with
ProductActivity.kt's initInsets() which likely consumes the insets to prevent
further propagation. To improve consistency, modify this method to consume the
insets by returning WindowInsetsCompat.CONSUMED after setting the padding.
if (response.isSuccessful) { | ||
val cartId = response.toIdOrNull() ?: throw IllegalStateException("") | ||
onResult(Result.success(cartId)) | ||
} |
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.
🛠️ Refactor suggestion
예외 처리 방식을 개선할 수 있을까요?
79번 줄에서:
val cartId = response.toIdOrNull() ?: throw IllegalStateException("")
이 방식에 대해 생각해보세요:
- 빈 메시지로 예외를 던지는 것이 디버깅에 도움이 될까요?
- 이 예외가 어디서 잡힐까요?
Result.failure
를 사용하는 방법은 어떨까요?
일관된 에러 핸들링 전략을 고민해보세요.
🤖 Prompt for AI Agents
In
app/src/main/java/woowacourse/shopping/data/datasource/remote/CartDataSourceImpl.kt
around lines 78 to 81, the code throws an IllegalStateException with an empty
message when response.toIdOrNull() returns null, which is unhelpful for
debugging and inconsistent with error handling. Instead of throwing an
exception, modify the code to use Result.failure with a meaningful error message
or exception to represent the failure case, ensuring consistent and clear error
handling within the onResult callback.
override fun onFailure( | ||
call: Call<CartsResponse>, | ||
t: Throwable, | ||
) { | ||
onResult(emptyList()) | ||
} | ||
}, |
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.
🛠️ Refactor suggestion
실패 케이스에서 빈 리스트를 반환하는 것이 적절할까요?
getPagedCartItems
의 onFailure
에서:
onResult(emptyList())
이 접근 방식에 대해 생각해보세요:
- 네트워크 오류와 데이터 없음을 구분할 수 있을까요?
- 호출하는 쪽에서 오류 상황을 어떻게 알 수 있을까요?
- 다른 메서드들과 일관성이 있나요?
사용자 경험 관점에서 어떤 방식이 더 나을지 고민해보세요.
🤖 Prompt for AI Agents
In
app/src/main/java/woowacourse/shopping/data/datasource/remote/CartDataSourceImpl.kt
around lines 57 to 63, the onFailure callback currently returns an empty list,
which does not distinguish between a network error and an empty data scenario.
To fix this, modify the onFailure method to propagate the error information to
the caller, such as by passing an error callback or wrapping the result in a
type that can represent both success and failure states. Ensure this approach
aligns with other methods in the codebase and improves error handling and user
experience by clearly signaling failure cases.
.fallback(R.drawable.ic_delete) | ||
.error(R.drawable.ic_delete) |
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.
🛠️ Refactor suggestion
이미지 로딩 실패 시 사용자 경험을 개선해보세요.
현재 fallback
과 error
모두에 ic_delete
아이콘을 사용하고 있습니다. 사용자 입장에서 생각해보면 어떨까요?
- 이미지 로딩이 실패했을 때 삭제 아이콘을 보면 어떤 느낌일까요?
fallback
(URL이 null인 경우)과error
(로딩 실패)를 구분해서 표시하는 것이 더 명확하지 않을까요?
Glide 공식 문서에서 placeholder, fallback, error의 차이점을 찾아보시고, 각각에 더 적절한 drawable을 고려해보세요.
🤖 Prompt for AI Agents
In
app/src/main/java/woowacourse/shopping/presentation/bindingadapter/ImageViewBindingAdapter.kt
around lines 16 to 17, the same drawable ic_delete is used for both fallback and
error cases, which can confuse users. Update the code to use distinct drawables:
assign a neutral placeholder or fallback image for when the URL is null, and a
clear error image for when loading fails. Refer to Glide's documentation to
choose appropriate drawables for placeholder, fallback, and error states to
improve user experience.
object DatabaseModule { | ||
private lateinit var appContext: Context | ||
private var database: CartDatabase? = null | ||
|
||
fun init(context: Context) { | ||
appContext = context.applicationContext | ||
} | ||
|
||
private fun provideDatabase(): CartDatabase = CartDatabase.getInstance(appContext).also { database = it } | ||
|
||
fun provideCartDao(): CartDao = provideDatabase().cartDao() | ||
|
||
fun provideRecentProductDao(): RecentlyProductDao = provideDatabase().recentlyProductDao() | ||
} |
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.
🛠️ Refactor suggestion
의존성 주입 모듈의 안전성 개선 고려사항
현재 구현에서 몇 가지 잠재적 위험요소들이 있습니다. 다음 사항들을 검토해보세요:
lateinit var appContext
의 초기화 시점이 보장되나요?database
필드의 스레드 안전성은 어떻게 보장할 수 있을까요?provideDatabase()
가 여러 스레드에서 동시 호출되면 어떤 문제가 발생할 수 있을까요?
고민해볼 점들:
@Volatile
어노테이션이나synchronized
블록의 필요성- Hilt나 Dagger 같은 현대적 DI 프레임워크 도입 가능성
- Application Context 누수 방지를 위한 WeakReference 사용 고려
어떤 방식으로 이러한 안전성 문제들을 해결할 수 있을지 생각해보세요.
🤖 Prompt for AI Agents
In app/src/main/java/woowacourse/shopping/di/DatabaseModule.kt around lines 8 to
21, the current use of lateinit var appContext risks uninitialized access, and
the database instance is not thread-safe, which can cause multiple instances or
race conditions. To fix this, ensure appContext is initialized before any
access, make the database variable @Volatile, and synchronize the
provideDatabase() method to guarantee a single instance across threads. Consider
replacing manual DI with a modern framework like Hilt or Dagger for safer
dependency management and use applicationContext directly to avoid context
leaks.
interface ProductDataSource { | ||
fun fetchPagingProducts( | ||
page: Int?, | ||
pageSize: Int?, | ||
category: String?, | ||
onResult: (List<Product>) -> Unit, | ||
) | ||
|
||
fun fetchProductById( | ||
id: Long, | ||
onResult: (Product) -> 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.
에러 처리 메커니즘 개선 필요
데이터 소스 인터페이스에서 중요한 부분이 빠져있습니다.
현재 문제점:
- 네트워크 실패, 서버 에러, 파싱 실패 등의 상황을 어떻게 처리할 건가요?
onResult
콜백이 성공 케이스만 고려하고 있는데, 실패 시나리오는 어떻게 처리할까요?
고민해볼 점들:
RecentProductRepository
에서는Result<T>
타입을 사용하는데, 여기서는 왜 다른 패턴을 사용했나요?- 네트워크 호출이 실패했을 때 UI는 어떤 상태를 보여줘야 할까요?
- 데이터 계층 전반에서 일관된 에러 처리 패턴을 유지하려면 어떻게 해야 할까요?
개선 방향:
- 다른 레이어와 일관된 에러 처리 패턴 도입을 고려해보세요
- 네트워크 계층의 견고성을 높이기 위한 방법들을 찾아보세요
🤖 Prompt for AI Agents
In
app/src/main/java/woowacourse/shopping/data/datasource/remote/ProductDataSource.kt
around lines 5 to 17, the current interface methods only handle successful
results via onResult callbacks and lack error handling for network failures,
server errors, or parsing issues. To fix this, modify the method signatures to
use a unified Result<T> type or a similar wrapper that encapsulates both success
and failure cases, ensuring consistent error handling across layers. Update the
callbacks to accept a Result<List<Product>> or Result<Product> respectively, so
that calling code can handle errors and successes uniformly, improving
robustness and UI state management.
@coderabbitai pause |
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.
1,2 단계 구현하시느라 고생많으셨습니다!
전반적으로 깔끔하게 구현해주셨는데,
이번 미션에서 챙겨볼 수 있는 부분들에 대해 코멘트를 남겨드렸어요.
미처 반영하지 못했던 부분도 있다면 함께 반영해주세요!
app/build.gradle.kts
Outdated
val localProperties = | ||
Properties().apply { | ||
load(File(rootDir, "local.properties").inputStream()) | ||
} | ||
val id = localProperties["id"] as String | ||
val pw = localProperties["password"] as String | ||
|
||
buildConfigField("String", "ID", "\"$id\"") | ||
buildConfigField("String", "PASSWORD", "\"$pw\"") |
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.
BuildConfig 활용 좋습니다 👍
plugins { | ||
alias(libs.plugins.android.application) | ||
alias(libs.plugins.android.junit5) | ||
alias(libs.plugins.kotlin.android) | ||
id("kotlin-kapt") | ||
alias(libs.plugins.serialization) |
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.
선택한 JSON 직렬화 라이브러리 : kotlinx.serialization
선택한 이유 : 코틀린언어에서 제공하고 있어 신뢰할 수 있다고 생각했고 리플렉션을 사용하지 않고 직렬화, 역직렬화를 지원하기에 성능상 이점과 안전성이 조금이라도 더 보장될 것 같다는 생각으로 선택하게 되었습니다.
Gson, Moshi 와 같은 라이브러리들과 어떤 차이점이 있을까요?
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.
kotlinx.serialization
은 코틀린 언어 기반이고 default value를 직렬화, 역직렬화에 활용할 수 있습니다.
그리고 코틀린 언어의 null type 안전성도 보장해주기에 코틀린 언어를 효율적으로 사용할 수 있습니다.
Gson
, Moshi
는 자바 기반 라이브러리이고 Gson
의 경우 default value를 인식하지 못하며 Moshi
의 경우 KotlinJsonAdapterFactory
를 추가해야 default value를 인식할 수 있습니다.
값이 없는 경우 반환 받을 타입이 not-null type일지라도 참조형 타입의 경우 null, 기본형 타입의 경우 기본값으로 반환하게 되어 코틀린 언어의 null type 안전성을 해치게 되는 경우가 발생하게 됩니다.
또한, kotlinx.serialization
과는 달리 리플렉션을 활용하여 직렬화, 역직렬화를 구현했다는 차이점이있습니다.
Gson
과 Moshi
둘을 비교한다면 Moshi
가 Gson
에 비해 코틀린 친화적이고 성능과 버전 업데이트 현황에서 이점이 있다고 할 수 있을 것 같습니다.
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.
좋습니다 👍
default value가 중요한 키워드가 될 수 있으니
리플렉션, non-null 과 같은 키워드들을 중점적으로 기억해두시면 도움이 될거에요
// fun toDomain(data: CartEntity): CartItem { | ||
// productDataSource.fetchProductById(data.productId){ result -> | ||
// result. | ||
// } | ||
// return CartItem(product, data.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.
이건 사용하지 않는 코드인걸까요?
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.
private val cartItemService: CartItemService, | ||
) : CartDataSource { | ||
override fun getTotalCount(onResult: (Result<Int>) -> Unit) = | ||
cartItemService.requestCartItemCount().enqueue( |
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.
execute와 enqueue 중 왜 enqueue를 사용하셨고,
어떤 차이가 있을까요?
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.
- execute()
- 동기 방식으로 현재 작업을 마친 후 다음 작업을 이어나가게 됩니다.
- 안드로이드에서는 메인 스레드에서 I/O 작업을 할 경우 예외를 발생하기에 백그라운드 스레드를 사용하도록 해야합니다.
- enqueue()
- Retrofit 내부에서 새로운 스레드에 요청을 처리하고, 메인 스레드는 차단되지 않습니다.
- UI 관련된 작업을 처리하기에 적합하지만 값을 직접 받을 수 없어 콜백구조로 값을 활용해야 합니다.
Retrofit이 thread도 관리해주고 성공, 실패 분기도 제공해주기에 enqueue를 선택했습니다.
call: Call<CartsResponse>, | ||
t: Throwable, | ||
) { | ||
onResult(emptyList()) |
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.
요거는 코드래빗 코멘트를 유심히 살펴볼만한거 같아요.
실패와 empty는 구분되어야 할 상황이 분명 있을거에요.
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.
실패를 empty(), null로 판별하는 것이 아닌 Result.failure로 성공, 실패 여부를 명확히 인식할 수 있도록 수정하였습니다.
관련 커밋 : 07ce825
private val _uiState: MutableLiveData<ResultState<Unit>> = MutableLiveData() | ||
val uiState: LiveData<ResultState<Unit>> = _uiState | ||
private val _cartItems: MutableLiveData<List<CartItemUiModel>> = MutableLiveData() | ||
val cartItems: LiveData<List<CartItemUiModel>> = _cartItems | ||
private val _selectedTotalPrice: MutableLiveData<Int> = MutableLiveData(0) | ||
val selectedTotalPrice: LiveData<Int> = _selectedTotalPrice | ||
private val _selectedTotalCount: MutableLiveData<Int> = MutableLiveData(0) | ||
val selectedTotalCount: LiveData<Int> = _selectedTotalCount | ||
private val _isCheckAll: MutableLiveData<Boolean> = MutableLiveData(false) | ||
val isCheckAll: LiveData<Boolean> = _isCheckAll |
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.
특정 값 변화에 따라 연쇄적으로 데이터 처리가 필요하다면
LiveData의 Map 또는 MediatorLiveData를 활용할 수 있을까요?
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.
연쇄적으로 데이터 처리하는 방법이 있을 줄 몰랐는데, 좋은 정보 감사합니다 !
MediatorLiveData
를 적용해보고 싶어 선택했습니다. 학습하며 Map
을 활용하는 방식도 적용해보도록 하겠습니다 !
처음 적용해본 부분이라 적절하게 사용되었는지 궁금합니다 !
관련 커밋 : 2439382
@@ -32,16 +34,21 @@ class ProductViewModel( | |||
private var currentPage = FIRST_PAGE | |||
|
|||
init { | |||
fetchData() | |||
fetchCartItemCount() | |||
(cartRepository as CartRepositoryImpl).fetchAllCartItems { |
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.
여기서 캐스팅은 어떤 이유로 필요할까요?
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.
캐싱데이터를 관리하는 CartRepositoryImpl
에서만 캐싱을 위한 메서드인 fetchAllCartItems
를 구현했습니다.
따라서 CartRepository
인터페이스에는 해당 메서드가 없었기에 캐스팅이 필요했었습니다.
현재 서버에서 제공하는 API들을 사용해 상품 상세페이지에서 장바구니 ID를 얻기 위해서는 캐싱을 활용하는 방향으로 가야 될 것 같다고 생각해 CartRepository
인터페이스에서 구현을 강제하도록 수정하였습니다. 캐스팅이 필요없도록 수정되었습니다!
관련 커밋 : c7d6ae2
fun addToCart(cartItem: CartItem) { | ||
cartRepository.insertProduct(cartItem) { result -> | ||
cartRepository.insertProduct(cartItem.product, 1) { result -> | ||
result | ||
.onSuccess { | ||
updateQuantity(productId = cartItem.product.productId, 1) |
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.
여러 View
에서 사용되는 상품 수량 증가, 감소 그리고 장바구니 담기 행위를 UseCase
로 만들었습니다.
처음 적용해본 내용이라 적절하게 구현했는지 제이든의 의견을 남겨주시면 감사하겠습니다 !
이렇게 사용할 경우 UseCase
가 많이 늘어날 것 같다고 생각이 듭니다.
그럴 때는 공통으로 사용되는 여러 UseCase
를 묶는 UseCases
를 만들어서 사용하면 될까요 ?? 🤔
ViewModel
마다 사용되는 UseCase
들이 다양할 것 같고 생성자에 많은 UseCase
들이 생겨날 것 같습니다.
관련 커밋 : ef42285
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.
ItemClickListener 인터페이스를 구현한 부분에서,
viewModel에서 수행되는 작업들이라면 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.
또한, 상품상세 진입 후 다시 상품목록으로 돌아올 때 스크롤 포지션이 초기화 되는데 어떤 이유에서 발생하는걸까요?
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.
ItemClickListener 인터페이스를 구현한 부분에서, viewModel에서 수행되는 작업들이라면 ViewModel에서 리스너를 구현하면 어떨까요?
viewModel에서 작업을 수행하도록 수정하였습니다 !
관련 커밋 : 07c800d
또한, 상품상세 진입 후 다시 상품목록으로 돌아올 때 스크롤 포지션이 초기화 되는데 어떤 이유에서 발생하는걸까요?
onStart()
생명주기에서 fetchData()
를 새로운 데이터를 불러오며 발생한 문제인 것 같습니다.
포함된 데이터라면 LiveData를 업데이트하지 않도록 수정하였지만 해당 문제가 여전히 남아있어 조금만 더 고민해보겠습니다.
제이든과 리뷰를 여러 번 주고 받고 싶어 완벽히 해내지 못했음에도 제출한 점 죄송합니다. 다음 리뷰에는 꼭 반영하겠습니다 !
fetchData()
가 상세페이지를 다녀오면 항상 0 페이지부터 받아오는 부분은 수정하였습니다.
관련 커밋 : 0c5b01e
### 2단계 기능 목록 | ||
|
||
- [x] 장바구니 화면에서 특정 상품만 골라 주문하기 버튼을 누를 수 있다. |
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.
상품목록에서 한가지 궁금한게 있어요.
이미지가 같은 상품에 대해서,
그 중 하나의 상품 개수를 증가시켰을 때, 이미지가 같은 상품들이 함께 깜빡이는 이슈가 있습니다.
어떤 이유이고 어떻게 개선할 수 있을까요?
screencapture-1748594987448.mp4
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.
이미지 정보가 없거나 문제가 있는 상품일 경우 Glide 설정에서 모두 동일한 drawable을 넣었습니다.
구현 당시에는 notifyDataSetChanged() 를 호출하며 전체를 갱신하다보니 동일한 이미지 소스에 애니메이션이 들어갔지 않나 하는 추측을 해보았습니다.
이 리뷰를 보기 전에 ListAdapter 로 수정해버려 위와 같은 문제가 발생하지 않아 정확한 원인을 찾지는 못했습니다. 🥲
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.
피드백 반영하시느라 고생 많으셨습니다!
남겨드린 코멘트는 다음 단계에서 함께 반영해주시면 됩니다.
이제 마지막 단계인만큼 조금만 더 파이팅해서 완주하시기를 응원할게요 👍
plugins { | ||
alias(libs.plugins.android.application) | ||
alias(libs.plugins.android.junit5) | ||
alias(libs.plugins.kotlin.android) | ||
id("kotlin-kapt") | ||
alias(libs.plugins.serialization) |
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.
좋습니다 👍
default value가 중요한 키워드가 될 수 있으니
리플렉션, non-null 과 같은 키워드들을 중점적으로 기억해두시면 도움이 될거에요
override fun onStart() { | ||
super.onStart() | ||
viewModel.loadItems() |
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.
onStart에서 해당 로직을 수행할 경우 Activity Lifecycle 관점에서 어떤 영향을 받을 수 있을지 깊게 고민해보시길 바랄게요.
예) configuration changed
장바구니 - 추가 상품 추천 페이지(RecommendActivity)에서 상품을 추가하고나서 뒤로가기 할 경우 장바구니 - 선택창 페이지(CartActivity)에 담긴 상품이 존재해야 된다고 생각했습니다.
뒤로 가기 할 경우 onRestart() 부터 시작된다는 이유로 배치했던 것 같습니다.
이러한 상황에서는 수업시간에 다뤘던 activity result api 를 활용하는건 어떨까요?
private val _selectedTotalPrice: MediatorLiveData<Int> = | ||
MediatorLiveData<Int>().apply { | ||
addSource(_cartItems) { cartItems -> | ||
value = cartItems?.filter { it.isSelected }?.sumOf { it.totalPrice } ?: 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.
MediatorLivedata 활용 좋습니다 👍
다만, 단일 옵저빙일 경우 map 으로도 리소스를 한정시키는 방법도 있으니 참고해주세요!
|
||
import woowacourse.shopping.domain.repository.CartRepository | ||
|
||
class DecreaseProductQuantityUseCase( |
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.
이렇게 사용할 경우 UseCase가 많이 늘어날 것 같다고 생각이 듭니다.
그럴 때는 공통으로 사용되는 여러 UseCase를 묶는 UseCases를 만들어서 사용하면 될까요 ?? 🤔
UseCase에 대한 정의를 공부해보면 그러한 상황은 거의 마주하지 않을거에요.
참고
다만, 공통의 UseCase를 묶어야 한다면 여러 UseCase를 연산하여 하나의 UseCase를 생성하는 로직도 구현이 가능은 하겠네요!
셀프 체크리스트
README.md
에 기능 목록을 정리하고 명확히 기술하였는가?어떤 부분에 집중하여 리뷰해야 할까요?
제가 어떤 부분을 못 하고 있는지 조차 모르겠어서 보면서 아닌 것 같은 부분 모두 리뷰해주시면 감사하겠습니다 !
코드 리뷰 커뮤니케이션
안녕하세요, 제이든
안드로이드 마지막 쇼핑 주문 미션 리뷰이 채넛입니다 🙇♂️
레벨 1에 이어서 2에서도 리뷰이로 만나게 되어 반갑습니다 :) 이번에도 잘 부탁드립니다 !!
이번 미션은 페어인 미플의 코드로 진행하게 되었습니다 !
제가 아직 안드로이드에 능숙하지 못해서 고쳐야 할 부분이 많을 것 같습니다.
부족한 코드여서 읽기 힘드시겠지만 리뷰 잘 부탁드립니다 ! 감사합니다 :-)
선택한 JSON 직렬화 라이브러리 : kotlinx.serialization
선택한 이유 : 코틀린언어에서 제공하고 있어 신뢰할 수 있다고 생각했고 리플렉션을 사용하지 않고 직렬화, 역직렬화를 지원하기에 성능상 이점과 안전성이 조금이라도 더 보장될 것 같다는 생각으로 선택하게 되었습니다.
이번 미션 커밋만 모아놓은 링크입니다
참고 자료
스크린샷
record.mp4