[Contributor] 컨트리뷰터 화면 Data 연결 #34
Conversation
|
#32 머지된 이후에 리뷰 요청 예정 |
| val libs = extensions.libs | ||
| dependencies { | ||
| "implementation"(libs.findLibrary("coroutines.core").get()) | ||
| "implementation"(libs.findLibrary("coroutines.test").get()) |
There was a problem hiding this comment.
test인데 testImplementation이 맞을것 같네요.
android일때도 test는 포함이라서
코루틴은 하나로 합치고 함수명이 어차피 분리라 한 파일로 처리되어도 괜찮을것 같아요.
There was a problem hiding this comment.
b927168 (#34)
testImplementation 체크 감사합니다 👍
| return Json { | ||
| ignoreUnknownKeys = true | ||
| }.asConverterFactory("application/json".toMediaType()) | ||
| } |
There was a problem hiding this comment.
Json 별도로 분리해두는것도 좋아요.
당장은 쓰지 않을수 있지만 json: Json 가져다 사용하는 경우도 있어서
There was a problem hiding this comment.
473c095 (#34)
에서 반영되었습니다.
| Contributor( | ||
| name = this.name, | ||
| imageUrl = this.imageUrl | ||
| ) |
There was a problem hiding this comment.
Usecase에서 data 모델 -> domain 모델 변환할 때 사용합니다.
| // TODO: DroidKnights2023_App로 변경 필요 | ||
| private const val NAME = "DroidKnights2021_App" | ||
| } | ||
| } |
There was a problem hiding this comment.
싱글인 경우에 굳이 UseCase가 필요하진 않아보이네요. 생략가능하니 생략하는게
There was a problem hiding this comment.
ContributorRepository에서 owner와 name을 인자로 받아 컨트리뷰터를 조회하고 있는데요,
GetContributorsUseCase을 호출하는 곳에서는 owner와 name을 굳이 명시하지 않고도 드로이드나이츠 2023 앱 컨트리뷰터를 받아오기를 기대할거라고 생각했어요. UseCase 없이 직접 명시하여 호출하는 것이 더 적절하다고 생각하시나요?
| scope = viewModelScope, | ||
| started = SharingStarted.WhileSubscribed(5_000), | ||
| initialValue = ContributorsUiState.Loading, | ||
| ) |
There was a problem hiding this comment.
캐싱값이 존재하므로 괜찮다고 생각했는데 최신의 데이터가 중요하다면 Eagerly 혹은 Lazily가 적절할 수 있겠군요 🤔
core/data/src/main/java/com/droidknights/app2023/core/data/di/DataModule.kt
Outdated
Show resolved
Hide resolved
core/data/src/main/java/com/droidknights/app2023/core/data/di/DataModule.kt
Outdated
Show resolved
Hide resolved
...ain/src/main/java/com/droidknights/app2023/core/domain/contributor/GetContributorsUseCase.kt
Outdated
Show resolved
Hide resolved
| override suspend fun getContributors(owner: String, name: String): List<ContributorEntity> { | ||
| return contributors | ||
| } |
There was a problem hiding this comment.
domain 모델로 변환은 어떻게 해야하나요?
There was a problem hiding this comment.
UseCase는 빼고 Entity로 바뀌는 부분이니 entity라는 이름이 명확하면 좋을것 같은데
지금 코드상으론 그런 부분이 없어서
그런 부분은 Repository에서 처리하죠.(Domain이 없더라도 Item 형태로 변환의 시점은 여기죠)
There was a problem hiding this comment.
현재는 domain이 data를 알고 있는 구조이므로 use case에서 한다고 생각했는데요,
다음 코멘트 고민의 연속인 듯 하네요 🤔
#32 (comment)
There was a problem hiding this comment.
nowinandroid 에서는 shard-model을 이용해서 공통 모델을 사용하기 때문에 가능한 방법이네요.
domain --> data
data --> domain
어떻게 잡아야 할까요? 저는 이 방식은 자주 사용해보지 않아서 레퍼런스가 부족하네요. 😿
There was a problem hiding this comment.
현석님 말씀처럼 좀 운영하다가 필요가 느껴지면 천천히 바꿔보는 것도 괜찮다고 생각해요!
|
|
||
| import com.droidknights.app2023.core.domain.contributor.Contributor | ||
|
|
||
| sealed interface ContributorsUiState { |
There was a problem hiding this comment.
ContributorsUiState 복수형태로 작성되는게 맞나요?
There was a problem hiding this comment.
UiState이니 복수개의 형태죠.
참고로 이 방식이 MVI 설명할때 들어가는 MVI라고 ... 하는 분들은 이걸 MVI라고 해석하죠(?)
There was a problem hiding this comment.
아.. 당연한 말이기는 하네요
이 화면에 contributors 외의 다른 정보가 필요하다면 어떤 식으로 전개할지가 궁금했습니다.
|
@taehwandev 대기중인 PR이 있어서 이 PR은 먼저 머지하겠습니다! 혹시 추가 코멘트가 있으시다면 다음 PR에 남겨주시면 반영할게요 🙂 |
Issue
Overview (Required)