Step4 - 페이먼츠(카드 수정) #53

oyj7677 · 2024-09-02T08:26:49Z

RegistrationUiState를 만들어서 신규 카드 등록과 수정을 구분했고 상태 호이스팅 된 onSaveClick에 대해서만 분기하면 된다고 생각하여 구현했습니다.

oldCard 데이터의 경우 특별하게 호출될 일이 없어 viewModel에서 보관하고 있는 형태로 구현했습니다.

현재는 RegistrationUiState의 값을 viewModel에서 관리하고 있지만,

uiState 노출 관심사 리뷰를 잘 이해했는지는 모르겠지만 만약 관심사가 RegistrationUiState에 존재할 한다면

viewModel애서 oldCard를 가져와 아래
sealed class RegistrationUiState의 companion object에 선언된 아래의 함수를 사용해도 될까요?

getUiState(card: Card?): RegistrationUiState {
    if(card == null) {
        ....
   } else { 
        ....
    }
}

아직 UiState에 대한 이해가 부족한 것 같습니다. ㅜㅜ

1. 로고 프로퍼티 @DrawableRes 추가 2. modifier 프로퍼티 사용

1. brandColor: Color -> bankType: BankType 교체 2. selectedBankType를 NewCardViewModel에서 관리 3. addCard시 NewCardViewModel 내부의 값으로 사용함

1. 재사용 가능 로직 캡슐화

1. CardLazyColumn 컴포넌트 대신 직접 PaymentCard 컴포넌트 호출

1. Text 컴포넌트를 사용하여 카드 회사명 노출

1. 3단계 기능 목록 체크

1. Card데이터 클래스에 Parcelize 활용하여 데이터 전송 2. 등록된 카드 선택시 카드 등록 화면으로 이동

1. 등록된 카드 선택시, 카드 수정 화면으로 이동 기능 체크

1. 한 장 이상의 카드 화면에 클릭이벤트 등록

1. 등록된 카드 구분을 위해 Card 데이터 클래스에 id속성 추가 2. 카드 수정 및 Id생성 함수 구현(CardRepository) 3. 등록된 카드 데이터가 newCardActivity로 전달될 상황에 대한 로직 구현 4. 카드 등록 UI상태 값 추가 및 관리

1. 카드 데이터 변경사항 유효성 검사 로직 추가

1. 구현된 기능 목록 체크 - 카드 수정 시 변경이 일어나지 않으면 수정이 불가능하다. - 카드가 수정되면 카드 목록 화면에 변경사항이 반영된다.

wisemuji

안녕하세요 영재님!

페이먼츠 미션 마지막 단계 구현을 잘 해주셨습니다 💯 깔끔한 구현이 좋았습니다.
마지막 단계인 만큼 몇 가지 고민해보시면 좋을만한 코멘트를 달았으니 확인해주시면 감사하겠습니다.

궁금한 점이나 논의하고 싶은 내용이 있으면 언제든 DM 주세요! 🙏 💪

wisemuji · 2024-09-02T13:08:25Z

app/build.gradle.kts

