모두의 PICK 서비스 Refactoring History

서론

해당 프로젝트를 하면서 객체지향, 클린 아키텍처, MVVM과 관련된 고민이 많았습니다. 그 과정에서 코드를 뒤엎는 리팩토링이 정말 많았는데, 그 과정을 기록한 내용입니다.

(해당 글의 내용은 좋은 코드를 위한 과정일 뿐 정답이 아니란 점..!)

 

 

[PICK-70]

클린 아키텍처 적용

 

기존에 네이밍(item, model)도 혼재되어 있었고 레이어 또한 제대로 구분되지 않은 채로 방치되어 있었습니다. 게다가 translator에서 책임을 져야할 내용(entity 혹은 dto를 model로 변환시키는 로직)을 repository가 처리하고 있었습니다.

이러한 많은 문제점을 아래 사진과 같은 클린 아키텍처를 적용함으로써 해결하였습니다.

기존의 더러운 코드로 인해 불편함을 겪다가 적용을 하니 각 영역이 왜 필요한지를 알기가 더욱 쉬웠던 것 같습니다.

 

[PICK-101]

ViewModel 내부에서’만’ ViewModel 값을 수정하도록 캡슐화

데이터를 변경하는 책임을 ViewModel이 확실하게 지게끔 하여 데이터 변경 로직에 오류가 있을 경우 뷰모델만 신경쓰면 됩니다.

private val _username = MutableStateFlow("")
val username: StateFlow<String> = _username

위 같은 코드를 사용하여 외부에서는 읽기만 가능하도록 하였습니다.(캡슐화)

 

LiveData to StateFlow

기존에 ViewModel에서 쓰던 LiveData를 StateFlow로 대체하였습니다. 이렇게 한 이유에 대해선 아래 링크를 참조합니다.

https://readystory.tistory.com/207

[Android] LiveData VS StateFlow, 왜 StateFlow 를 써야할까?

 

 

[PICK-113]

Retrofit2 repository → service

Retrofit2의 interface의 네이밍을 보통 -Service로 짓는다는 것을 뒤늦게 알게 되어 리네이밍을 하였습니다.

 

Room Dao, retrofit2 service를 DataStore 영역으로 이동

위 리네이밍을 적용하고 나니 repository 내부에 PhotoService, GroupAlbumService가 놓이게 되었으며 다른 영역에 가야하는 것이 아닌가 하는 의문에 구글링을 해보았습니다. 그 결과, Room과 Retrofit2을 통해 Entity를 가져오는 것이 Data 레이어의 DataStore에 해당 한다는 것을 뒤늦게 알았고, 이에 따라 data 레이어에 source 디렉토리를 생성하고 내부를 local, remote로 구분하여 Room, Retrofit2 클래스들을 이동시켰습니다.

 

[PICK-73]

Fragment(UI) 내에서 UseCase를 참조하는 로직을 ViewModel로 이동

기존에 특정 유즈케이스를 프래그먼트 단에서 사용할 수밖에 없었던 이유는, UseCase의 API 호출 로직을 실행했을 때 에러가 발생하면 Toast를 띄워야 하는데 Toast는 ViewModel에서 접근할 수 없는 UI Thread에서만 실행이 가능하기 때문이었습니다.

 

하지만 다음과 같은 방법으로 이를 해결할 수 있었습니다.

먼저 뷰모델에 위와 같은 Flow를 만들어줍니다.

// in ViewModel
private val _toastMessage = MutableStateFlow("")
val toastMessage: StateFlow<String> = _toastMessage

 

그 다음 Fragment(UI)에서 해당 Flow를 observe하고 값이 변경될 때마다 Toast를 띄우는 코드를 짜줍니다.

// in Fragment
override fun onCreateView(
    inflater: LayoutInflater, container: ViewGroup?,
    savedInstanceState: Bundle?
): View {
    // Skipped other codes
    subscribeUi()
    // Skipped other codes
}

