Feature/jaino/#14 #21
Conversation
| suspend fun hasPendingResult(): Boolean | ||
|
|
||
| suspend fun signIn(email: String): Result<String> | ||
| } |
There was a problem hiding this comment.
함수명 좋네요.
signIn함수는 email만 있으면 되나요 ?.?
There was a problem hiding this comment.
AuthDataSourceImpl에서 보실 수 있는데, Activity Context를 사용해, Github Login 웹으로 넘어가 비밀번호를 입력하는 것 같아요!
아마 보안상 이슈때문에, 이런것 같아요
| @Binds | ||
| @Singleton | ||
| abstract fun provideAuthDataSource( | ||
| authDataSourceImpl: AuthDataSourceImpl | ||
| ): AuthDataSource | ||
| } |
There was a problem hiding this comment.
오호라 Provides가 아니라 Binds를 쓰시네요.
혹시 Binds 쓰시는 이유가 따로 있나요 ?
Binds는 외부 라이브러리를 지원못하는 걸로 알고있어서요.
Provides쓰면 다 해결되자나요 하하
There was a problem hiding this comment.
하하하하하 습관적으로 Binds 처리해버렸군요... 생각해보니까 협업할 때 Provides랑 Binds를 같이쓸지도 정했어야겠네요...
사실 아시겠지만 Provides랑 Binds랑 정말 뜨거운 감자입니다... 구글에 조금만쳐도 댓글에 열뛴 토론하는거 볼 수 있어요 ㅎㅎ..
Dagger에서도 이런거 알고 FAQ에서 명시해놨어요. 근데 사람들도 이거 알고 싸우는 거라 하하 ..
https://dagger.dev/dev-guide/faq.html#why-is-binds-different-from-provides
서로 따지자면,
Binds -> DataSoruce같이 Interface 매핑 전용 어노테이션입니다. 그래서 Provides에 비해 생성되는 파일과 크기가 작고 효율적이에요.
근데 보시다시피, 단일 매개변수 포함밖에 안돼요. 그래서 나머지를 다 Provides 시켜야 해요. AuthDataSourceImpl에서 보실수 있는데, 실제 매개변수가 ActivityContext, FirebaseAuth가 들어있어요. 근데 주입하는 코드는 비어있죠. 그래서 사실 많이 직관적이지 않아요.
그래서 Provides vs Binds 나뉘는게
P : Binds가 인터페이스 전용 매퍼로 엄청 효율적이야!!!
C : 그러면 뭐해, 알아보지 못하잖아. 매개변수 이거 하나하나 다 까볼꺼야??
이렇게 나뉘는듯 해요.. 하하하
| @Module | ||
| @InstallIn(SingletonComponent::class) | ||
| object NetworkModule { | ||
| @Provides | ||
| @Singleton | ||
| fun providesFirebaseAuth(): FirebaseAuth = Firebase.auth | ||
|
|
||
| @Provides | ||
| @Singleton | ||
| fun providesFirestore(): FirebaseFirestore = Firebase.firestore | ||
| } |
| activityContext, provider.build() | ||
| ).await() | ||
|
|
||
| checkNotNull(result.user).uid |
There was a problem hiding this comment.
혹시 .uid 부분 설명 가능할까요? 처음보는 키워드라서요.
There was a problem hiding this comment.
이부분 클린 코드에서 지양하는 기차충돌인것 같아요.
저도 처음 저 코드를 보면 바로 이해가 안될 것 같네요 ㅠㅠ
바로 코드 수정했습니다!!
해당 코드는 FirebaseAuth 메소드를 사용하면 AuthResult라는 값을 반환하는데, 해당 AuthResult에서 FirebaseUser에 접근하는 코드입니다.
FirebaseUser를 통해 회원 정보를 접근하고 해당 유저의 uid, FirebaseAuth를 사용하면 자동으로 userId 식별자를 생성해주는데, 해당 식별자를 반환하는 코드입니다!
로그인 절차가 Github Login을 통해 Firebase Auth를 인증하고, uid를 반환받으면 해당 uid를 가지고 유저 테이블에 uid가 있는지 없는지로 첫회원인지 확인하기 때문에, uid를 반환받도록 했습니다!
나중에 회원가입 로직구현할 때, 받은 uid를 기반으로 테이블 검색 메소드까지 구현할 예정입니당!
| override suspend fun resign(): Result<Unit> { | ||
| TODO("Not yet implemented") | ||
| } |
There was a problem hiding this comment.
이 함수는 어떤 역할을 할 함수인가요 ?.?
제가 영어를 잘 못해서....
There was a problem hiding this comment.
하하 회원탈퇴 메소드 네이밍인데, 후보군이
withDraw, resign, cancel, leave 정도 있어서
위에 sign이 중복되는 resign으로 결정했는데, 혹시 다른게 더 나을까요??
| suspend fun <T> Task<T>.await(): T { | ||
| return suspendCancellableCoroutine { cont -> | ||
| addOnCompleteListener { | ||
| if (it.exception != null) { | ||
| cont.resumeWithException(it.exception!!) | ||
| } else { | ||
| cont.resume(it.result, null) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
이 부분도 처음봐요 ..
혹시 설명이나 아니면 링크 참조 가능할까요?
There was a problem hiding this comment.
firebase 로직들이 전부 Listener를 통해 Callback을 받도록 되어 있는데, 해당 콜백을 코루틴으로 변경시키는 코드입니다!
코루틴으로 변경시키면서, 다른 코루틴 코드랑 혼용도 할 수 있고,
무엇보다 여러 스코프가 겹치면서 가독성이 떨어지는 이슈도 대응할 수 있어요!
해당 글에 suspendCoroutine에 대해 잘 나와있어요!
https://tourspace.tistory.com/442?category=797357
https://devroach.tistory.com/162
tgyuuAn
added
🌟머지 해주세요🌟
|
리뷰이거 진짜 좋네요... 리뷰 달면서 제 코드 다시 보고, 문제점 바로 찾고 더 좋은 코드로 작성할 수 있는 거같아서 아주 좋습니다!! |
1. 📄 관련된 이슈 및 소개
#14 Firebase Github 로그인 로직 구현
2. 🔥변경된 점
FirebaseAuth를 활용한 Github 로그인 로직 구현
3. 📸 스크린샷(선택)
4. 💡알게된 혹은 궁금한 사항들
린팅 버그 확인 -> (린팅 룰, 버그) #20 수정 예정입니다...