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

Week4 #8

Open
wants to merge 9 commits into
base: develop
Choose a base branch
from
Open

Week4 #8

wants to merge 9 commits into from

Conversation

tunaunnie
Copy link
Contributor

Related issue πŸ› 

  • closed #μ΄μŠˆλ„˜λ²„

Work Description ✏️

  • MVVM ꡬ쑰 적용
  • νšŒμ›κ°€μž… μ‹œ 이메일 μž…λ ₯ -> username μž…λ ₯ λ°©μ‹μœΌλ‘œ μˆ˜μ •
  • 둜그인 μ‹œ 이메일 μž…λ ₯ -> username μž…λ ₯ λ°©μ‹μœΌλ‘œ μˆ˜μ •
  • νšŒμ›κ°€μž… API μ—°κ²°(CreateUser): μ„±κ³΅μ μœΌλ‘œ νšŒμ›κ°€μž… μ‹œ μœ μ € 정보λ₯Ό 담을 ResponseCreateUserSuccessDto μ •μ˜
  • 둜그인 API μ—°κ²°(GetUser): μ„±κ³΅μ μœΌλ‘œ 둜그인 μ‹œ μœ μ € 정보&토큰 담을 ResponseGetUserSuccessDto μ •μ˜
  • 둜그인 ν›„ μ‚¬μš©μž 이름 μ—°κ²°: SignUp success μ‹œ μ „μ—­λ³€μˆ˜ UserViewModel에 username κ°’ μ—…λ°μ΄νŠΈ
  • μ‚¬μš©μž μ·¨λ―Έ λΆˆλŸ¬μ˜€λŠ” API μ—°κ²° (GetUserHobby): 둜그인 ν›„, μœ μ €μ˜ token을 μ „λ‹¬ν•˜μ—¬(RequestUserHobbyDto) μœ μ €μ˜ μ·¨λ―Έλ₯Ό κ°€μ Έμ˜€λŠ” ResponseUserHobbySuccessDto μ •μ˜

Screenshot πŸ“Έ

Uncompleted Tasks πŸ˜…

  • Postman μ‘°μž‘ λ―Έμˆ™μœΌλ‘œ μ˜ˆμ‹œ 데이터가 μžˆμ„ λ•Œ μ •λ§λ‘œ 값이 κ°€μ Έμ™€μ§€λŠ”μ§€ ν…ŒμŠ€νŠΈν•˜μ§€ λͺ»ν–ˆμŠ΅λ‹ˆλ‹€.. λΌˆλŒ€λ§Œ λ§Œλ“€μ–΄λ‘” μƒνƒœμ΄κ³  λ™μž‘μ€ 아직 λ˜μ§€ μ•ŠλŠ” κΈ°λŠ₯듀이 μžˆμŠ΅λ‹ˆλ‹€γ… 
  • success/failed일 λ•Œ DTOκ°€ λ‹€λ₯Έλ°, 이 DTO듀을 ν¬ν•¨ν•˜λŠ” 큰 DTO (wrapperDTO)λ₯Ό ν™œμš©ν•˜λŠ” 게 λ§žλŠ”μ§€λ„ 잘 λͺ¨λ₯΄κ² μŠ΅λ‹ˆλ‹€. ... 일단은 μ „λΆ€ 두 DTOλ₯Ό ν¬ν•¨ν•œ wrapperDTOλ₯Ό λ°˜ν™˜ν•˜λ„λ‘ ν–ˆλŠ”λ°, λ§žλŠ” 방식인지 κ³ λ―Ό 쀑에 μžˆμŠ΅λ‹ˆλ‹€

To Reviewers πŸ“’

@tunaunnie
Copy link
Contributor Author

γ…œγ…œ μ΄λ²ˆμ—λ„ 과제 μˆ˜ν–‰λ„κ°€ 많이 λ―Έν‘ν•˜λ„€μš”... μ°¨μ£Ό λ‚΄λ‘œ μ†”μ§ν•œ λ°˜μ„± & λΉ λ₯Έ 볡ꡬ에 λ“€μ–΄κ°€κ² μŠ΅λ‹ˆλ‹€πŸ₯Ή

Copy link

@t1nm1ksun t1nm1ksun left a comment

Choose a reason for hiding this comment

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

과제 브랜치 νŒŒμ‹€ λ•Œ 이전 μ£Όμ°¨ 브랜치 develop에 λ¨Έμ§€ν•˜κ³  developμ—μ„œ νŒŒμ£Όμ„Έμš”..!!
λŒ€λΆ€λΆ„μ˜ file듀이 Files changed둜 μΈμ‹λ©λ‹ˆλ‹€ γ… γ… 

Choose a reason for hiding this comment

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

DTOμž‘μ„±μ΄ 맀우 μžμ„Έν•˜λ„€μš”πŸ‘ 래퍼클래슀λ₯Ό 넣은 μ΄μœ κ°€ μžˆλ‚˜μš”? κΆκΈˆν•΄μ„œ!!!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