private fun subscribeUi() {
    lifecycleScope.launch {
        lifecycle.repeatOnLifecycle(Lifecycle.State.STARTED) {
            launch {
                viewModel.toastMessage.collectLatest {
                    if (it.isNotEmpty()) {
                        Toast.makeText(requireContext(), it, Toast.LENGTH_SHORT).show()
                    }
                }
            }
        }
    }
}

 

이에 따라 기존에 Toast를 띄워야 하는 상황에서 toastMessage 값을 바꿔줌으로써 Toast를 띄울 수 있게 됩니다.

// in ViewModel
fun readUserList() {
    viewModelScope.launch {
        try {
            userUseCase.readUserList()
        } catch (e: Exception) {
            _toastMessage.value = "정보를 불러오는 데 실패했습니다."
        }
    }
}

 

@JvmName을 통한 함수명 간략화

기존 코드에서의 함수명은 아래와 같습니다.

fun MutableList<User>.userListToUserModelList(): MutableList<UserModel>
fun MutableList<UserLocal>.userLocalListToUserModelList(): MutableList<UserModel>

처음에는 toUserModelList()라는 함수로 이름을 통일하려고 하였으나 JVM 시그니쳐가 겹쳐서 그렇게 할 수가 없었기에 위처럼 지저분하게 함수명을 지어야 했습니다.

 

하지만 @JvmName 어노테이션을 통해 해결할 수 있었습니다. 코드는 다음과 같습니다.

@JvmName("userListToUserModelList")
fun MutableList<User>.toUserModelList(): MutableList<UserModel>
@JvmName("userLocalListToUserModelList")
fun MutableList<UserLocal>.toUserModelList(): MutableList<UserModel>

 

자세한 내용은 아래 래퍼런스를 참조해주세요!

https://codechacha.com/ko/kotlin-annotations/

Kotlin에서 자주 사용하는 annotation 정리

 

상위 모듈의 액티비티로 이동하는 하드코딩 로직을 DI를 통해 제거

하위 feature 모듈에서 상위 app 모듈로 이동해야 하는 상황이 있었고, 처음에는 내키진 않지만 하드코딩으로 이를 해결하였습니다. 하지만 오류가 나기 너무나도 쉬운 취약점이므로 제거해야만 했습니다.

val intent = Intent(activity, Class.forName("org.soma.everyonepick.app.ui.HomeActivity"))
startActivity(intent)

 

아래와 같이, DI(Hilt)를 이용해서 Intent를 생성하는 데 필요한 Class<*>를 주입시킴으로써 해결할 수 있었습니다.

// 상위 모듈의 DI
@InstallIn(SingletonComponent::class)
@Module
class HomeModule {
    @Singleton
    @Provides
    fun provideHomeActivityClass(): Class<*> = HomeActivity::class.java
}

// 하위 모듈의 Framgent
@Inject lateinit var homeActivityClass: Class<*>

private fun startHomeActivity() {
    val intent = Intent(activity, homeActivityClass)
    requireActivity().startActivity(intent)
}

 

[PICK-123]

build.gradle에 api를 적용하여 의존성 정리

먼저 모두의 PICK의 멀티모듈은 위와 같은 의존성을 지니고 있습니다. 이때 각 모듈마다 아래와 같은 코드가 하나씩 존재했습니다.

implementation "androidx.core:core-ktx:$rootProject.ktxVersion"
implementation "androidx.appcompat:appcompat:$rootProject.appCompatVersion"
implementation "com.google.android.material:material:$rootProject.materialVersion"
implementation "androidx.navigation:navigation-fragment-ktx:$rootProject.navigationVersion"
implementation "androidx.navigation:navigation-ui-ktx:$rootProject.navigationVersion"
implementation "androidx.constraintlayout:constraintlayout:$rootProject.constraintLayoutVersion"
implementation "androidx.recyclerview:recyclerview:$rootProject.recyclerViewVersion"
implementation "androidx.lifecycle:lifecycle-livedata-ktx:$rootProject.lifecycleVersion"
implementation "androidx.lifecycle:lifecycle-viewmodel-ktx:$rootProject.lifecycleVersion"

