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

[FEAT/#17] week7-compose / 필수과제 #19

Open
wants to merge 6 commits into
base: develop-compose
Choose a base branch
from

Conversation

MoonsuKang
Copy link
Member

📌issue

📄Work Description

  • Repository 패턴 적용
  • ViewModelFactory 사용
  • Coroutine으로 변경

📷Record

📢To Reviewers

  • 과제 미리미리 할껄..

@MoonsuKang MoonsuKang added ⭐ FEAT 새로운 기능 구현 📘필수 과제 필수 과제 🟦 COMPOSE 컴포즈 labels Jun 7, 2024
MoonsuKang added a commit that referenced this pull request Jun 7, 2024
Copy link
Member

@leeeyubin leeeyubin left a comment

Choose a reason for hiding this comment

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

굿굿!! 7주차 과제까지 너무 수고하셨습니당~!!!! 외쳐 갓문수

Comment on lines +7 to +15
class SignUpViewModelFactory(private val signUpRepository: SignUpRepository) : ViewModelProvider.Factory {
override fun <T : ViewModel> create(modelClass: Class<T>): T {
if (modelClass.isAssignableFrom(SignUpViewModel::class.java)) {
@Suppress("UNCHECKED_CAST")
return SignUpViewModel(signUpRepository) as T
}
throw IllegalArgumentException("알 수 없는 ViewModel 클래스")
}
}
Copy link
Member

@leeeyubin leeeyubin Jun 9, 2024

Choose a reason for hiding this comment

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

팩토리에서 이 부분이 반복되는 것 같은데 어떻게 반복을 없앨 수 있을지 생각해보면 좋을 것 같아요!

Comment on lines +20 to +23
}
}.getOrElse {
Result.failure(it)
}
Copy link
Member

Choose a reason for hiding this comment

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

getOrElse는 어떤 기능을 하나용..?

Comment on lines +20 to +24
val result = signUpRepository.signUp(signUpDto)
_signUpState.value = if (result.isSuccess) {
SignUpState.Success("회원가입 성공")
} else {
SignUpState.Error(result.exceptionOrNull()?.message ?: "회원가입 실패")
Copy link
Member

@leeeyubin leeeyubin Jun 9, 2024

Choose a reason for hiding this comment

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

Suggested change
val result = signUpRepository.signUp(signUpDto)
_signUpState.value = if (result.isSuccess) {
SignUpState.Success("회원가입 성공")
} else {
SignUpState.Error(result.exceptionOrNull()?.message ?: "회원가입 실패")
signUpRepository.signUp(
signUpDto
).onSuccess {
_signUpState.value = SignUpState.Success("회원가입 성공")
}.onFailure{
_signUpState.value = SignUpState.Error(result.exceptionOrNull()?.message ?: "회원가입 실패")
}

취향차이일 수는 있을 것 같은데 result를 따로 변수화하지 않고도 사용해줄 수 있을 것 같아요!

Copy link

@2hyunjinn 2hyunjinn left a comment

Choose a reason for hiding this comment

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

열심히 하셨군요!!!!

val response = chain.proceed(request)

// 응답 헤더에서 Location 값 추출해서 memberId에 저장 때리기

Choose a reason for hiding this comment

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

ㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋㅋ 때리기 삭제한 거 웃기네

import com.sopt.now.data.model.RequestLoginDto
import com.sopt.now.data.model.ResponseLoginDto

interface LoginRepository {

Choose a reason for hiding this comment

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

레포지토리 다 나눈 거 깔끔하네요-!!

is LoginState.Loading -> CircularProgressIndicator()
is LoginState.Success -> {
LaunchedEffect(state.message) {
Toast.makeText(context, state.message, Toast.LENGTH_LONG).show()
toast(context, state.message)

Choose a reason for hiding this comment

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

이렇게 쓰니까 훨씬 깔끔하네요! 배우고 갑니당

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⭐ FEAT 새로운 기능 구현 📘필수 과제 필수 과제 🟦 COMPOSE 컴포즈
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants