Skip to content
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

[CT-3-3] 차트게임 기능 - data 레이어 - repository #22

Conversation

nosorae
Copy link
Collaborator

@nosorae nosorae commented May 18, 2024

reference #3

이 PR을 보시기 전에 #19 PR -> #20 PR 을 보는 것이 순서에 맞습니다.
이 PR이 #3 의 마지막 PR입니다.

Overview

@nosorae nosorae added this to the 최소 기능 구현 milestone May 18, 2024
@nosorae nosorae requested a review from f-lab-nathan May 18, 2024 16:17
@nosorae nosorae self-assigned this May 18, 2024
@nosorae nosorae changed the title [CT-3] 차트게임 기능 - data 레이어 - repository [CT-3-3] 차트게임 기능 - data 레이어 - repository May 18, 2024
nosorae and others added 2 commits May 20, 2024 22:33
…twork-source

[CT-3-1] 차트게임 기능 - data 레이어 - network
@nosorae nosorae requested a review from f-lab-nathan May 20, 2024 14:36
Base automatically changed from feature/ct-3-chart-game-data-local-source to feature/ct-3-chart-game-data-network-source May 20, 2024 14:40
@nosorae
Copy link
Collaborator Author

nosorae commented May 21, 2024

슬랙에서 질문 답변해주신 withTransaction을 적용했습니다.

@nosorae
Copy link
Collaborator Author

nosorae commented May 21, 2024

아마 23일까지는 리뷰 반영할 수 있을 것 같습니다.

  1. LocalDataSource 인터페이스화 하여 Repository가 Database에 직접적으로 의존하지 않게 하기
  2. launch 리스트에 joinAll 제거

Copy link

import androidx.room.withTransaction
import javax.inject.Inject

class ChartTrainerDatabaseTransactionHelper @Inject constructor(
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit; helper 대신 executor는 어떨까요?

import javax.inject.Inject
import kotlinx.coroutines.flow.Flow

class ChartTrainerLocalDBDataSourceImpl @Inject constructor(
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit; 지금은 대부분 코드가 localDatasource하나에 모여있는데 모델을 어떻게 관리하고 나눌지에 따라서 여러개의 datasource로 나눠질수도 있습니다.

@f-lab-nathan
Copy link
Collaborator

고생하셨습니다. ㅎㅎ 추가 코멘트는 다음 PR에서 적용해주세요~!

@f-lab-nathan f-lab-nathan merged commit 30c395d into feature/ct-3-chart-game-data-network-source May 24, 2024
2 checks passed
@nosorae nosorae deleted the feature/ct-3-chart-game-data-repository-impl branch June 2, 2024 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants