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

week7: 서버통신 심화 및 권한 요청, 저장소 활용 #17

Open
wants to merge 9 commits into
base: develope-xml
Choose a base branch
from

Conversation

SYAAINN
Copy link
Collaborator

@SYAAINN SYAAINN commented Jun 7, 2024

Related issue 🛠

Work Description ✏️

  • 아직 남아있던 콜백들을 다 코루틴으로 변경했습니다.
  • 자잘하게 남아있던 쓸데없는 코드들을 최대한 리팩했습니다.
  • 힐트를 이용하여 레포지토리 패턴을 적용시켰습니다.

Screenshot 📸

7._XML_.mp4

Uncompleted Tasks 😅

  • (선택) 비밀번호 변경기능

To Reviewers 📢

하면서 피를 토할 것 같았지만, 어찌저찌 다 적용시켜냈습니다 ㅠㅠㅠ,,, 아직 남아있는 불필요 코드들이나 개선점이 있다면 마음껏 패주세요.

++ 잘되다가 갑자기 memberId 전달에 문제가 생겨서.. 해결되는대로 올리겠습니다. - 해결완료

Copy link
Member

@junseo511 junseo511 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 +84 to +86
// Hilt
implementation 'com.google.dagger:hilt-android:2.48'
kapt 'com.google.dagger:hilt-compiler:2.48'
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
Collaborator Author

Choose a reason for hiding this comment

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

없었으면 이번 과제 못했습니다.. 압도적 감사

Comment on lines +18 to +24
suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> {
return authService.signUp(request)
}

suspend fun signIn(request: RequestSignInDto): Response<ResponseSignInDto> {
return authService.signIn(request)
}
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
Collaborator Author

Choose a reason for hiding this comment

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

과찬이십니다 허허 이제 이해하는 일만 남았네요

Comment on lines +48 to 63
private fun observeInfoList() {
val homeProfiledAdapter = HomeViewAdapter()
binding.rvHomeFriendList.run {
adapter = homeProfiledAdapter
layoutManager = LinearLayoutManager(requireContext())
homeProfiledAdapter.setUserProfileList(viewModel.userInfo.value)
homeProfiledAdapter.setFriendProfileList(viewModel.friendsInfo.value)

viewModel.userInfo.observe(viewLifecycleOwner, Observer { userProfile ->
userProfile?.let { viewModel.updateUserProfileUI(this,it) }
userProfile?.let { viewModel.updateUserProfileUI(this, it) }
})
viewModel.friendsInfo.observe(viewLifecycleOwner, Observer { friendProfiles ->
friendProfiles?.let { viewModel.updateFriendProfilesUI(this,it) }
friendProfiles?.let { viewModel.updateFriendProfilesUI(this, it) }
})
}
}
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 66 to 67
super.onDestroyView()
_binding = null
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
super.onDestroyView()
_binding = null
_binding = null
super.onDestroyView()

Copy link

