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

[Feature] 정보 수정 비밀번호 확인 다이얼로그 #315

Merged
merged 13 commits into from
Aug 9, 2024

Conversation

nodobi
Copy link
Contributor

@nodobi nodobi commented Jul 9, 2024

개요

작업 상세내용

  • 클릭 이벤트와 같은 여러 이벤트를 처리하기 위해 EventFlow 를 추가했습니다 관련 자료
  • 서버에서 내려주는 에러 메세지를 출력하기 위해 ErrorResponse 을 추가하고 확장 메소드를 통해 가져올 수 있도록 하였습니다.

결과 화면

@nodobi nodobi requested a review from a team as a code owner July 9, 2024 08:22
@nodobi nodobi self-assigned this Jul 9, 2024
Copy link
Contributor

@yunjaena yunjaena left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 :) 커멘트 확인해주세요!

override fun onDestroyView() {
super.onDestroyView()

binding.tietPassword.removeTextChangedListener(passwordWatcher)
Copy link
Contributor

Choose a reason for hiding this comment

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

리소스를 해제하는 경우는 super 이전에 하는게 좋아요.
ex) onPause, onStop, onDestroy

Copy link
Contributor Author

@nodobi nodobi Jul 13, 2024

Choose a reason for hiding this comment

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

수정했습니다 감사합니다!
혹시 super 이후에 해제하게 되는 경우 어떤 문제가 발생하는지 알 수 있을까요?

상위 뷰가 먼저 제거되면서 발생하는 여러 문제가 있는거라고 이해했습니당
https://stackoverflow.com/questions/37760417/removing-event-listener-as-activity-pauses-or-stops

Copy link
Contributor

Choose a reason for hiding this comment

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

@nodobi 잘 찾아봤네요 :) 부모 class에서 해제를 할 수도 있기 떄문에 없는 null이거나 이미 리소스가 해제된 것을 해제할 수도 있기 떄문에 조심해야되요!

interface EventFlow<out T> : Flow<T> {
companion object {

const val DEFAULT_REPLAY: Int = 3
Copy link
Contributor

Choose a reason for hiding this comment

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

블로그 글 참고해서 구현한것 같은데 이벤트가 3개 이상 넘어오면 어떻게 핸들하나요?

Copy link
Contributor Author

@nodobi nodobi Jul 13, 2024

Choose a reason for hiding this comment

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

이벤트가 넘어오는 시점에 따라서 핸들되는 순서가 다소 달라질 것 같습니다.

  • EventFlow 를 collect 하고 있는 시점
    • 관련 작업 시행
  • collect 하고 있지 않은 시점 (onStop)
    • 최대 3개의 이벤트에 대해 SharedFlow 의 버퍼에 저장
    • 이후 collect 가 다시 되는 경우 consume 되지 않은 이벤트에 대해 (onStart) 관련 작업 시행

Copy link
Contributor

Choose a reason for hiding this comment

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

이벤트가 N개 이상 내려올때 이벤트가 유실될 수 있는게 아닐까 싶어서요. 정답은 없지만 해당 부분도 한번 고려 해보면 좋을거 같네요 :)

Copy link
Contributor

@yunjaena yunjaena left a comment

Choose a reason for hiding this comment

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

LGTM 수고하셨습니다!

Copy link
Contributor

@ThirFir ThirFir left a comment

Choose a reason for hiding this comment

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

수고하셨습니다 !

@nodobi nodobi merged commit c30a532 into develop Aug 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants