Conversation
| class AuthRepositoryImpl(private val authService: AuthService) : AuthRepository { | ||
| override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> { | ||
| return authService.signUp(request) | ||
| } | ||
|
|
||
| override suspend fun login(request: RequestLoginDto): Response<ResponseLoginDto> { | ||
| return authService.login(request) | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
repository 패턴을 사용할 때 repository와 repositoryimpl로 나눠서 작성하셨군요 ! 메서드 정의와 구현을 의존성 없이 나눠서 표현할 수 있어서 좋은 것 같아요 !
|
|
||
| interface FollowerRepository { | ||
| suspend fun getUserInfo(userId: Int): Response<ResponseUserInfoDto> | ||
| suspend fun changeUserPwd(userId: Int, request: RequestChangePwdDto) : Response<ResponseChangePwdDto> |
There was a problem hiding this comment.
follower repository 부분에 비밀번호 변경 메서드까지 ! 파일 개수가 깔끔하네용
| class BaseViewModelFactory<T>(private val creator: () -> T) : ViewModelProvider.Factory { | ||
| override fun <V : ViewModel> create(modelClass: Class<V>): V { | ||
| return creator() as V | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
viewmodelfactory를 한번에 제네릭하게 표현해서 좋네요 ! 저는 뷰모델 로직마다 분리했는데 합쳐야겠어요
| }.onSuccess { | ||
| if (it.code() in 200..299) { | ||
| _signUpState.value = SignUpState(true, "회원가입 성공") |
There was a problem hiding this comment.
제 코드랑 signUp 함수랑 AuthService 구조도 같아서, viewmodel에서 onSuccess 부분에 code 200번대 검사하는 if문은 작성 안해도 될 것 같은데 ..! 혹시 아직도 잘못 입력했을 때 앱이 터지는 문제가 발생해서 그런건가용?!
| class BaseViewModelFactory<T>(private val creator: () -> T) : ViewModelProvider.Factory { | ||
| override fun <V : ViewModel> create(modelClass: Class<V>): V { | ||
| return creator() as V | ||
| } | ||
| } No newline at end of file |
| private val userService: UserService, | ||
| private val friendService: FriendService |
There was a problem hiding this comment.
요거 hilt로 바꿔주면 더 편하게 의존성 분리가 가능해질 것 같아요
Eonji-sw
left a comment
There was a problem hiding this comment.
리팩토링 잘하셨네욤! 쉽지 않았을텐데 완전 굿!!! 👍👍👍
| super.onCreate() | ||
| appContext = applicationContext | ||
|
|
||
| AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_NO) |
There was a problem hiding this comment.
di 폴더는 밖으로 빼면 좋을 것 같아요! 또한 앱잼에서 따를 구조를 생각해서 data에서 remote와 local로 구분해보는 것도 좋을 것 같네욤
There was a problem hiding this comment.
@Serializable와 @SerialName는 데이터를 직렬화/역직렬화할 때 사용해요. 데이터를 JSON 등과 같은 형식으로 변환하거나, 반대로 그 형식에서 Kotlin 객체로 변환할 때 사용합니당. 따라서 서버 데이터를 dto로 받기 때문에 entity에서는 해당 어노테이션들이 불필요해요. 때문에 제거하고 사용하면 좋을 것 같아요~
| private val _changePwdState = MutableLiveData<ChangePwdState>() | ||
| val changePwdState: LiveData<ChangePwdState> | ||
| get() = _changePwdState |
There was a problem hiding this comment.
flow에 대해서도 공부해보고 적용해보면 좋을 것 같네요! 쉽게 비동기를 다룰 수 있고 코루틴에 대해서도 더 깊게 공부해보실 수 있을거예요
There was a problem hiding this comment.
Dagger Hilt를 사용하여 의존성을 자동으로 주입하는 경우 수동으로 팩토리를 작성할 필요가 없어져요. @HiltViewModel 어노테이션을 사용하여 ViewModel을 정의하고, @Inject를 통해 의존성을 주입할 수 있어요. Hilt는 ViewModel을 관리하고, Activity나 Fragment에서 by viewModels()를 통해 간편하게 ViewModel을 주입받을 수 있게 해줘요!
| @Provides | ||
| fun provideAuthDataSource( | ||
| authService: AuthService | ||
| ): AuthDataSource { | ||
| return AuthDataSourceImpl(authService) | ||
| } |
There was a problem hiding this comment.
@Provides 어노테이션의 경우 객체 생성을 직접 관리해요. 저는 추가적인 유연성과 테스트에 용이하기 위해 @Binds와 interface를 사용하는 편이예요. 복잡도에 따라 적절하게 판단하여 활용하면 좋을 것 같네요!
| override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> { | ||
| return authService.signUp(request) | ||
| } |
There was a problem hiding this comment.
| override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> { | |
| return authService.signUp(request) | |
| } | |
| override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> = | |
| authService.signUp(request) |
이렇게 리팩토링 할 수 있겠네요!
| override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> { | ||
| return authDataSource.signUp(request) | ||
| } |
There was a problem hiding this comment.
| override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> { | |
| return authDataSource.signUp(request) | |
| } | |
| override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> = | |
| authDataSource.signUp(request) |
여기도 리팩 가눙!
Related issue 🛠
Work Description ✏️
Screenshot 📸
Screen_recording_20240605_144422.mp4
To Reviewers 📢
이렇게 하는게 맞는지 잘 모르겠어요...