-
Notifications
You must be signed in to change notification settings - Fork 1
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/#315] 딱 맞는 인턴 공고 페이징 구현 #317
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수고 완전 많았어요ㅠㅠ!! 로직 이해하는 데에도 시간이 꽤 걸렸네용 ㄷㄷ 최고!!👍👍
val recommendInternFlow: Flow<PagingData<HomeRecommendedIntern>> = combine( | ||
_recommendInternFlow.flatMapLatest { | ||
it.cachedIn(viewModelScope) | ||
}, scrapStateFlow | ||
) { paging, scrapState -> | ||
paging.map { intern -> | ||
val isScrapped = scrapState[intern.internshipAnnouncementId] ?: intern.isScrapped | ||
intern.copy( | ||
isScrapped = isScrapped | ||
) | ||
} | ||
} | ||
|
||
private fun refreshRecommendInternFlow() { | ||
_recommendInternFlow.value = getRecommendInternData() | ||
} | ||
|
||
private fun getRecommendInternData(): Flow<PagingData<HomeRecommendedIntern>> { | ||
val pagingFlow = homeRepository.getRecommendIntern( | ||
sortBy = _homeState.value.sortBy.type | ||
).cachedIn(viewModelScope) | ||
|
||
return combine( | ||
pagingFlow, scrapStateFlow | ||
) { paging, scrapState -> | ||
paging.map { intern -> | ||
val isScrapped = scrapState[intern.internshipAnnouncementId] ?: intern.isScrapped | ||
intern.copy( | ||
isScrapped = isScrapped |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
combine하는 부분이 반복되는 것 같은데 하나의 함수로 묶기에는 무리일까요....?
그나저나 로직은 엄청나네요 ..ㄷㄷ
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분은 수정했습니다~!
configureKotlin() | ||
configureCoroutineKotlin() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
configureCoroutineKotlin()
함수를 추가해준 이유가 있을까용?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
페이징 처리를 위해 domain 층에서 coroutine이 필요하여 여기에 추가를 해뒀습니다,,ㅎ
그냥 여기에 넣었는데, 코루틴이 필요한 도메인에 각각 implement 해주는게 낫겟죠?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코틀린 라이브러리만 필요한 파일에도 coroutine이 생기는 거라서.. coroutine을 필요로하는 도메인에 각각 넣어주는 게 좋을 것 같아요...!!
|
||
private var hasNextPage = false | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hasNextPage
변수만 전역적으로 사용하는 이유가 있나요.....?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
그..러게요?
launchSingleTop = true | ||
restoreState = true | ||
restoreState = false | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
restoreState의 디폴트 값은 false여서 아예 삭제해도 될 것 같아요!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
이 부분이 바텀바 내 활성화된 아이콘을 눌렀을 때 다시 호출되는 것을 방지하는 녀석이더라구요..
그래서 다시 true로 바꾸고 스크랩 상태를 잘 관리하도록 바꿨습니다!
domain/home/build.gradle.kts
Outdated
dependencies { | ||
implementation(libs.paging.common) | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
확인했습니당 저도 찾아봤는데 일단 이렇게 가도 괜찮을 것 같아요..!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
와 로직 대박이네요.. 수고하셨습니다!!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
바뀐 부분 확인했습니다!! 새벽까지 일하는 개발자 ㄷㄷㄷ
configureKotlin() | ||
configureCoroutineKotlin() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
코틀린 라이브러리만 필요한 파일에도 coroutine이 생기는 거라서.. coroutine을 필요로하는 도메인에 각각 넣어주는 게 좋을 것 같아요...!!
…_paging # Conflicts: # data/home/build.gradle.kts # feature/home/build.gradle.kts # feature/home/src/main/java/com/terning/feature/home/HomeViewModel.kt # gradle/libs.versions.toml
⛳️ Work Description
📸 Screenshot
241230_.mp4
📢 To Reviewers
libs.paging.common
라이브러리는 안드로이드 플랫폼에 종속적이지 않고 순수 코틀린 코드로 이루어졌다고 해서 일단 도메인에 포함시켜두었습니다!