μ„œλ²„ μš”μ²­μ΄ μ„±κ³΅ν•œ κ²½μš°μ™€ μ‹€νŒ¨ν•œ κ²½μš°μ— μ‚¬μš©ν•˜λŠ” DTOκ°€ λΆ„λ¦¬λ˜μ–΄ μžˆλŠ”λ°, 응닡 νƒ€μž…μ€ 늘 ν•˜λ‚˜μ—¬μ•Ό ν•˜λ‹ˆ.. λΆ„κΈ° 처리λ₯Ό ν•˜λŠ” 것보단 WrapperDTO μ•ˆμ— failedDTOλž‘ successDTOλ₯Ό ν•¨κ»˜ 넣어두고 늘 WrapperDTOλ₯Ό λ°˜ν™˜ν•˜λŠ” 방식이 λ§žλ‹€κ³  μƒκ°ν–ˆμ–΄μš”! 그런데 ν•©μ„Έ λ•Œ μž¬λ―Όλ‹˜κ»˜ κΌ­ κ·Έλ ‡κ²Œ ν•˜μ§€ μ•Šμ•„λ„ λœλ‹€λŠ” 말씀을 λ“€μ–΄μ„œ (그리고 이게 μ§€κΈˆ μ—λŸ¬μ˜ 원인 κ°™μ•„μ„œ πŸ˜… 방식을 μˆ˜μ •ν•΄λ³Ό μ˜ˆμ •μž…λ‹ˆλ‹Ή)

import org.sopt.and.model.dto.signup.RequestCreateUserDto
import org.sopt.and.model.dto.signup.ResponseCreateUserSuccessDto
import org.sopt.and.model.dto.signup.ResponseCreateUserWrapperDto
import retrofit2.Call

Choose a reason for hiding this comment

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

저도 μ΄λ²ˆμ— λ ˆνŠΈλ‘œν•2λ₯Ό κ³΅λΆ€ν•˜λ©΄μ„œ Call vs Response에 λŒ€ν•΄μ„œ μ°Ύμ•„λ΄€μ–΄μš”.
차이점을 ν•œλ²ˆ μ°Ύμ•„λ³΄μ‹œκ³  λ‚˜μ€‘μ— 코루틴 μ μš©μ— Responseκ°€ 더 이점이 μžˆλ‹€κ³  ν•˜λ‹ˆ λ°”κΏ”λ³΄μ‹œλŠ”κ²ƒλ„ 쒋을 것 κ°™μ•„μš”!

Log.e(TAG, "μœ μ € 둜그인 μ—λŸ¬ ${response.code()}")
}
}

Choose a reason for hiding this comment

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

둜그인 μ‹€νŒ¨μ— λŒ€ν•΄ 더 μžμ„Ένžˆ μ—λŸ¬μ²˜λ¦¬κ°€ 있으면 쒋을 것 κ°™μ•„μš”

}
}

fun PasswordValidCheck(password: String): Boolean {

Choose a reason for hiding this comment

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

μ΄μ œλŠ” 둜그인 λͺ…μ„Έλ₯Ό λ³΄μ‹œλ©΄ 8자 μ΄ν•˜λ©΄ 되기 λ•Œλ¬Έμ— μ—†μ• μ€˜λ„ 될 것 κ°™μ•„μš”!

Copy link

@angryPodo angryPodo left a comment

Choose a reason for hiding this comment

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

μ΄λ²ˆμ— μ„Έλ―Έλ‚˜ 참석 λͺ»ν•΄μ„œ μ΄ν•΄ν•˜λŠ”λ° 어렀움이 μžˆμ—ˆμ„ν…λ° μž˜ν•˜μ…¨μ–΄μš”. ν•©μ„Έ μ§„ν–‰ν•˜λ©΄μ„œ 이번 리뷰에 달린 λ‚΄μš©λ“€μ΄λ‚˜ λ‘œμ§λΆ„λ¦¬μ™€ 같은 것듀 λ‹€μ‹œν•œλ²ˆ μ μš©ν•˜λ©΄ ν™• 와닿을 것 κ°™μ•„μš”! λΉ μ΄νŒ…!!

Copy link

@chattymin chattymin 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 +67 to +71
var userNameState = loginViewModel.userNameState.collectAsState().value
var passwordState = loginViewModel.passwordState.collectAsState().value
var isUserNameValid = loginViewModel.isUserNameValid.collectAsState().value
var isPasswordValid = loginViewModel.isPasswordValid.collectAsState().value
var shouldShowPassword = loginViewModel.shouldShowPassword.collectAsState().value

Choose a reason for hiding this comment

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

collectAsStateμ—λŠ” 단점이 ν•˜λ‚˜ μžˆμŠ΅λ‹ˆλ‹€.
μ •ν™•νžˆλŠ” flow의 단점이라고 ν•  수 μžˆκ² λ„€μš”.

liveData와 flow의 차이점을 κ³΅λΆ€ν•΄λ³΄μ‹œκ³ , collectAsStateWithLifecycle을 μ‚¬μš©ν•˜μ‹œλ©΄ 쒋을 것 κ°™μŠ΅λ‹ˆλ‹€ :)

