Skip to content
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

[Step3] 메모 (Update, Delete) #12

Merged
merged 6 commits into from Sep 6, 2021
Merged

Conversation

malibinYun
Copy link
Member

이전 단계에서 이것 저것 많이 구현해두었더니 금방 끝난 기분입니다.
피드백 주신대로 mockk() 라이브러리를 활용하니 코드도 줄어들고 더 깔끔하게 테스트가 표현되네요. 이걸 체감하니 모킹 라이브러리는 이제 꼭 사용해야겠다는 생각이 드네요 ☺

이번 리뷰도 잘 부탁드립니다.

Copy link
Collaborator

@galcyurio galcyurio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3단계 기능 구현 고생하셨습니다!
한 가지 코멘트를 남겨두었으니 확인해주세요.
다음 단계도 힘내세요!

Comment on lines +24 to +29
fun deleteMemo(memoId: String) {
memosRepository.deleteMemo(memoId)
val currentMemos = _memos.value.orEmpty()
val memo = currentMemos.find { it.id == memoId } ?: return
_memos.value = currentMemos - memo
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

실무에서 ViewModel 부분이 많이 복잡해진다면 List<Memo>를 일급 컬렉션으로 분리하는 방법을 고려해보면 좋겠어요.
실제로 저는 많이 사용하는 방법이기도 합니다.

data class Memos(val values: List<Memo>) {
    operator fun minus(memoId: String): Memos {
        val newValues = values.filterNot { it.id == memoId }
        return Memos(newValues)
    }

    companion object {
        val EMPTY = Memos(emptyList())
    }
}
class MainViewModel(
    private val memosRepository: MemosSource,
) : ViewModel() {
    private val _memos = MutableLiveData(Memos.EMPTY)
    val memos: LiveData<Memos> = _memos

    fun loadAllMemos() {
        _memos.value = Memos(memosRepository.getAllMemos())
    }

    fun deleteMemo(memoId: String) {
        memosRepository.deleteMemo(memoId)
        _memos.value = _memos.value?.minus(memoId)
    }
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LiveData는 코드가 뭔가 아쉬운데 StateFlow를 사용하면 공식적으로 제공하는 API를 통해 아래와 같이 써줄 수 있습니다.

_memos.update { it - memoId }

이렇게 하면 update() 함수가 내부적으로 compareAndSet()을 하기 때문에 동시성 이슈도 해결할 수 있습니다.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 일급 컬렉션을 생각을 못하고있었네요!
viewmodel의 저 부분은 저도 짜면서 맘에 들지 않았던 부분이었는데, 일급컬렉션을 사용하면 굉장히 깔끔해지네요 😮

Comment on lines +25 to +26
memosLocalSource = mockk(relaxed = true)
memosRepository = MemosRepository(memosLocalSource)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

MockK를 활용하여 테스트 코드 간결하게 유지 👍

@galcyurio galcyurio merged commit 2415a44 into next-step:malibinyun Sep 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants