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
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ package camp.nextstep.edu.memo.utils
import android.content.Context
import androidx.lifecycle.ViewModel
import androidx.lifecycle.ViewModelProvider
import camp.nextstep.edu.data.repository.MemosRepository
import camp.nextstep.edu.data.MemosRepositoryInjector
import camp.nextstep.edu.memo.MainViewModel
import camp.nextstep.edu.memo.edit.EditMemoViewModel

Expand All @@ -25,10 +25,10 @@ class ViewModelFactory(
}

private fun createMainViewModel(): MainViewModel {
return MainViewModel(MemosRepository.getInstance())
return MainViewModel(MemosRepositoryInjector.provideMemosRepository())
}

private fun createEditMemoViewModel(): EditMemoViewModel {
return EditMemoViewModel(MemosRepository.getInstance())
return EditMemoViewModel(MemosRepositoryInjector.provideMemosRepository())
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package camp.nextstep.edu.data

import camp.nextstep.edu.data.local.MemosLocalSource
import camp.nextstep.edu.data.repository.MemosRepository
import camp.nextstep.edu.domain.MemosSource

/**
* Created By Malibin
* on 9월 07, 2021
*/

object MemosRepositoryInjector {

private var instance: MemosRepository? = null

@JvmStatic
fun provideMemosRepository(): MemosSource = synchronized(this) {
instance ?: MemosRepository(MemosLocalSource())
.also { this.instance = it }
}
}
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
package camp.nextstep.edu.data.repository

import camp.nextstep.edu.data.local.MemosLocalSource
import camp.nextstep.edu.domain.Memo
import camp.nextstep.edu.domain.MemosSource
import java.util.concurrent.ConcurrentHashMap
Expand All @@ -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에 관련된 관심사항이 분리되었다는 것은 동일하기 때문입니다.

private val memoCache: MutableMap<String, Memo> = ConcurrentHashMap()
Expand Down Expand Up @@ -46,13 +45,4 @@ class MemosRepository internal constructor(
memoCache.remove(id)
memosLocalSource.deleteMemo(id)
}

companion object {

private var instance: MemosRepository? = null
fun getInstance(): MemosSource = synchronized(this) {
instance ?: MemosRepository(MemosLocalSource())
.also { this.instance = it }
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@ import org.junit.jupiter.api.assertAll
*/

class MemosRepositoryTest {
lateinit var memosRepository: MemosRepository
lateinit var memosLocalSource: MemosSource
private lateinit var memosRepository: MemosSource
private lateinit var memosLocalSource: MemosSource

@BeforeEach
fun setUp() {
Expand Down Expand Up @@ -47,16 +47,6 @@ class MemosRepositoryTest {
)
}

@Test
fun `메모 저장소는 싱글턴 객체이다`() {
// given
val memoRepository1 = MemosRepository.getInstance()
val memoRepository2 = MemosRepository.getInstance()

// then
assertThat(memoRepository1).isEqualTo(memoRepository2)
}

@Test
fun `모든 메모를 두 번 불러오면 두 번째는 캐시된 메모를 불러온다`() {
// given
Expand Down