-
Notifications
You must be signed in to change notification settings - Fork 0
[PC-1300] 다중매칭 메인 화면 구현 #171
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
Changes from 53 commits
28a9eee
3979980
ac6fa07
189238d
11ef29d
99ec0c4
1e9d769
9341533
9e58eee
a38a337
fdb2051
fe05ba1
82b26dc
e4b6584
c67367b
8cdb7f2
82c1771
b0f49d3
d22e943
c7b92b7
8a7b4c2
9204d36
1e47444
9876a2e
76ba9fc
ec512f5
23f083b
b22629b
b30ac63
625c263
e3c8233
8b64632
cc43e52
f551d00
7334d68
1d87f50
3f58bbe
1230891
875932d
dead7b0
5ddeb5d
7a7e750
10b20de
e4f69ac
40cea61
bd306bb
3a2ce13
1d54142
a0ecddf
38c3d07
0fd6c5e
5cb7e86
b277348
108ef0e
b68170e
5c05c7f
4a14e83
63d59ad
06ea723
e3eb786
bf07166
ff2c859
bfc478b
444444d
d11a725
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p1) 유틸 함수 추가시에 테스트 코드 꼭 추가해주세요...!
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 추가 완료했습니다. 38c3d07 |
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍👍👍 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,17 +1,19 @@ | ||
| package com.puzzle.data.repository | ||
|
|
||
| import com.puzzle.common.suspendRunCatching | ||
| import com.puzzle.datastore.datasource.matching.LocalMatchingDataSource | ||
| import com.puzzle.domain.model.error.HttpResponseException | ||
| import com.puzzle.domain.model.error.HttpResponseStatus | ||
| import com.puzzle.domain.model.match.MatchInfo | ||
| import com.puzzle.domain.model.match.MatchType | ||
| import com.puzzle.domain.model.profile.Contact | ||
| import com.puzzle.domain.model.profile.OpponentProfile | ||
| import com.puzzle.domain.model.profile.OpponentProfileBasic | ||
| import com.puzzle.domain.model.profile.OpponentValuePick | ||
| import com.puzzle.domain.model.profile.OpponentValueTalk | ||
| import com.puzzle.domain.repository.MatchingRepository | ||
| import com.puzzle.network.model.profile.ContactResponse | ||
| import com.puzzle.network.source.matching.MatchingDataSource | ||
| import kotlinx.coroutines.async | ||
| import kotlinx.coroutines.coroutineScope | ||
| import kotlinx.coroutines.supervisorScope | ||
| import javax.inject.Inject | ||
|
|
||
| class MatchingRepositoryImpl @Inject constructor( | ||
|
|
@@ -33,14 +35,35 @@ class MatchingRepositoryImpl @Inject constructor( | |
| return response.contacts.map(ContactResponse::toDomain) | ||
| } | ||
|
|
||
| override suspend fun getMatchInfo(): MatchInfo = matchingDataSource.getMatchInfo() | ||
| .toDomain() | ||
| override suspend fun getCanFreeMatch(): Boolean = | ||
| matchingDataSource.getCanFreeMatch().canFreeMatch | ||
|
|
||
| override suspend fun getNewInstantMatch(): Unit = | ||
| matchingDataSource.getNewInstantMatch() | ||
|
|
||
| override suspend fun getBasicMatchInfo() = matchingDataSource.getMatchInfo() | ||
| .toDomain(matchType = MatchType.BASIC) | ||
|
|
||
| override suspend fun getMatchInfoList(): List<MatchInfo> = coroutineScope { | ||
| val basicMatch = suspendRunCatching { | ||
comst19 marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| listOf(matchingDataSource.getMatchInfo().toDomain(MatchType.BASIC)) | ||
| }.getOrElse { throwable -> | ||
| if (throwable is HttpResponseException && throwable.status == HttpResponseStatus.NotFound) { | ||
| emptyList() | ||
| } else throw throwable | ||
| } | ||
|
|
||
| val toMeMatchList = async { matchingDataSource.getToMeMatchInfoList().map { it.toDomain(MatchType.TO_ME) } } | ||
| val fromMeMatchList = async { matchingDataSource.getFromMeMatchInfoList().map { it.toDomain(MatchType.FROM_ME) } } | ||
|
|
||
| basicMatch + toMeMatchList.await() + fromMeMatchList.await() | ||
| } | ||
|
Comment on lines
47
to
62
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. p2) 내부 구현이 길어서 회귀가 발생할 수 있을 것 같아요. 이후 리팩토링이나 다른 사람이 봤을 떄 이해하기 쉽도록 테스트 코드가 작성되어야 할 것 같아요.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이부분은 matchInfoList 가 3개로 나누어져있었는데, 서버쪽에서 한개의 메서드로 변경해주시기로 하였습니다!
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suspendRunCatching {
listOf(matchingDataSource.getMatchInfo().toDomain(MatchType.BASIC))
}.getOrElse { throwable ->
if (throwable is HttpResponseException && throwable.status == HttpResponseStatus.NotFound) {
emptyList()
} else throw throwable
}이 부분에 runCatching으로 감싸주신 이유가 궁금해요. viewModel에서 에러 핸들링 하면 안되나요 ? 아래 toMeMatchList랑 fromMeMatchList를 호출하는 과정에서 똑같은 예외 코드가 떨어졌을 때 에러 핸들링이 다른 것일까요?!
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 일단 MatchType.BASIC 으로 들어오는 기본 매치 정보( 오후 10시마다 갱신 )의 예외는 HttpResponseStatus.NotFound 예외만 정상작동 처리하고, (사용자를 찾을수없음) 나머지는 예외처리 하는것으로 알고 있습니다. 하지만 toMe, fromMe는 위 같이 http코드로 예외처리를 하지않고, 없다면 그냥 빈 리스트를 던져주고 있습니다! 따라서 3개의 api가 예외처리방식이 다르기때문에 요렇게 작성했습니당.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 뷰모델에서 각각 핸들링하는 방식도 생각해봤는데, 3개의 매치정보조회 api를 통합해서 올려보내는게 깔끔해보여서 작성한 것도 있어요. 고민이 만히 되었습니다. 에러케이스도 처리방식이 다르다보니..
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
toMe랑 fromMe가 똑같은 에러를 던지지 않는다면 viewModel에서 똑같은 조건 그대로 처리해도 로직이 같지 않나 싶어요~! |
||
|
|
||
| override suspend fun getOpponentProfile(): OpponentProfile = coroutineScope { | ||
| val valueTalksDeferred = async { getOpponentValueTalks() } | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 이거 제거하신 이유 궁금해요~~
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 위와 같이 toDomain 만 달려있는 함수를 굳이 따로 분리해서 만들어야하는가 에 대한 생각이 들어서 제거했습니다! |
||
| val valuePicksDeferred = async { getOpponentValuePicks() } | ||
| val profileBasicDeferred = async { getOpponentProfileBasic() } | ||
| val profileImageDeferred = async { getOpponentProfileImage() } | ||
| val valueTalksDeferred = async { matchingDataSource.getOpponentValueTalks().toDomain() } | ||
| val valuePicksDeferred = async { matchingDataSource.getOpponentValuePicks().toDomain() } | ||
| val profileBasicDeferred = async { matchingDataSource.getOpponentProfileBasic().toDomain() } | ||
| val profileImageDeferred = async { matchingDataSource.getOpponentProfileImage().toDomain() } | ||
|
|
||
| val valuePicks = valuePicksDeferred.await() | ||
| val valueTalks = valueTalksDeferred.await() | ||
|
|
@@ -64,18 +87,6 @@ class MatchingRepositoryImpl @Inject constructor( | |
| ) | ||
| } | ||
|
|
||
| private suspend fun getOpponentValueTalks(): List<OpponentValueTalk> = | ||
| matchingDataSource.getOpponentValueTalks().toDomain() | ||
|
|
||
| private suspend fun getOpponentValuePicks(): List<OpponentValuePick> = | ||
| matchingDataSource.getOpponentValuePicks().toDomain() | ||
|
|
||
| private suspend fun getOpponentProfileBasic(): OpponentProfileBasic = | ||
| matchingDataSource.getOpponentProfileBasic().toDomain() | ||
|
|
||
| private suspend fun getOpponentProfileImage(): String = | ||
| matchingDataSource.getOpponentProfileImage().toDomain() | ||
|
|
||
| override suspend fun checkMatchingPiece() = matchingDataSource.checkMatchingPiece() | ||
|
|
||
| override suspend fun acceptMatching() = matchingDataSource.acceptMatching() | ||
|
|
||
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.
이건 어떤 애니메이션인가요 ?! 영상이 없어서 확인하기가 힘드네요 ㅠㅠㅠㅠ