Skip to content

[Week7] Compose 필수 과제#20

Open
arinming wants to merge 3 commits into
develop-composefrom
feat/week-compose7
Open

[Week7] Compose 필수 과제#20
arinming wants to merge 3 commits into
develop-composefrom
feat/week-compose7

Conversation

@arinming

@arinming arinming commented Jun 7, 2024

Copy link
Copy Markdown
Member

🧩 Issue number

이슈 번호 : #18

✨ Summary

팔로워 리스트 레포지토리 패턴 적용

🔍 PR Type

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

📷 Screenshot

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

xml 코드 야무지게 쓰셨군요~~ 삼성티엠~~

Comment thread app/build.gradle

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

안녕하세요 명예오비 이준원입니다. 정말 오랜만에 코드리뷰를 남기네요😁
많이 바쁘신 것 같아 바뀐 것이 크게 없네요..
추가적으로 공부하면 좋을 것 같은 점을 위주로 리뷰를 남겼습니다!
나중에 시간 되실 때 과제 구현하신 후 리뷰 재요청주셔도 됩니다 ㅎ ㅎ

Comment on lines +12 to +14
suspend fun getUserList(page: Int): Response<ResponseUserDto> {
return withContext(Dispatchers.IO) {
followerService.getUserList(page).execute()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

레트로핏 Service 에서 suspend 키워드를 붙이면 내부적으로 코루틴으로 서버 통신을 비동기 처리합니다!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

조금 어려운 내용일 수 있는데요..!
안드로이드에서 Dispatcher 의 경우 하드코딩하는 것보다는 생성자 주입 받는 것을 권장하고 있습니다.

https://developer.android.com/kotlin/coroutines/coroutines-best-practices#inject-dispatchers

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

withContext 와 CoroutineScope 의 차이는 뭘까요?! 🤔

이것도 추후 코루틴에 대해 자세히 학습하실 때 공부하시면 좋을 것 같습니다!

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.

CoroutineScope는 코루틴을 시작하고 관리하지만 withContext는 다른 Dispatcher에서 실행할 때 사용된다..정도일까용!! 코루틴 관련해서 더 깊게 공부해야 할 것 같아요 감사합니다 ㅜㅜ

Comment on lines +22 to +25
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.

해당 변수는 사용하고 있지 않군요 🥲
이 부분은 왜 StateFlow 가 아닌 LiveData를 사용하셨을까요??

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로 전체적용하려고 하다가 빠뜨린 것 같습니당..! 지우도록 하겄습니다 🫡

@@ -2,5 +2,6 @@ package com.sopt.now.compose.ui.signUp

sealed class SignUpSideEffect {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

UiEvent 의 네이밍을 왜 XXXSideEffect 로 명명하셨는지 궁금하군요!

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

추가적으로 Loading 중에 message 가 정말 필요한지 궁금합니다!

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.

message는 따로 필요하지 않을 것 같아요..! 삭제하겠습니다,.!
상태를 피드백해주는 코드라고 생각해서 SideEffect로 명명했는데, Event로 쓰는게 좀더 명확할까요?!..!!

Comment on lines +9 to +10
class FollowerRepository {
private val followerService = ServicePool.followerService

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

    1. 왜 Repository 라는 중개자 역할의 저장소라는 개념이 나왔을까요??
    1. service 를 생성자 주입이 아닌 필드에 선언하신 이유가 있을까요?

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.

  1. 데이터가 변경되어도 비즈니스 로직에 영향을 주지 않기 위함인 것 같아요.! 단일 책임 원칙과도 관련이 있는 것 같습니다.
  2. 의존성 주입을 하다 말았는데, 현재 Hilt 공부하면서 리팩토링중입니당..!

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