@briandr97 briandr97 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 56 to +60
viewModel.userInfo.observe(viewLifecycleOwner, Observer { userProfile ->
userProfile?.let { viewModel.updateUserProfileUI(this,it) }
userProfile?.let { viewModel.updateUserProfileUI(this, it) }
})
viewModel.friendsInfo.observe(viewLifecycleOwner, Observer { friendProfiles ->
friendProfiles?.let { viewModel.updateFriendProfilesUI(this,it) }
friendProfiles?.let { viewModel.updateFriendProfilesUI(this, it) }

Choose a reason for hiding this comment

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

[3]
Kotlin에서는 trailing lambda(후행 람다)를 사용하면 더 깔끔하게 코드를 작성할 수 있습니다.

viewModel.userInfo.observe(viewLifecycleOwner) { userProfile ->
        userProfile?.let { viewModel.updateUserProfileUI(this, it) }
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

확실히 Observer를 명시적으로 적어주지 않아도 되고 깔끔해보이네요! 감사합니다!

Comment on lines +48 to +60
private fun observeInfoList() {
val homeProfiledAdapter = HomeViewAdapter()
binding.rvHomeFriendList.run {
adapter = homeProfiledAdapter
layoutManager = LinearLayoutManager(requireContext())
homeProfiledAdapter.setUserProfileList(viewModel.userInfo.value)
homeProfiledAdapter.setFriendProfileList(viewModel.friendsInfo.value)

viewModel.userInfo.observe(viewLifecycleOwner, Observer { userProfile ->
userProfile?.let { viewModel.updateUserProfileUI(this,it) }
userProfile?.let { viewModel.updateUserProfileUI(this, it) }
})
viewModel.friendsInfo.observe(viewLifecycleOwner, Observer { friendProfiles ->
friendProfiles?.let { viewModel.updateFriendProfilesUI(this,it) }
friendProfiles?.let { viewModel.updateFriendProfilesUI(this, it) }

Choose a reason for hiding this comment

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

[2]
어댑터를 초기화하고 뷰에 연결하는 코드와 뷰모델에서 상태를 관찰하는 코드는 전혀 다른 일을 하고 있습니다.
함수를 분리해보면 좋을 것 같습니다.

Choose a reason for hiding this comment

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

[1]
53, 54행에서 뷰모델의 상태를 직접 가져와서 어댑터에 넣어주고 있습니다.

override fun onViewCreated(view: View, savedInstanceState: Bundle?) {
    super.onViewCreated(view, savedInstanceState)
    val memberId = activity?.intent?.getStringExtra("memberId") ?: "0"
    fetchUserInfo(memberId)
    fetchFriendsInfo()
    observeInfoList()
}

private fun observeInfoList() {
    val homeProfiledAdapter = HomeViewAdapter()
    binding.rvHomeFriendList.run {
        adapter = homeProfiledAdapter
        layoutManager = LinearLayoutManager(requireContext())
        homeProfiledAdapter.setUserProfileList(viewModel.userInfo.value)
        homeProfiledAdapter.setFriendProfileList(viewModel.friendsInfo.value)
        ...
    }
}

onViewCreated에서 fetch~ 함수들이 네트워크 통신을 통해 데이터를 가져오고 있습니다.
observeInfoList()는 그 직후 실행되며 이전에 호출한 네트워크 통신은 비동기로 이뤄집니다.

observeInfoList() 함수 내부에서 어댑터에 데이터를 넣어주는 순간 우리는 네트워크 통신이 완료되었다고 보장할 수 없습니다.
그래서 통신이 완료되고 데이터를 받아온 순간을 알기 위해 우리는 라이브데이터를 관찰하는 observe 함수를 사용합니다.

그러므로 53, 54행은 없어도 될 것 같습니다.

단순히 MVVM 형태의 코드를 사용하려고 하기보다, 옵저빙 패턴을 학습해보시고 어떤 원리로 작동하는지 공부해보시면 좋을 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아직은 완벽하게 이해되지 않지만, 함수화 과정이 설득력이 부족하다는 것은 이해가 됐습니다! 옵저빙 패턴 학습해보겠습니다!

get() = _friendsInfo

fun fetchUserInfo(memberId: Int) {
viewModelScope.launch(Dispatchers.IO) {

Choose a reason for hiding this comment

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

[3]
안드로이드는 메인 스레드에서 네트워크 통신 하는 것을 막습니다. 그래서 Dispatchers.IO를 사용하신 것 같아요! 합리적인 선택입니다.
하지만 아마 Dispatchers.IO를 지워도 잘 작동하는 모습을 보실 수 있으실 겁니다.
안드로이드는 메인 스레드에서 네트워크 통신을 할 수 없는데 어떻게 가능할까요?
그 부분에 대해 학습해보셔도 좋을 것 같습니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

사실 아직 스레드에 관한 이해도가 부족해서 스레드에 대한 학습 후, 추가적으로 학습해보도록 하겠습니다. 디테일한 리뷰 감사합니다 ㅎㅎㅎ!

Comment on lines +24 to +26
private val _userInfo = MutableLiveData<HomeData.UserProfile?>()
val userInfo: LiveData<HomeData.UserProfile?>
get() = _userInfo

Choose a reason for hiding this comment

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

[3]

private val _userInfo = MutableLiveData<HomeData.UserProfile?>()
val userInfo: LiveData<HomeData.UserProfile?>
    get() = _userInfo

위와 같이 get()을 제외하고 작성해도 정상 작동합니다. 이유에 대해 알아보시면 좋을 것 같습니다!

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

백킹 프로퍼티 선언 목적에서 세미나때 봤던 코드대로 관성적으로 작성한 것 같은데 다시 한번 학습해보겠습니다!

Comment on lines +73 to +82
fun updateUserProfileUI(view: RecyclerView, it: HomeData.UserProfile) {
val adapter = view.adapter as? HomeViewAdapter
adapter?.setUserProfileList(it)
}


fun updateFriendProfilesUI(view: RecyclerView, it: List<HomeData.FriendProfile>) {
val adapter = view.adapter as? HomeViewAdapter
adapter?.setFriendProfileList(it)
}

Choose a reason for hiding this comment

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

[2]
MVVM에서 가장 중요한 점은 뷰는 뷰모델을 알지만 뷰모델은 뷰를 모르는 단방향 형태입니다.
하지만 현재 이런 형태는 양방향 구조를 만들며 오히려 (추상화가 안된) MVP에 옵저빙을 추가한 형태와 비슷합니다.

MVVM이 무조건 MVP보다 좋다라고 하는 말은 아닙니다. 여전히 MVP를 사용하는 회사들도 많으며 이 또한 필요한 학습입니다. 또한 어떠한 패턴을 따라야하는 것도 아닙니다. 다만 MVVM이라고 생각하고 사용하실까봐 말씀드립니다.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MVVM을 제외한 다른 패턴들에 대한 학습이 부족해서 생긴 문제인 것 같아서 MVP 등 다른 패턴들도 학습해보겠습니다!

Comment on lines +18 to +20
suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> {
return authService.signUp(request)
}

Choose a reason for hiding this comment

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

[2]
레포지토리 패턴은 데이터 소스를 추상화하는데 의미가 있습니다.
즉 뷰단에서는 이 값들이 우리 서버에서 왔는지, 로컬 디비에서 왔는지 혹은 파이어베이스에서 왔는지 관심이 없다는 뜻이죠.
그런 레포지토리가 레트로핏의 Response 형태로 값을 반환하게 된다면 네트워크 통신의 결과물이라는 의미를 내포하게 됩니다.
그러므로 Response로 래핑된 형태가 아닌 ResponseSignUpDto의 형태로 반환해보는 것은 어떨까요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

와,, Response 와 ResponseSignUpDto 가 그런 차이가 있었는지는 생각도 못했습니다. 그럼 ResponseSignUpDto 는 서버나 로컬디비, 파이어베이스 어디서 받아와도 가능한 형태겠군요. 레포지토리 패턴에 대해 다시 한번 생각해보겠습니다. 너무 도움 많이 됐습니다! 감사합니다!

Copy link
Collaborator Author

@SYAAINN SYAAINN 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 +84 to +86
// Hilt
implementation 'com.google.dagger:hilt-android:2.48'
kapt 'com.google.dagger:hilt-compiler:2.48'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

없었으면 이번 과제 못했습니다.. 압도적 감사

Comment on lines +18 to +24
suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> {
return authService.signUp(request)
}

suspend fun signIn(request: RequestSignInDto): Response<ResponseSignInDto> {
return authService.signIn(request)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

과찬이십니다 허허 이제 이해하는 일만 남았네요

Comment on lines 56 to +60
viewModel.userInfo.observe(viewLifecycleOwner, Observer { userProfile ->
userProfile?.let { viewModel.updateUserProfileUI(this,it) }
userProfile?.let { viewModel.updateUserProfileUI(this, it) }
})
viewModel.friendsInfo.observe(viewLifecycleOwner, Observer { friendProfiles ->
friendProfiles?.let { viewModel.updateFriendProfilesUI(this,it) }
friendProfiles?.let { viewModel.updateFriendProfilesUI(this, it) }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

확실히 Observer를 명시적으로 적어주지 않아도 되고 깔끔해보이네요! 감사합니다!

Comment on lines +48 to +60
private fun observeInfoList() {
val homeProfiledAdapter = HomeViewAdapter()
binding.rvHomeFriendList.run {
adapter = homeProfiledAdapter
layoutManager = LinearLayoutManager(requireContext())
homeProfiledAdapter.setUserProfileList(viewModel.userInfo.value)
homeProfiledAdapter.setFriendProfileList(viewModel.friendsInfo.value)

viewModel.userInfo.observe(viewLifecycleOwner, Observer { userProfile ->
userProfile?.let { viewModel.updateUserProfileUI(this,it) }
userProfile?.let { viewModel.updateUserProfileUI(this, it) }
})
viewModel.friendsInfo.observe(viewLifecycleOwner, Observer { friendProfiles ->
friendProfiles?.let { viewModel.updateFriendProfilesUI(this,it) }
friendProfiles?.let { viewModel.updateFriendProfilesUI(this, it) }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

아직은 완벽하게 이해되지 않지만, 함수화 과정이 설득력이 부족하다는 것은 이해가 됐습니다! 옵저빙 패턴 학습해보겠습니다!

get() = _friendsInfo

fun fetchUserInfo(memberId: Int) {
viewModelScope.launch(Dispatchers.IO) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

사실 아직 스레드에 관한 이해도가 부족해서 스레드에 대한 학습 후, 추가적으로 학습해보도록 하겠습니다. 디테일한 리뷰 감사합니다 ㅎㅎㅎ!

Comment on lines +24 to +26
private val _userInfo = MutableLiveData<HomeData.UserProfile?>()
val userInfo: LiveData<HomeData.UserProfile?>
get() = _userInfo
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

백킹 프로퍼티 선언 목적에서 세미나때 봤던 코드대로 관성적으로 작성한 것 같은데 다시 한번 학습해보겠습니다!

Comment on lines +73 to +82
fun updateUserProfileUI(view: RecyclerView, it: HomeData.UserProfile) {
val adapter = view.adapter as? HomeViewAdapter
adapter?.setUserProfileList(it)
}


fun updateFriendProfilesUI(view: RecyclerView, it: List<HomeData.FriendProfile>) {
val adapter = view.adapter as? HomeViewAdapter
adapter?.setFriendProfileList(it)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

MVVM을 제외한 다른 패턴들에 대한 학습이 부족해서 생긴 문제인 것 같아서 MVP 등 다른 패턴들도 학습해보겠습니다!

Comment on lines +18 to +20
suspend fun signUp(request: RequestSignUpDto): Response<ResponseSignUpDto> {
return authService.signUp(request)
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

와,, Response 와 ResponseSignUpDto 가 그런 차이가 있었는지는 생각도 못했습니다. 그럼 ResponseSignUpDto 는 서버나 로컬디비, 파이어베이스 어디서 받아와도 가능한 형태겠군요. 레포지토리 패턴에 대해 다시 한번 생각해보겠습니다. 너무 도움 많이 됐습니다! 감사합니다!

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.

3 participants