Comment on lines +19 to +20
//ν˜„μž¬ λ‘œκ·ΈμΈν•œ ν† ν°μœΌλ‘œ ν˜„ μ‚¬μš©μžμ˜ μ·¨λ―Έλ₯Ό 뢈러였기 μœ„ν•¨
private val userService by lazy { ServicePool.userService }

Choose a reason for hiding this comment

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

μ΄λ ‡κ²Œ μ„ μ–Έν•˜λŠ” 것과 viewModel의 인자둜 μ„ μ–Έν•˜λŠ” κ²ƒμ˜ 차이점을 μ°Ύμ•„λ³΄μ‹œλ©΄ 쒋을 것 κ°™μ•„μš”

Copy link
Contributor

@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.

μž‘μ—… μ „ 이전 μ£Όμ°¨ 과제 PRλ“€ λ¨Έμ§€ν•˜κ³  μ§„ν–‰ν•΄μ£Όμ‹œκ³ , μ‹œμ—° μ˜μƒ ν•„μˆ˜λ‘œ μΆ”κ°€ν•΄μ£Όμ„Έμš”.
과제 μ œμΆœν•˜μ‹€ λ•Œ λ…Έμ…˜μ— μžˆλŠ” 과제 제좜 방식 μ „λΆ€ 따라주셔야 ν•˜κ³  κ³Όμ œμ— μžˆλŠ” κΈ°λŠ₯ λͺ…μ„Έ μ „λΆ€ κ΅¬ν˜„ν•΄μ£Όμ…”μ•Ό ν•©λ‹ˆλ‹€.
ν•„μˆ˜ κ³Όμ œλŠ” 정말 ν•„μˆ˜μ μΈ 뢀뢄이고 μ œλŒ€λ‘œ μ§„ν–‰λ˜μ§€ μ•ŠμœΌλ©΄ μΆ”ν›„ μ•±μžΌ λ“± ν˜‘μ—… 진행이 μ–΄λ €μš°λ‹ˆ μ‹œκ°„ λ‚˜μ‹€ λ•Œ κΌ­ μˆ˜μ • λΆ€νƒλ“œλ¦΄κ²Œμš”.
ν•„μˆ˜ κ³Όμ œκ°€ μ œλŒ€λ‘œ μ§„ν–‰λ˜μ§€ μ•Šμ„ μ‹œ μ•±μžΌ μ°Έμ—¬κ°€ μ–΄λ €μšΈ 수 μžˆμŠ΅λ‹ˆλ‹€.

Comment on lines +6 to +12
@Serializable
data class ResponseGetUserWrapperDto(
@SerialName("success")
val success: ResponseGetUserSuccessDto,
@SerialName("failed")
val failed: ResponseGetUserFailDto
)
Copy link
Contributor

Choose a reason for hiding this comment

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

이거 이런 μ‹μœΌλ‘œ λ§Œλ“€μ–΄λ„ 톡신 잘 이루어 μ§€λ‚˜μš”?
μ„±κ³΅μ˜ κ²½μš°μ—λŠ” success에 λŒ€ν•œ κ²ƒλ§Œ μ‹€νŒ¨μ˜ κ²½μš°μ—λŠ” failed 만 내렀보내주어 직렬화가 잘 μ•ˆ 될 것 같은데 잘 λ™μž‘ν•˜λ‚˜μš”?


@GET("/user/my-hobby")
fun getMyHobby(
@Header("token") request: RequestGetUserHobbyDto
Copy link
Contributor

Choose a reason for hiding this comment

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

μ΄λ ‡κ²Œ 넣지 μ•Šκ³  인터셉터λ₯Ό 톡해 λ„£λŠ” 방식도 μžˆμœΌλ‹ˆ λ‚˜μ€‘μ— κ³΅λΆ€ν•΄λ³΄μ‹œλ©΄ 쒋을 것 κ°™μ•„μš”!

}
}

override fun onFailure(call: Call<ResponseGetUserWrapperDto>, t: Throwable) {
Copy link
Contributor

Choose a reason for hiding this comment

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

onFailure λΈ”λŸ­μ€ μ–΄λ–€ κ²½μš°μ— λ“€μ–΄μ˜€κ²Œ λ κΉŒμš”?

Comment on lines +47 to +51
@Serializable
data class LoginScreen(
val userNameText: String,
val passwordText: String
)
Copy link
Contributor

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
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants