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

[7주차/필수] Repository 활용_xml #24

Open
wants to merge 10 commits into
base: develop-xml
Choose a base branch
from

Conversation

gaeulzzang
Copy link
Collaborator

Related issue 🛠

Work Description ✏️

  • repository로 로직 분리하기

Screenshot 📸

Screen_recording_20240605_144422.mp4

To Reviewers 📢

이렇게 하는게 맞는지 잘 모르겠어요...

Copy link
Member

@yskim6772 yskim6772 left a comment

Choose a reason for hiding this comment

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

깔끔하네요 수고하셨습니다 ~~

Comment on lines 11 to 19
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)
}
}
Copy link
Member

Choose a reason for hiding this comment

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

repository 패턴을 사용할 때 repository와 repositoryimpl로 나눠서 작성하셨군요 ! 메서드 정의와 구현을 의존성 없이 나눠서 표현할 수 있어서 좋은 것 같아요 !


interface FollowerRepository {
suspend fun getUserInfo(userId: Int): Response<ResponseUserInfoDto>
suspend fun changeUserPwd(userId: Int, request: RequestChangePwdDto) : Response<ResponseChangePwdDto>
Copy link
Member

Choose a reason for hiding this comment

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

follower repository 부분에 비밀번호 변경 메서드까지 ! 파일 개수가 깔끔하네용

Comment on lines +6 to +10
class BaseViewModelFactory<T>(private val creator: () -> T) : ViewModelProvider.Factory {
override fun <V : ViewModel> create(modelClass: Class<V>): V {
return creator() as V
}
}
Copy link
Member

Choose a reason for hiding this comment

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

viewmodelfactory를 한번에 제네릭하게 표현해서 좋네요 ! 저는 뷰모델 로직마다 분리했는데 합쳐야겠어요

Comment on lines 23 to 25
}.onSuccess {
if (it.code() in 200..299) {
_signUpState.value = SignUpState(true, "회원가입 성공")
Copy link
Member

Choose a reason for hiding this comment

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

제 코드랑 signUp 함수랑 AuthService 구조도 같아서, viewmodel에서 onSuccess 부분에 code 200번대 검사하는 if문은 작성 안해도 될 것 같은데 ..! 혹시 아직도 잘못 입력했을 때 앱이 터지는 문제가 발생해서 그런건가용?!

Copy link

@jihyunniiii jihyunniiii left a comment

Choose a reason for hiding this comment

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

고생하셨습니다

Choose a reason for hiding this comment

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

패키지 분리까지 야무지게 ㄷ.ㄷ

Comment on lines +6 to +10
class BaseViewModelFactory<T>(private val creator: () -> T) : ViewModelProvider.Factory {
override fun <V : ViewModel> create(modelClass: Class<V>): V {
return creator() as V
}
}

Choose a reason for hiding this comment

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

오왕 이런 식으로 뷰모델 팩토리를 작성해주셨군요!!

Copy link

@leeseokchan00 leeseokchan00 left a comment

Choose a reason for hiding this comment

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

레이어 분리까지 너무 고생했습니다!

Comment on lines 13 to 14
private val userService: UserService,
private val friendService: FriendService

Choose a reason for hiding this comment

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

요거 hilt로 바꿔주면 더 편하게 의존성 분리가 가능해질 것 같아요

Copy link
Member

@Eonji-sw Eonji-sw left a comment

Choose a reason for hiding this comment

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

리팩토링 잘하셨네욤! 쉽지 않았을텐데 완전 굿!!! 👍👍👍

super.onCreate()
appContext = applicationContext

AppCompatDelegate.setDefaultNightMode(AppCompatDelegate.MODE_NIGHT_NO)
Copy link
Member

Choose a reason for hiding this comment

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

함수로 분리하면 더 깔끔할 것 같아요

Copy link
Member

Choose a reason for hiding this comment

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

di 폴더는 밖으로 빼면 좋을 것 같아요! 또한 앱잼에서 따를 구조를 생각해서 data에서 remote와 local로 구분해보는 것도 좋을 것 같네욤

Copy link
Member

Choose a reason for hiding this comment

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

url 상수화하면 관리하기 편하고 좋을 것 같네요

Copy link
Member

Choose a reason for hiding this comment

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

@Serializable@SerialName는 데이터를 직렬화/역직렬화할 때 사용해요. 데이터를 JSON 등과 같은 형식으로 변환하거나, 반대로 그 형식에서 Kotlin 객체로 변환할 때 사용합니당. 따라서 서버 데이터를 dto로 받기 때문에 entity에서는 해당 어노테이션들이 불필요해요. 때문에 제거하고 사용하면 좋을 것 같아요~

Comment on lines 17 to 19
private val _changePwdState = MutableLiveData<ChangePwdState>()
val changePwdState: LiveData<ChangePwdState>
get() = _changePwdState
Copy link
Member

Choose a reason for hiding this comment

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

flow에 대해서도 공부해보고 적용해보면 좋을 것 같네요! 쉽게 비동기를 다룰 수 있고 코루틴에 대해서도 더 깊게 공부해보실 수 있을거예요

Copy link
Member

Choose a reason for hiding this comment

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

Dagger Hilt를 사용하여 의존성을 자동으로 주입하는 경우 수동으로 팩토리를 작성할 필요가 없어져요. @HiltViewModel 어노테이션을 사용하여 ViewModel을 정의하고, @Inject를 통해 의존성을 주입할 수 있어요. Hilt는 ViewModel을 관리하고, Activity나 Fragment에서 by viewModels()를 통해 간편하게 ViewModel을 주입받을 수 있게 해줘요!

Comment on lines +28 to +33
@Provides
fun provideAuthDataSource(
authService: AuthService
): AuthDataSource {
return AuthDataSourceImpl(authService)
}
Copy link
Member

Choose a reason for hiding this comment

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

@Provides 어노테이션의 경우 객체 생성을 직접 관리해요. 저는 추가적인 유연성과 테스트에 용이하기 위해 @Binds와 interface를 사용하는 편이예요. 복잡도에 따라 적절하게 판단하여 활용하면 좋을 것 같네요!

Comment on lines +17 to +19
override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> {
return authService.signUp(request)
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> {
return authService.signUp(request)
}
override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> =
authService.signUp(request)

이렇게 리팩토링 할 수 있겠네요!

Comment on lines +17 to +19
override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> {
return authDataSource.signUp(request)
}
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> {
return authDataSource.signUp(request)
}
override suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> =
authDataSource.signUp(request)

여기도 리팩 가눙!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[7주차/필수] Repository 활용_xml
5 participants