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

[Step4] 메모 (리팩터링) #13

Merged
merged 2 commits into from Sep 10, 2021
Merged

Conversation

malibinYun
Copy link
Member

MemoRepository를 internal로 변경했고,
MemoRepository Injector를 만들었습니다.

여기에서 리뷰어님이 가르쳐주신 hilt-core를 사용해보려했는데, 이상하게 빵빵 터지네요 ㅠㅠ 필기가 좀 부족했나봅니다.
저번에 hilt-core를 multiModule에 적용해서 internal을 유지했던 예제 코드를 혹시 공유해주실 수 있으신지요?

리뷰 잘부탁드립니다! 감사합니다☺

Copy link
Collaborator

@galcyurio galcyurio left a comment

Choose a reason for hiding this comment

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

galcyurio/skeleton-hilt 브랜치에 예제 코드 올려두었습니다.
코멘트 남겨둔 한 가지만 개선해보면 좋을 것 같습니다.

@@ -10,7 +9,7 @@ import java.util.concurrent.ConcurrentHashMap
* on 8월 24, 2021
*/

class MemosRepository internal constructor(
internal class MemosRepository constructor(
private val memosLocalSource: MemosSource,
) : MemosSource {
Copy link
Collaborator

Choose a reason for hiding this comment

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

보통 DataSource와 Repository는 다른 layer로 구현합니다.
따라서 domain 모듈에 새롭게 Repository interface를 만들고 MemosRepository 구현체는 Repository interface를 구현하도록 만드는게 좋을 것 같아요.

그리고 MemoSource interface는 domain에 있어도 기능상 문제는 없지만 대부분 data 모듈에 위치시킵니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

예제 코드 감사합니다!

그렇군요.. Google Todo 앱에서 Repository와 DataSource 둘 다 DataSource Interface를 구현해서, 다른 레이어지만 데이터를 가져오는 역할은 동일하니 같은 interface를 상속받는구나 라고 생각했습니다.
지금까지 DataSource와 Repository가 각자 다른 종류의 책임을 지녀야 하는 경우를 겪지 못해서 이런 고정관념이 생겼나봅니다. 반영해서 푸시하겠습니다 😊

궁금한게 생겼는데, LocalSource와 RemoteSource는 각각 멀티모듈로 나누어 관리하시나요? 아니면 한 모듈에서 패키지로 나눠 관리하시나요?
Local의 경우 Room이나 DataStore, SharedPreference 같은 친구들은 Context가 필요해서 따로 멀티모듈을 관리할 것 같은데요, 그러면 공통의 DataSource interface를 각 datasource layer가 다 가져야하기도 하고..
이런 경우에는 data module을 만들고, 그 하위에 또 local과 remote 모듈을 두어서 총 3개의 모듈을 가져가는 형태가 나을까요?
data layer에서는 DataSource interface를 가지고 Repository의 구현체를 넘기는 역할.
각 local, remote layer에서는 DataSource의 구현체를 넘기는 역할.
이렇게 분리하는 방법이 떠오르네요!

말이 좀 길었네요... 결론은, 리뷰어님은 어떤식으로 관리하시는지 궁금합니다 😀

Copy link
Collaborator

Choose a reason for hiding this comment

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

궁금한게 생겼는데, LocalSource와 RemoteSource는 각각 멀티모듈로 나누어 관리하시나요? 아니면 한 모듈에서 패키지로 나눠 관리하시나요?

모듈로 나누는 방식도 있고 모듈로 나누지 않고 패키지로만 나누는 방식도 있습니다.
말씀해주신 방식대로 모듈로 나누는 경우에는 local 모듈, remote 모듈로 나눌 수 있겠지요.
이렇게 하면 local 모듈은 안드로이드에 의존성을 가지게 하고 remote 모듈은 순수 코틀린 모듈로 나눌 수 있습니다.

이 방식의 장점을 따지자면 조금 더 느슨한 결합이 된다는 점이고 단점은 추가적으로 2가지 모듈을 더 관리 해주어야 한다는 점이겠지요.


개인적으로는 모듈로 나누는 방식과 패키지로 나누는 방식이든 크게 신경쓰지 않아서 조금 더 간단한 패키지로 나누는 방식을 많이 선택합니다.
패키지로 나누더라도 마찬가지로 DataSource interface로 하나의 layer를 더 만들어 줄 수 있고 data에 관련된 관심사항이 분리되었다는 것은 동일하기 때문입니다.

Copy link
Collaborator

@galcyurio galcyurio left a comment

Choose a reason for hiding this comment

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

3주차 마지막 단계까지 고생하셨습니다.
질문해주신 부분은 해당 쓰레드에 코멘트로 남겨두었습니다.

@@ -10,7 +9,7 @@ import java.util.concurrent.ConcurrentHashMap
* on 8월 24, 2021
*/

class MemosRepository internal constructor(
internal class MemosRepository constructor(
private val memosLocalSource: MemosSource,
) : MemosSource {
Copy link
Collaborator

Choose a reason for hiding this comment

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

궁금한게 생겼는데, LocalSource와 RemoteSource는 각각 멀티모듈로 나누어 관리하시나요? 아니면 한 모듈에서 패키지로 나눠 관리하시나요?

모듈로 나누는 방식도 있고 모듈로 나누지 않고 패키지로만 나누는 방식도 있습니다.
말씀해주신 방식대로 모듈로 나누는 경우에는 local 모듈, remote 모듈로 나눌 수 있겠지요.
이렇게 하면 local 모듈은 안드로이드에 의존성을 가지게 하고 remote 모듈은 순수 코틀린 모듈로 나눌 수 있습니다.

이 방식의 장점을 따지자면 조금 더 느슨한 결합이 된다는 점이고 단점은 추가적으로 2가지 모듈을 더 관리 해주어야 한다는 점이겠지요.


개인적으로는 모듈로 나누는 방식과 패키지로 나누는 방식이든 크게 신경쓰지 않아서 조금 더 간단한 패키지로 나누는 방식을 많이 선택합니다.
패키지로 나누더라도 마찬가지로 DataSource interface로 하나의 layer를 더 만들어 줄 수 있고 data에 관련된 관심사항이 분리되었다는 것은 동일하기 때문입니다.

@galcyurio galcyurio merged commit 912759c into next-step:malibinyun Sep 10, 2021
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.

None yet

2 participants