이를 api 키워드를 통해 개선시킬 수 있다는 것을 알게 되었고, 위 코드를 모든 모듈에서 다 지운 뒤 common-ui에 아래와 같이 작성하였습니다.

*reference: https://jongmin92.github.io/2019/05/09/Gradle/gradle-api-vs-implementation/

api "androidx.core:core-ktx:$rootProject.ktxVersion"
api "androidx.appcompat:appcompat:$rootProject.appCompatVersion"
api "com.google.android.material:material:$rootProject.materialVersion"
api "androidx.navigation:navigation-fragment-ktx:$rootProject.navigationVersion"
api "androidx.navigation:navigation-ui-ktx:$rootProject.navigationVersion"
api "androidx.constraintlayout:constraintlayout:$rootProject.constraintLayoutVersion"
api "androidx.recyclerview:recyclerview:$rootProject.recyclerViewVersion"
api "androidx.lifecycle:lifecycle-livedata-ktx:$rootProject.lifecycleVersion"
api "androidx.lifecycle:lifecycle-viewmodel-ktx:$rootProject.lifecycleVersion"

이렇게 하면 common-ui의 상위 모듈이 모두 위 코드에서의 의존성 라이브러리를 알게 됩니다. 기존에 해당 코드 블럭이 app, login, camera, groupalbum, setting, common-ui에 존재했다면, 이젠 오로지 common-ui에서만 찾아볼 수 있게 되었습니다.

 

또한 module 자체에도 api를 적용시켰습니다. 예를 들어 app, feature modules에서 최하위 모듈인 foundation의 코드를 쓰기 위해서 각 모듈의 build.gradle에 아래의 코드를 작성해야 했습니다.

implementation project(path: ':foundation')

하지만 api를 사용하게 되면, foundation의 바로 상위 모듈에 아래 코드를 넣는 것만으로도 상위 모듈들이 모두 foundation을 사용할 수 있게 됩니다.

api project(path: ':foundation')

 

AppDatabase getInstance()를 DI를 통한 간단한 코드로 대체

기존 AppDatabase 코드는 아래와 같습니다.

@Database(entities = [User::class], version = 1, exportSchema = false)
@TypeConverters(Converter::class)
abstract class AppDatabase: RoomDatabase() {
    abstract fun userDao(): userDao

    companion object {
        // RoomDatabase는 리소스가 매우 크기 때문에 Singleton 패턴을 따라야 합니다.
        // 인스턴스가 1개만 있음을 보장하기 위해 @Volatile과 동기화를 사용합니다.
        @Volatile private var instance: AppDatabase? = null

        fun getInstance(context: Context): AppDatabase {
            return instance ?: synchronized(this) {
                instance ?: buildDatabase(context).also { instance = it }
            }
        }

        private fun buildDatabase(context: Context): AppDatabase {
            return Room.databaseBuilder(context, AppDatabase::class.java, DATABASE_NAME)
                .build()
        }
    }
}

이때 주석의 내용처럼, RoomDatabase가 매우 큰 리소스이므로 싱글톤을 보장하기 위해 위와 같은 코드를 작성하였습니다. 하지만 생각해보니 Hilt의 @Singleton을 통해 쉽게 해결되는 문제였습니다.

개선 결과는 아래와 같습니다.

 

@Database(entities = [User::class], version = 1, exportSchema = false)
@TypeConverters(Converter::class)
abstract class AppDatabase: RoomDatabase() {
    abstract fun userDao(): userDao
}

// DatabaseModule
@Singleton
@Provides
fun provideAppDatabase(@ApplicationContext context: Context): AppDatabase {
    return Room.databaseBuilder(context, AppDatabase::class.java, DATABASE_NAME).build()
}

 

NetworkModule을 foundation 모듈로 이동

OkHttpClient, Retrofit을 빌드하여 사용하는 내용은 어떤 앱이든 너무나도 흔한 것이므로 foundation 모듈(다른 앱에서도 쓸법한 매우 common한 것들이 있는 모듈로 정의함)로 이동시켰습니다. 단, response, request, entity 등의 내용은 여전히 상위 모듈에 있습니다.

 

