-
Notifications
You must be signed in to change notification settings - Fork 70
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
[#332] 컨트리뷰터 리팩토링 #333
[#332] 컨트리뷰터 리팩토링 #333
Conversation
val contributors = repository.getContributors( | ||
|
||
operator fun invoke(): Flow<Map<Int, List<Contributor>>> = | ||
repository.flowContributors( |
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.
단일 레포지토리라 하나에서 처리하도록 수정
Test Results20 tests 20 ✅ 19s ⏱️ Results for commit 57cb172. ♻️ This comment has been updated with latest results. |
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.
고생하셨습니다~! 몇 가지 제안사항이 있습니다
core/data/src/main/java/com/droidknights/app/core/data/api/fake/AssetsGithubRawApi.kt
Show resolved
Hide resolved
.../domain/src/test/java/com/droidknights/app/core/domain/usecase/GetContributorsUseCaseTest.kt
Outdated
Show resolved
Hide resolved
.../domain/src/test/java/com/droidknights/app/core/domain/usecase/GetContributorsUseCaseTest.kt
Outdated
Show resolved
Hide resolved
val newList = mutableListOf<ContributorsUiState.Contributors.Item>() | ||
|
||
// key, value 추가 | ||
forEach { (key, values) -> | ||
newList.add( | ||
ContributorsUiState.Contributors.Item.Section( | ||
title = key.toString(), | ||
) | ||
) | ||
values.forEach { item -> | ||
newList.add( | ||
ContributorsUiState.Contributors.Item.User( | ||
id = item.id, | ||
imageUrl = item.imageUrl, | ||
githubUrl = item.githubUrl, | ||
name = item.name, | ||
) | ||
) | ||
} | ||
} |
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.
flatMap을 활용해보면 어떨까요?
val newList = mutableListOf<ContributorsUiState.Contributors.Item>() | |
// key, value 추가 | |
forEach { (key, values) -> | |
newList.add( | |
ContributorsUiState.Contributors.Item.Section( | |
title = key.toString(), | |
) | |
) | |
values.forEach { item -> | |
newList.add( | |
ContributorsUiState.Contributors.Item.User( | |
id = item.id, | |
imageUrl = item.imageUrl, | |
githubUrl = item.githubUrl, | |
name = item.name, | |
) | |
) | |
} | |
} | |
val newList = this.flatMap { (key, values) -> | |
sequenceOf( | |
ContributorsUiState.Contributors.Item.Section(title = key.toString()) | |
) + values.map { item -> | |
ContributorsUiState.Contributors.Item.User( | |
id = item.id, | |
imageUrl = item.imageUrl, | |
githubUrl = item.githubUrl, | |
name = item.name | |
) | |
} | |
} |
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.
오 어떻게 할까 고민하다가 한건데 좋은 방법이네요.
): List<Contributor> | ||
|
||
suspend fun getContributorsWithYears(): List<ContributorWithYears> | ||
): Flow<Map<Int, List<Contributor>>> |
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.
고생하셨습니다!
Issue
Overview (Required)