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.

Sign up for GitHub

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

Jump to bottom

#203 [refact] 로그인 리팩토링 #204

Closed
wants to merge 4 commits into from
Closed

#203 [refact] 로그인 리팩토링 #204

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
450b6cb
#203 [refact] 로그인 리팩토링
2zerozu Mar 4, 2023
c915356
#203 [add] 로그인 토큰 저장하는 로직 복구
2zerozu Apr 9, 2023
1e2f67a
#203 [fix] startKakaoLogin 함수로 빼기
2zerozu Apr 9, 2023
271c7ee
#203 [fix] 로그인 이후 인텐트 테스크 초기화하기
2zerozu Apr 9, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 6 additions & 2 deletions app/src/main/java/hous/release/android/presentation/login/LoginActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,11 +80,15 @@ class LoginActivity : BindingActivity<ActivityLoginBinding>(R.layout.activity_lo
}

private fun initKakaoLoginBtnClickListener() {
binding.btnLoginKakao.setOnSingleClickListener {
kakaoLoginService.startKakaoLogin(loginViewModel.kakaoLoginCallback)
binding.btnLoginKakao.setOnClickListener {
startKakaoLogin()
}
}

private fun startKakaoLogin() {
kakaoLoginService.startKakaoLogin(loginViewModel.kakaoLoginCallback)
}
Copy link
Member

Choose a reason for hiding this comment

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

음.. 이렇게 함수로 빼는 것이 아닌 지금 우리 프로젝트 구조를 보면

UI <--> Domain <--> Data
이런 구조로 유지 중입니다.

data 모듈에 있는 service를 ui 모듈에서 필드로 주입하는 것이
data 모듈을 ui 모듈과 분리해보는 것이 어떤가 합니다..!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

음.. 이렇게 함수로 빼는 것이 아닌 지금 우리 프로젝트 구조를 보면

UI <--> Domain <--> Data

이런 구조로 유지 중입니다.

data 모듈에 있는 service를 ui 모듈에서 필드로 주입하는 것이

data 모듈을 ui 모듈과 분리해보는 것이 어떤가 합니다..!

아으~ 확인이요

Copy link
Contributor

Choose a reason for hiding this comment

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

@KWY0218 @2zerozu
제가 알기로는 KakaoLoginService에서 Context가 필요한걸로 알고있는데 모듈 분리하면 data모듈에서 context 사용이 어려울 것 같은데 어떻게 생각하시나요?

Copy link
Member

Choose a reason for hiding this comment

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

@yjooooo

Kakao 로그인할 때 Activity Context가 필수적이니까 Kakao 로그인에 한에서 매개변수로 context를 전달하면 어떨까 합니다..!

Activity --(context)--> viewmodel --(context)--> domain --(context)--> data

context가 이런식으로 이동하겠네요..!

이렇게 생각한 이유는 나중에 feature 모듈로 분리를 하면 app 모듈과 떨어지게 되고, 해당 feature 모듈의 종속엔 data 모듈이 포함되지 않기 때문에 KakaoService를 주입 받을 때 문제가 생길 수 있다고 생각 들기 때문입니다..!

Copy link
Contributor

Choose a reason for hiding this comment

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

@KWY0218 @2zerozu @murjune
공식 문서 : [ViewModel은 뷰, Lifecycle 또는 활동 컨텍스트 참조를 포함하는 클래스를 참조해서는 안 됩니다.]
만약 분리를 하게 된다면 위의 말에 따라서 ViewModel 내부에 context가 존재하게는 하지말고, 메소드의 매개변수로 넘겨야겠네요..

제가 망설였던 부분은 data모듈에 저희가 넘기는 변수들은 뷰의 정보를 담고 있지는 않은데(보통 데이터 그 자체의 값이니까, ex. 이름, 생일, 룸코드 등등), context는 뷰의 정보들이 담겨있는 부분인데 넘기는 게 맞나? 하는 생각이 들었어요.
혹시 이 부분에 대해서는 어떻게 생각하시나요?

더불어 context는 메모리릭이 발생할 수 있는 부분이기도 하니까 더 조심스러워지는 것 같아요...
ViewModel 내부에 선언하거나 주입받는게 아니라면 메모리릭 문제에서는 벗어날 수도 있지 않을까 생각이 드네요...! (확인은 필요한 것 같아요!)

다른 분들의 의견도 궁금합니다!

Copy link
Member

Choose a reason for hiding this comment

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

KakaoTalk_20230516_134740933.mp4

현재 제가 하고 있는 졸작에서
Activity --(context)--> viewmodel --(context)--> domain --(context)--> data
이런 형식으로 카카오 로그인을 구현한 상태입니다.

로그인을 누르고 카카오 토큰을 받기 전 배경화면으로 갔을 때 leak canary에서 응답이 없는 걸 보니
메모리 누수가 없는 것 같아 보이긴 하는데

이 상황이 근거가 될진 확신이 안섭니다.. 😂😂

Copy link
Contributor

@yjooooo yjooooo May 18, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@KWY0218 카카오 로그인 이후 다음 화면으로 넘어갈 때도 릭카나리에서 응답이 없다면 메모리릭은 없는 것으로 봐도 되지 않을까요?
그러면 해당 구조로 가도 괜찮을 것 같아요!


private fun collectIsKakaoLogin() {
repeatOnStarted {
loginViewModel.isKakaoLogin.collect { success ->
Expand Down