@@ -68,4 +69,5 @@ dependencies {
    androidTestImplementation(libs.androidx.ui.test.junit4)
    debugImplementation(libs.androidx.ui.tooling)
    debugImplementation(libs.androidx.ui.test.manifest)
+    implementation(kotlin("script-runtime"))


혹시 어떤 이유로 추가된걸까요~?

wisemuji · 2024-09-02T14:01:46Z

app/src/main/java/nextstep/payments/data/PaymentCardsRepository.kt

+    fun editCard(oldCard: Card?, newCard: Card) {
+        val index = _cards.indexOfFirst { it.id == oldCard!!.id }
+        _cards[index] = newCard
+    }
+
+    fun createId(): Int {
+        return _cards.maxOfOrNull { it.id }?.plus(1) ?: 1
+    }


도메인에 너무 많은 신경을 쓰지 않고 학습 목표에 집중한 구현을 해주셨네요!

(선택사항) 그래도 혹시 구조를 개선하고 싶으시다면 다음 변경사항을 만들어 유지보수성을 높여보시는건 어떨까요?

oldCard의 id가 이미 newCard에 포함되어 있어서 oldCard를 받지 않아도 될 것 같아요.

외부에 createId 함수가 공개될 필요가 있을까요?

이외에도 프로젝트에서 !!를 불필요하게 활용하고 있다면 null safety하게 구현해볼 수 있을 것 같아요.

마지막 단계이니까 편하게 도전해보시죠!

wisemuji · 2024-09-02T14:03:47Z

app/src/main/java/nextstep/payments/ui/PaymentCard.kt

    modifier: Modifier = Modifier,
-    brandColor: Color = Color(0xFF333333)
+    content: @Composable () -> Unit = {}


의도대로 리뷰를 넘 잘 반영해주셨습니다 👍
이전에 비해 훨씬 컴포넌트 하나하나의 로직을 캡슐화되도록 설계되었어요.

wisemuji · 2024-09-02T14:04:39Z

app/src/main/java/nextstep/payments/ui/PaymentCard.kt

 @Composable
-fun PaymentCard(
+fun PaymentCardContents(
    card: Card,
-    modifier: Modifier = Modifier,
-    content: @Composable () -> Unit = {}
+    onClick: (Card) -> Unit = {}
 ) {
    Box(
-        modifier = Modifier.padding(top = 10.dp)
+        modifier = Modifier
+            .clickable { onClick(card) }


혹시 modifier 파라미터는 왜 사라졌을까요?

사용되지 않아서 지웠었는데 Modifier규칙과 관련된 글을 보니 항상 받아야한다고 하네요.
이전에 파라미터로 전달된 modifier를 내부 속성에 포함시켜 의도치 않는 상황을 만들었습니다.
modifier를 항상 넘겨야한다는 건 컴포넌트의 가장 외부 속성에는 항상 적용시켜줘야한다는 건가요?
이전에 설명해주신 modifier - Don't 사례입니다. 제가 말한 외부 속성이란 Box()를 뜻합니다.

wisemuji · 2024-09-02T14:14:11Z

app/src/main/java/nextstep/payments/ui/card/registration/NewCardActivity.kt

+        (intent?.getParcelableExtra("card") as Card?)?.let {
+            viewModel.setOldCard(it)
+            viewModel.setUiState(RegistrationUiState.EditCard)
+        }


ViewModel SavedStateHandle을 적용해보시면 어떨까요?

카드 목록과 카드 등록 로직에 SavedStateHandle을 추가하여 라이프사이클 이슈에 대응하였습니다.

wisemuji · 2024-09-02T14:17:28Z

app/src/main/java/nextstep/payments/ui/card/registration/NewCardScreen.kt

+    val saveFunction: KFunction0<Unit> = when (uiState) {
+        RegistrationUiState.NewCard -> {
+            viewModel::addCard
+        }
+
+        RegistrationUiState.EditCard -> {
+            viewModel::editCard
+        }
+    }


이건 UI에서 판단하지 않고 ViewModel에서 판단하게 두면 어떨까요?
현재의 ViewModel은 이미 수정/추가 화면인지를 구분할 수 있습니다. 🙂

wisemuji · 2024-09-02T14:18:06Z

app/src/main/java/nextstep/payments/ui/card/registration/RegistrationUiState.kt

+sealed interface RegistrationUiState {
+    data object NewCard : RegistrationUiState
+    data object EditCard : RegistrationUiState
+}


현재는 RegistrationUiState의 값을 viewModel에서 관리하고 있지만,
만약 관심사가 RegistrationUiState에 존재할 한다면
viewModel애서 oldCard를 가져와 아래
sealed class RegistrationUiState의 companion object에 선언된 아래의 함수를 사용해도 될까요?
getUiState(card: Card?): RegistrationUiState {
if(card == null) {
....
} else {
....
}
}

권장 앱 아키텍처에서 설명하는 UI 상태는 말 그대로 UI에 노출되어야 하는 상태를 의미합니다. 현재의 RegistrationUiState은 UI에 노출되어야 하는 상태를 갖고 있기보단, 이 화면이 카드 수정 화면인지 추가 화면인지 분기하는 용도로 사용되고 있는 것 같아요.

이번 미션에서는 권장 앱 아키텍처가 중요한 학습 목표가 아니므로(4주차 미션에서 더 경험하게 되실겁니다) UI 상태에 대해 깊게 피드백드리진 않겠습니다. 영재님의 선택에 맡길게요! 혹시 이 주제로 좀 더 구조를 개선시키고 싶으시다면 알려주세요. 그에 맞게 더 피드백드리겠습니다.

1. 불필요한 의존성 삭제 2. 테스트 코드 수정 3. PaymentCardContents 컴포넌트 파라미터에 modifier추가

1. UiState에 따른 카드 등록 분기 로직 이동 (NewCardScreen -> NewCardViewModel) 2. 관련 함수 수정

1. editCard() oldCard 파라미터 제거, oldCard의 id 불필요 2. Card의 유니크값인 id 디폴트값 0으로 변경 3. Card추가 시 Card의 id값이 0이라면 신규 Id 부여, 해당 로직을 저장소에 처리함. 4. let을 사용하여 !! 표시 제거

1. SavedStateHandle를 활용하여 라이프사이클에 따른 데이터 소멸 방지 2. 카드 목록, 카드 등록 데이터 보존

1. 카드 저장 uiState 상태 저장 로직 변경 2. 카드 데이터 저장 및 열기 네이밍 변경

wisemuji

안녕하세요 영재님!

리뷰 반영을 잘 해주셨습니다 🙂
마지막으로 고민해보시면 좋을만한 코멘트를 달았으니 확인해주시면 감사하겠습니다.
이 코멘트만 반영되면 다음 리뷰 요청에서는 미션을 완료할 수 있을 것 같아요.

궁금한 점이나 논의하고 싶은 내용이 있으면 언제든 DM 주세요! 🙏 💪

wisemuji · 2024-09-20T05:41:26Z

app/src/main/java/nextstep/payments/ui/card/registration/NewCardViewModel.kt

+    fun openCardData() {
+        savedStateHandle.get<String>(KEY_CARD_NUMBER)?.let {
+            _cardNumber.value = it
+        }
+        savedStateHandle.get<String>(KEY_EXPIRED)?.let {
+            _expiredDate.value = it
+        }
+
+        savedStateHandle.get<String>(KEY_OWNER_NAME)?.let {
+            _ownerName.value = it
+        }
+
+        savedStateHandle.get<String>(KEY_PASSWORD)?.let {
+            _password.value = it
+        }
+
+        savedStateHandle.get<BankType>(KEY_BANK_TYPE)?.let {
+            _selectedBankType.value = it
+        }
+
+        savedStateHandle.get<RegistrationUiState>(KEY_UI_STATE)?.let {
+            _uiState.value = it
+        }
+        savedStateHandle.get<Card>(KEY_OLD_CARD)?.let {
+            oldCard = it
+        }
+    }
+
+    fun saveCardData() {
+        savedStateHandle[KEY_CARD_NUMBER] = cardNumber.value
+        savedStateHandle[KEY_EXPIRED] = expiredDate.value
+        savedStateHandle[KEY_OWNER_NAME] = ownerName.value
+        savedStateHandle[KEY_PASSWORD] = password.value
+        savedStateHandle[KEY_BANK_TYPE] = selectedBankType.value
+        savedStateHandle[KEY_UI_STATE] = uiState.value
+        savedStateHandle[KEY_OLD_CARD] = oldCard
+    }


원본 코멘트의 의도와 다르게 반영된 것 같아요 😅 제가 설명이 부족했었네요!

원본 코멘트에서는 아래 블럭에 단 리뷰였는데요,

override fun onCreate(savedInstanceState: Bundle?) { val viewModel by viewModels<NewCardViewModel>() super.onCreate(savedInstanceState) (intent?.getParcelableExtra("card") as Card?)?.let { viewModel.setOldCard(it) viewModel.setUiState(RegistrationUiState.EditCard) }

Activity에서 별도로 전달받은 intent를 ViewModel에서 활용할 수 있어서 별도로 초기화 코드를 작성하지 않고도 ViewModel에 전달된 값을 활용할 수 있습니다.

class NewCardViewModel( rivate val repository: PaymentCardsRepository = PaymentCardsRepository, private val savedStateHandle: SavedStateHandle ) : ViewModel() { // ... init { savedStateHandle.get<Card>("card")?.run { // } }

다음 글이 도움되길 바랍니다.
https://pluu.github.io/blog/android/2020/02/20/savedstatehandle/

wisemuji · 2024-09-20T05:42:21Z

app/src/main/java/nextstep/payments/ui/card/registration/NewCardActivity.kt

+    override fun onResume() {
+        super.onResume()
+        viewModel.openCardData()
+    }
+
+    override fun onPause() {
+        super.onPause()
+        viewModel.saveCardData()
+    }


Activity와 AAC ViewModel의 LifeCycle은 별도로 굴러가기 때문에 이러한 코드는 작성하지 않으셔도 됩니다. 🙂

oyj7677 added 13 commits August 31, 2024 13:56

Refactor : 로고 및 수식자 속성으로 구성요소 유연성 향상

9ef8565

1. 로고 프로퍼티 @DrawableRes 추가 2. modifier 프로퍼티 사용

Refactor : Card 데이트 클래스 프로퍼티 변경 및 카드 등록 로직 변경

2343f5c

1. brandColor: Color -> bankType: BankType 교체 2. selectedBankType를 NewCardViewModel에서 관리 3. addCard시 NewCardViewModel 내부의 값으로 사용함

Refactor : 카드 이미지 컴포넌트 수정

3cbdfb5

1. 재사용 가능 로직 캡슐화

Refactor : 단일 카드 노출 컴포넌트 수정

af3e538

1. CardLazyColumn 컴포넌트 대신 직접 PaymentCard 컴포넌트 호출

Refactor : 등록 카드 회사명 추가

366b597

1. Text 컴포넌트를 사용하여 카드 회사명 노출

Chore : README.md 수정

9a10420

1. 3단계 기능 목록 체크

Chore : Step4 (카드수정) 기능 목록 추가

9e3eaa0

Feat : 등록 카드 선택 시 카드 등록 화면으로 이동.

d319fbb

1. Card데이터 클래스에 Parcelize 활용하여 데이터 전송 2. 등록된 카드 선택시 카드 등록 화면으로 이동

Chore : README.md 수정

c577a00

1. 등록된 카드 선택시, 카드 수정 화면으로 이동 기능 체크

Fix : 한장 이상의 카드 화면에서 카드 선택시 화면 이동 안하던 이슈 해결

73c0d77

1. 한 장 이상의 카드 화면에 클릭이벤트 등록

Refactor : 등록된 카드 수정 기능 리펙토링

9fcbb3d

1. 카드 데이터 변경사항 유효성 검사 로직 추가

Chore : README.me 수정

8a81524

1. 구현된 기능 목록 체크 - 카드 수정 시 변경이 일어나지 않으면 수정이 불가능하다. - 카드가 수정되면 카드 목록 화면에 변경사항이 반영된다.

wisemuji requested changes Sep 2, 2024

View reviewed changes

oyj7677 added 5 commits September 10, 2024 18:08

Chore : 리뷰 반영

aa82010

1. 불필요한 의존성 삭제 2. 테스트 코드 수정 3. PaymentCardContents 컴포넌트 파라미터에 modifier추가

Refactor : 카드 저장 로직 수정

1b91e7f

1. UiState에 따른 카드 등록 분기 로직 이동 (NewCardScreen -> NewCardViewModel) 2. 관련 함수 수정

Refactor : 카드 목록, 카드 등록 데이터 보존 기능 추가

4a39882

1. SavedStateHandle를 활용하여 라이프사이클에 따른 데이터 소멸 방지 2. 카드 목록, 카드 등록 데이터 보존

Refactor : 카드 저장 uiState 상태 저장 로직 변경 및 네이밍 변경

656f36a

1. 카드 저장 uiState 상태 저장 로직 변경 2. 카드 데이터 저장 및 열기 네이밍 변경

wisemuji requested changes Sep 20, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Step4 - 페이먼츠(카드 수정) #53

Step4 - 페이먼츠(카드 수정) #53

oyj7677 commented Sep 2, 2024

wisemuji left a comment

wisemuji Sep 2, 2024

wisemuji Sep 2, 2024

wisemuji Sep 2, 2024

wisemuji Sep 2, 2024

oyj7677 Sep 10, 2024

wisemuji Sep 2, 2024

oyj7677 Sep 16, 2024

wisemuji Sep 2, 2024

wisemuji Sep 2, 2024

wisemuji left a comment

wisemuji Sep 20, 2024

wisemuji Sep 20, 2024

Step4 - 페이먼츠(카드 수정) #53

Are you sure you want to change the base?

Step4 - 페이먼츠(카드 수정) #53

Conversation

oyj7677 commented Sep 2, 2024

wisemuji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wisemuji left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment