Skip to content

[Week7] XML 필수 과제#19

Open
arinming wants to merge 3 commits into
develop-xmlfrom
feat/week-xml7
Open

[Week7] XML 필수 과제#19
arinming wants to merge 3 commits into
develop-xmlfrom
feat/week-xml7

Conversation

@arinming

@arinming arinming commented Jun 7, 2024

Copy link
Copy Markdown
Member

🧩 Issue number

이슈 번호 : #17

✨ Summary

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

🔍 PR Type

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

📷 Screenshot

Screen_recording_20240607_213850.mp4

📔 Other Information

@junseo511 junseo511 left a comment

Copy link
Copy Markdown
Member

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
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

Choose a reason for hiding this comment

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

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

@briandr97 briandr97 left a comment

Copy link
Copy Markdown

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)

Copy link
Copy Markdown

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
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

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

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

Copy link
Copy Markdown

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()을 제외하고 작성해도 정상 작동합니다. 이유에 대해 알아보시면 좋을 것 같습니다!

Copy link
Copy Markdown

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
Copy Markdown
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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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

Copy link
Copy Markdown
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