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

HW-7 Koin 적용 #580

Merged
merged 11 commits into from Apr 16, 2020
Merged

HW-7 Koin 적용 #580

merged 11 commits into from Apr 16, 2020

Conversation

mtjin
Copy link
Contributor

@mtjin mtjin commented Apr 13, 2020

factory 가 필요한데가 없어보여서 single로만 했는데 맞는지 모르겠네요.

@mtjin mtjin added the Review Needed 리뷰어의 리뷰가 필요한 경우 label Apr 13, 2020
@mtjin mtjin self-assigned this Apr 13, 2020
Comment on lines 16 to 27
single<MovieLocalDataSource> { MovieLocalDataSourceImpl(get()) }
single<LoginLocalDataSource> { LoginLocalDataSourceImpl(get()) }
single<PreferenceManager> { PreferenceManager(androidContext()) }
single<MovieDao> { get<MovieDatabase>().movieDao() }
single<MovieDatabase> {
Room.databaseBuilder(
androidContext(),
MovieDatabase::class.java, "Movie.db"
)
.allowMainThreadQueries()
.build()
}
Copy link
Contributor

Choose a reason for hiding this comment

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

다른 모듈 에서도 보이는 코드인데 androidContext들을 get() 으로 바꿀 수 있을 것 같아요 ( 제가 이해한게 맞다면, startKoin에서 context를 전달받고 get()으로 전달받은 context를 사용 할 수 있는것 같아요 )

Copy link
Contributor Author

Choose a reason for hiding this comment

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

e3a6a8c
수정했습니다.

@kangraemin kangraemin added Answer Needed PR담당자의 응답이 필요한 경우 and removed Review Needed 리뷰어의 리뷰가 필요한 경우 labels Apr 14, 2020
@mtjin mtjin added Review Needed 리뷰어의 리뷰가 필요한 경우 and removed Answer Needed PR담당자의 응답이 필요한 경우 labels Apr 15, 2020
Copy link
Contributor

@kangraemin kangraemin left a comment

Choose a reason for hiding this comment

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

@sooakim approve했습니다 ~

@kangraemin kangraemin added the Mentor Needed 멘토가 필요한 경우 label Apr 15, 2020
Comment on lines 15 to 16
if (BuildConfig.DEBUG) androidLogger()
else androidLogger(Level.NONE)
Copy link
Collaborator

Choose a reason for hiding this comment

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

{} 추가해주세요

Copy link
Contributor Author

Choose a reason for hiding this comment

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

170728d
수정했습니다.

@sjjeong sjjeong added Answer Needed PR담당자의 응답이 필요한 경우 and removed Review Needed 리뷰어의 리뷰가 필요한 경우 labels Apr 15, 2020
@mtjin mtjin added Review Needed 리뷰어의 리뷰가 필요한 경우 and removed Answer Needed PR담당자의 응답이 필요한 경우 labels Apr 15, 2020
Copy link
Collaborator

@sjjeong sjjeong left a comment

Choose a reason for hiding this comment

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

보통 mvp 할때 Presenter를 factory로 많이 써요
지금 우리 예제에서는 mvvm이라서 viewModel을 사용하고 model 쪽은 single로만 구현하면 돼요
LGTM ✊ ✌️ 👋

@sjjeong sjjeong added Merge Needed 모든 리뷰어가 Approve를 한 경우 and removed Review Needed 리뷰어의 리뷰가 필요한 경우 labels Apr 16, 2020
@mtjin mtjin merged commit 980065b into StudyFork:2003/mtjin Apr 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mentor Needed 멘토가 필요한 경우 Merge Needed 모든 리뷰어가 Approve를 한 경우
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants