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

충남대 Android_정기주 5주차 Step1 #56

Open
wants to merge 3 commits into
base: ddangcong80
Choose a base branch
from

Conversation

ddangcong80
Copy link

어려웠던 점

  • 우선 몸 상태가 좋지 않아서 step1 제출이 많이 늦어진 점 죄송합니다.
  • 의존성 주입을 위해 Hilt를 사용하였지만 막상 왜 사용해야 하는지 필요성을 잘 느끼지 못한 채로 코드를 작성한 것 같습니다.

중점적으로 리뷰해주셨으면 하는 점

  • room으로 데이터베이스 사용을 잘 했는지 hilt로 의존성 주입을 적절히 했는지 전체적인 구현이 궁금합니다.

Copy link

@InyoungAum InyoungAum left a comment

Choose a reason for hiding this comment

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

수고하셨습니다!
날이 더운데 컨디션 관리 잘 하시기 바랍니다


override fun saveLastPosition(latitude: Double, longitude: Double) {
val editor = spf.edit()
editor.putString("latitude", latitude.toString())

Choose a reason for hiding this comment

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

"latitude" 이러한 값들은 상수로 사용하면 실수를 줄일 수 있습니다

Copy link
Author

Choose a reason for hiding this comment

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

Step2 과제에서 피드백 수정 코드 올렸습니다!

}

override fun getPlaceList(categoryGroupName: String, callback: (List<PlaceInfo>?) -> Unit) {
retrofit.getPlaceList("KakaoAK ${BuildConfig.KAKAO_API_KEY}", categoryGroupName)

Choose a reason for hiding this comment

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

API키는 항상들어가야 하는거라 기본 프로퍼티로 넣어두면 더 좋을 것 같습니다.

@ApplicationContext context: Context,
private val retrofit: KakaoLocalService
) : SearchRepository {
private val db = Room.databaseBuilder(

Choose a reason for hiding this comment

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

이 db도 hilt로 주입받으면 좋을 것 같네요

@Singleton
fun provideKakaoLocalService(): KakaoLocalService {
return Retrofit.Builder()
.baseUrl("https://dapi.kakao.com/")

Choose a reason for hiding this comment

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

이러한 고정된 String들도 주입하는 방법으로 쓰면 유용한데요
(DB, BaseUrl등)
하나 이상의 같은 형태를 주입하기 위해서는 @qualifier 어노테이션을 사용해야합니다.

https://seosh817.tistory.com/76

위 블로그를 참고하여 String 상수들도 주입해보시기 바랍니다

fun provideMapRepository(
@ApplicationContext context: Context
): MapRepository {
return MapRepositoryImpl(context)

Choose a reason for hiding this comment

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

이렇게 Repo자체를 생성해도 가능하지만 이렇게 되면 Repo에서 굳이 inject를 해줄 필요가 없어집니다.
또한 완벽한 클린아키텍쳐 기반의 멀티모듈을 적용했을때 Repo의 구현체를 알고있어야하는 문제가 생기게 되는데요

https://hungseong.tistory.com/29

위의 블로그를 참고해서 Repo interface, RepoImpl을 생성하고 bind하는 방법으로 한번 변경해보시기 바랍니다

context.applicationContext,
AppDatabase::class.java, "placedb"
).build()
private val savePlaceDao = db.savePlaceDao()

Choose a reason for hiding this comment

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

여기선 dao만 알고있으면 좋을 것 같습니다!

@InyoungAum
Copy link

의존성 주입을 위해 Hilt를 사용하였지만 막상 왜 사용해야 하는지 필요성을 잘 느끼지 못한 채로 코드를 작성한 것 같습니다.

의존성 주입은 여러가지 이유로 사용하게 되는데요 SOLID 원칙의 D인 Dip를 지킬수 있게 되며 그에 따른 장점들은

https://velog.io/@peanut_/스프링-핵심-원리-기본편-SOLID 를 참고 바랍니다.

지금은 모듈화를 하지 않았지만

추가적으로 안드로이드에선 모듈화를 진행하게 되면 DI를 통해 각 모듈간의 결합도를 낮출수 있게 되고

한 모듈에 변경이 생겼을때 다른 모듈은 신경 쓰지 않아도 되는 장점이 생기는데요

이렇게되면 여러명이 작업하는 상황에서 코드 컨플릭이 줄고

변경이 있는 모듈에서만 모듈 빌드가 일어나게 되어 빌드시간이 획기적으로 단축되게 됩니다.

아직은 경험해보지 못하셔서 추상적인 부분으로 와닿을 수 있을텐데 기억해 두셨다가 추후 모듈화를 진행하는 프로젝트에서 경험해보시는걸 추천드립니다

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants