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] XML 필수 과제 #19

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

[Week7] XML 필수 과제 #19

wants to merge 3 commits into from

Conversation

arinming
Copy link
Member

@arinming arinming commented Jun 7, 2024

🧩 Issue number

이슈 번호 : #17

✨ Summary

팔로워 리스트 불러오기를 Repository 패턴으로 적용했습니다

🔍 PR Type

  • 필수 XML
  • 필수 Compose
  • 심화 XML
  • 심화 Compose
  • 도전 XML
  • 도전 Compose

📷 Screenshot

Screen_recording_20240607_213850.mp4

📔 Other Information

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.

너무 간단하게 레포지토리 붙이신~~ ㄷㄷ LGTM

private val followerService = ServicePool.followerService

suspend fun getUserList(page: Int): Response<UserDataResponse> {
return withContext(Dispatchers.IO) {
Copy link
Member

Choose a reason for hiding this comment

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

Dispatchers.IO를 추가하신 이유가 무엇인가요?

Copy link
Member Author

Choose a reason for hiding this comment

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

코루틴을 사용할 때 서버에서 파일 다운로드시 IO를 주입해야한다고 알고있숩니다.!

import retrofit2.Response

class FollowerRepository {
private val followerService = ServicePool.followerService
Copy link
Member

Choose a reason for hiding this comment

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

follower 라는 네이밍을 한 이유가 있나요?

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.

리뷰를 두 번밖에 남기지 않았는데, 벌써 한 기수가 끝났네요.
리뷰에 대해 궁금한 점 있으시면 편하게 질문 주세요!
얼마 없는 리뷰였지만 도움이 되셨기를 바랍니다.
앱잼 화이팅입니다~!

private var _eventNetworkError = MutableLiveData(false)
val eventNetworkError: LiveData<Boolean>
get() = _eventNetworkError
private var _isNetworkErrorShown = MutableLiveData(false)

Choose a reason for hiding this comment

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

[2]
MutableLiveData를 var로 선언한 이유는 무엇일까요? _isNetworkErrorShown 자체가 바뀌진 않고 LiveData가 감싸고 있는 값만 변경하고 있는 것 같아서 여쭤봅니다!

또 backing property 형태를 사용하지 않고 있는데 앞에 언더바를 붙인 이유도 궁금합니다!

Copy link
Member Author

Choose a reason for hiding this comment

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

_isNetworkErrorShown 자체가 변경되지 않아서 val로 선언하는 것이 적절해보이네욥..!
언더바를 붙인 이유는 외부에서의 접근을 막기 위해 습관적으로 들인 것입니당 ,,!


private val _followerState = MutableStateFlow<List<UserData>>(emptyList())
val followerState = _followerState.asStateFlow()
private var _eventNetworkError = MutableLiveData(false)
val eventNetworkError: LiveData<Boolean>
get() = _eventNetworkError

Choose a reason for hiding this comment

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

[3]

private var _eventNetworkError = MutableLiveData(false)
val eventNetworkError: LiveData<Boolean> = _eventNetworkError

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

Choose a reason for hiding this comment

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

[2]
이벤트를 LiveData 형태로 들고 있으며 다른 observer들이 관찰하게 되면 어떤 문제가 발생할 수 있을까요?
이벤트는 한 번 발생하고 처리되면 사라져야하는 것이 아닐까요?
상태이벤트에 대해 생각해보면 좋을 것 같습니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

LiveData가 MutableLiveData를 감싸서 MutableLiveData의 변화를 관찰할 수 있기 때문에 get을 지워도 되는 것 맞을까요..!??!

Comment on lines 19 to +23
private val _followerState = MutableStateFlow<List<UserData>>(emptyList())
val followerState = _followerState.asStateFlow()
private var _eventNetworkError = MutableLiveData(false)
val eventNetworkError: LiveData<Boolean>
get() = _eventNetworkError

Choose a reason for hiding this comment

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

[5]
StateFlowLiveData를 섞어서 사용하신 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

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

StateFlow로 통일하려다가 추가적으로 고치지 못하였습니다..! ㅜㅜ

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