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] WEEK6 - xml 필수과제 #11

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

Conversation

nagaeng
Copy link
Collaborator

@nagaeng nagaeng commented May 24, 2024

📌𝘐𝘴𝘴𝘶𝘦𝘴

📎𝘞𝘰𝘳𝘬 𝘋𝘦𝘴𝘤𝘳𝘪𝘱𝘵𝘪𝘰𝘯

  • ViewModel 사용
  • 명예 OB분께서 남겨주신 코드리뷰 반영

📷𝘚𝘤𝘳𝘦𝘦𝘯𝘴𝘩𝘰𝘵

시연 영상은 달라진 바가 없습니다 !
https://github.com/NOW-SOPT-ANDROID/nakyung-lee/assets/109855280/8ad91db6-1027-4aa4-992d-7b8780476f26

💬𝘛𝘰 𝘙𝘦𝘷𝘪𝘦𝘸𝘦𝘳𝘴

명예 OB분께서 너무 꼼꼼히 잘 봐주신 덕에 폴더 구조와 컨벤션 등등이 아주 아주 깔끔해졌습니다 !!
제가 일일이 댓글을 달면 알림이 갈까봐 . . 하트만 몇 개 눌렀는데요 .. 정말 감사합니다 많이 배웠습니다 !!

@nagaeng nagaeng self-assigned this May 24, 2024
@nagaeng nagaeng changed the base branch from main-xml to develop-xml May 24, 2024 14:40
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.

마지막까지 화이팅입니당~!!!!

Comment on lines +11 to +15

class LoginViewModel : ViewModel() {
private val authService by lazy { ServicePool.authService }
private val _loginStateLiveData = MutableLiveData<LoginState>()

Copy link
Member

Choose a reason for hiding this comment

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

뷰모델 따로 분리한 거 넘 좋아요!!!

Comment on lines +17 to +23
viewModelScope.launch {
try {
val response = authService.login(request)
_loginStateLiveData.value = LoginState(true, "로그인 성공")

} catch (e: HttpException) {
_loginStateLiveData.value = LoginState(false, "로그인 실패 ${e.code()}")
Copy link
Member

Choose a reason for hiding this comment

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

세미나에서 runCathcing에 대해서 배웠으니까 활용해보면 좋을 것 같네요!

Comment on lines +17 to +18
setContentView(binding.root)
SignupButtonClickListener()
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
setContentView(binding.root)
SignupButtonClickListener()
setContentView(binding.root)
signupButtonClickListener()

사소한 디테일!

Comment on lines +41 to +52
private fun getSignUpRequestDto(): RequestSignUpDto {
val id = binding.edtSignupId.text.toString()
val password = binding.edtSignupPassword.text.toString()
val nickname = binding.edtSignupNickname.text.toString()
val phoneNumber = binding.edtSignupPhonenumber.text.toString()
return RequestSignUpDto(
authenticationId = id,
password = password,
nickname = nickname,
phone = phoneNumber
)
}
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
private fun getSignUpRequestDto(): RequestSignUpDto {
val id = binding.edtSignupId.text.toString()
val password = binding.edtSignupPassword.text.toString()
val nickname = binding.edtSignupNickname.text.toString()
val phoneNumber = binding.edtSignupPhonenumber.text.toString()
return RequestSignUpDto(
authenticationId = id,
password = password,
nickname = nickname,
phone = phoneNumber
)
}
private fun getSignUpRequestDto(): RequestSignUpDto =
with(binding) {
RequestSignUpDto(
authenticationId = edtSignupId.text.toString(),
password = edtSignupPassword.text.toString(),
nickname = edtSignupNickname.text.toString(),
phone = edtSignupPhonenumber.text.toString()
)
}

이렇게 바꿔줄 수 도 있을 것 같아요!

Comment on lines +12 to +13
private val authService by lazy { ServicePool.authService }
val liveData = MutableLiveData<SignUpState>()
Copy link
Member

Choose a reason for hiding this comment

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

배킹 프로퍼티 사용해주세욤 ㅜㅜ

Comment on lines 57 to +58
android:layout_marginStart="70dp"
android:layout_marginTop="300dp"
Copy link
Member

Choose a reason for hiding this comment

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

허거덩 너무 큰 dp 값은 지양해주시면 감사하겠습니당

Comment on lines +42 to +44
companion object {
var memberId = "1"
}
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
companion object {
var memberId = "1"
}
companion object {
const val MEMBER_ID = "1"
}

상수화하려면 const val로 바꿔줘야 할 것 같아요! 그리고 저는 보통 변하지 않는 값을 강조하기 위해 대문자로 표시해줍니다!

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.

[FEAT] WEEK6 - xml 필수과제
2 participants