모듈 내 하드코딩 텍스트를 strings.xml로 대체

기존에는 아래처럼 하드코딩 된 텍스트가 만연하였습니다.

Toast.makeText(context, "이미지를 저장하였습니다.", Toast.LENGTH_SHORT).show()

만약 이 상태에서 이 앱을 미국에 내놓는 상황에 직면하게 되면 Toast 메시지를 쏘는 곳마다 지역을 체크하고 분기하는 코드를 추가해야 합니다.(i18n)

이에 따라 strings.xml에 모든 내용을 넣고 기존 코드를 아래처럼 변경하였습니다.

Toast.makeText(context, context.getString(R.string.toast_save_image_success), Toast.LENGTH_SHORT).show()

이렇게 하드코딩되어 있는 string을 모두 strings.xml에 작성하여 적용하였습니다. 이 때문에 ViewModel에서 @ApplicationContext를 통해 context를 주입받아야 했습니다.

 

ListWithDummy interface를 추가하여 추상화

위 recycler view를 보면, 마지막 item이 다른 아이템과 다르다는 것을 알 수 있습니다. 저는 이것을 위해 GroupAlbumModelList라는 클래스를 만들어서 마지막 아이템에 더미 아이템이 오게끔 보장하고 있었습니다.

하지만 '마지막 아이템에 더미아이템이 오게끔 보장'하는 ModelList로 끝나는 클래스가 여러 개 있었고, 안정성을 위해(각 ModelList에서의 로직이 조금씩 다르게 되면, 코드를 사용하는 입장에서 잘못하여 사용할 여지가 큼) 상위에 interface를 추가하였습니다.

/**
 * [data]의 마지막 아이템에 항상 dummyData가 위치하는 것을 보장하는 클래스입니다.
 */
interface ListWithDummy<T> {
    val data: List<T>
    fun getItemCountWithoutDummy(): Int
    fun getListWithoutDummy(): List<T>
}

 

Checkable을 추가하여 추상화

아래처럼 체크박스가 달려있어 해당 데이터를 기록해야 하는 Model이 꽤 많았기에 추상화가 필요하였습니다.

class PhotoModel(
    val photo: Photo,
    var isChecked: MutableStateFlow<Boolean>,
    var isCheckboxVisible: Boolean
)

 

결과적으로 다음과 같이 추상화를 하였습니다.

data class PhotoModel(
    val photo: Photo,
    override var isChecked: MutableStateFlow<Boolean>,
    override var isCheckboxVisible: Boolean
): Checkable

interface Checkable {
    var isChecked: MutableStateFlow<Boolean>
    var isCheckboxVisible: Boolean
}

 

그리고 Checkbox와 관련된 자주 쓰이지만 중복되는 코드가 꽤 많았습니다.(아래 코드에서의 함수의 내용이 여러 뷰모델에 중복되어 있었습니다.) 하지만 Checkable로 추상화를 함으로써 이 중복되는 코드들을 모두 제거할 수 있게 되었습니다.

interface Checkable {
    var isChecked: MutableStateFlow<Boolean>
    var isCheckboxVisible: Boolean

    companion object {
        fun <T: Checkable> List<T>.toCheckedItemList() = this.filter { it.isChecked.value }
        fun <T: Checkable> List<T>.setIsCheckboxVisible(flag: Boolean) {
            for (i in this.indices) {
                this[i].isChecked.value = false
                this[i].isCheckboxVisible = flag
            }
        }
        fun <T: Checkable> List<T>.checkAll() {
            val isAllChecked = all { it.isChecked.value }
            for (i in this.indices) {
                this[i].isChecked.value = !isAllChecked
                this[i].isCheckboxVisible = this[i].isCheckboxVisible
            }
        }
    }
}

이에 따라 해당 로직을 처리하던 ViewModel의 코드가 줄어들게 되었고, 안정성 또한 확보할 수 있었습니다.