리팩토링 회고 + mvvm clean architecture는 왜 쓰는걸까

리팩토링

작년 12월부터 예전에 작성했던 코드를 둘러보며 리팩토링을 해보는 시간을 가졌다.
현재 2개정도 진행했는데 프로젝트는 두개정도?

• FloatingTwitter
  페이스북 메신저의 '버블'과 동일하게 오버레이로 아이콘을 띄워주다 알림이 오면 표시해주는 앱
  기존 트위터앱의 경우 DM, 멘션등의 알림이 즉각적으로 notify되지 않는 경우가 있기때문에 직접 API를 이용해서 일정 주기로 풀링하고 알려주는 앱으로 만들었었다.
  https://github.com/choi-hyeseong/FloatingTwitter

• NDM (Nfc Based MDM)
  휴대전화의 NFC기능을 이용해서 출/퇴근 카드의 역할을 할 수 있는 MDM 앱
  기본적으로 카메라 차단의 기능이 있으며, NFC태깅을 통해 서버와의 정보전달이 가능하다.
  https://github.com/choi-hyeseong/NfcBasedMdm

느낀점

내가 2년전에 진짜 코드를 이렇게 짰던게 맞나? - FT

• Before : (JSON Deserialize와 멘션 파악로직, API 호출이 하나로 뒤엉켜 매우 보기 힘든 코드)

 override fun run() {
        while (!Thread.interrupted()) {
            try {
                var mention = false
                var dm = false
                Thread.sleep(7000)
                val result = tweet.tweets()?.usersIdMentions(id)?.execute()?.data
                if (lastMentionId.isEmpty()) {
                    //서비스 처음 시작된경우
                    result?.get(0).apply {
                        lastMentionId = this?.id!!
                    }
                } else {
                    //let 쓰면 객체 자체가 바뀜
                    result?.apply {
                        if (get(0).id != lastMentionId) {
                            var updates = false
                            for (i in 0 until size) {
                                if (get(i).id == lastMentionId) {
                                    handleUI { increaseNotificationCounterBy(i) } //작동안될리는 없긴한데.. 음.. 멘션 많이오면 안될수도..?
                                    counter += i
                                    updates = true
                                    break
                                }
                            }
                            mention = true
                            lastMentionId = get(0).id
                            if (!updates)
                                increaseNotificationCounterBy(1)
                        }
                    }
                }
                val request = Request.Builder().url(URL(DM_URL))
                    .header("Authorization", "Bearer ${setting.token}").header("Accept", "/*/")
                    .header("Connection", "keep-alive").build()
                val response = OkHttpClient().newCall(request).execute()
                response.body?.apply {
                    val jObj = JSONObject(response.body!!.string())
                    val array = jObj.getJSONArray("data")
                    if (lastDmId.isEmpty()) {
                        lastDmId = array.getJSONObject(0).getString("id")
                    }
                    else {
                        if (array.getJSONObject(0).getString("id") != lastDmId) {
                            var updates = false
                            for (i in 0 until array.length()) {
                                if (array.getJSONObject(i).getString("id") == lastDmId) {
                                    handleUI { increaseNotificationCounterBy(i) }
                                    counter += i
                                    updates = true
                                    break
                                }
                            }
                            dm = true
                            lastDmId = array.getJSONObject(0).getString("id")
                            if (!updates)
                                increaseNotificationCounterBy(1)
                            //나 뭐해...
                        }
                    }
                }
                if (mention || dm) {
                    var color: Int? = Color.RED
                    when {
                        dm && mention -> color = setting.twin
                        dm -> color = setting.dm
                        mention -> setting.mention
                    }
                    bubbleView.findViewById<ImageView>(com.siddharthks.bubbles.R.id.notification_background)
                        .setColorFilter(color!!)
                }
                Thread.sleep(53000)
            } catch (e: Exception) {
                if (e is InterruptedException)
                    break
                else
                    e.printStackTrace()
            }
        }
    }

• After : 지금도 좋은 코드라고 장담할 순 없지만, repository - usecase를 통해 각 호출부를 분리 및 Retrofit을 이용한 JSON Deserialize 분리 등등을 통해 최대한 서비스를 가볍게 유지하게..

	CoroutineScope(Dispatchers.IO).launch {
            val avatar = avatarResult.getOrThrow() // 위에서 Failure 체크
            withContext(Dispatchers.Main) {
                changeIcon(avatar) // 버블 아이콘 업데이트
            }

            startAPIListeningUseCase(user, oAuthToken) { event ->
                val amount = event.sumOf { it.amount } // 총 notify할 수량
                if (amount != 0) {
                    val firstEvent = event[0]
                    // color setup
                    val color = if (event.size == 2) settingData.bothNotifyColor
                    else if (firstEvent.type == EventType.DM) settingData.directMessageColor
                    else settingData.mentionColor

                    // main dispatcher에서 setup
                    CoroutineScope(Dispatchers.Main).launch {
                        increaseCounter(amount)
                        changeBackgroundColor(Color.parseColor(color))
                    }
                }
            }
        }
    }

좀더 더 나은 방식이 없을까? - NDM

• Before : 현재는 RSA 암호화와 AES 복호화기능만 쓰는 Util방식도 나쁘진 않았지만, 이걸 클래스단위로 분리할 수 없을까?

class EncryptUtil {
    companion object {
        fun RSAEncrypt(input: String, key: String): String {
            return try {
                val keyFactory = KeyFactory.getInstance("RSA")
                val bytePublicKey: ByteArray = Base64.getUrlDecoder().decode(key)
                val publicKeySpec = X509EncodedKeySpec(bytePublicKey)
                val publicKey: PublicKey = keyFactory.generatePublic(publicKeySpec)
                val cipher: Cipher = Cipher.getInstance("RSA/NONE/PKCS1Padding")
                    .apply { init(Cipher.ENCRYPT_MODE, publicKey) } //패딩 문제로 맞춰줘야됨.
                val encryptByte: ByteArray = cipher.doFinal(input.toByteArray(StandardCharsets.UTF_8))
                Base64.getUrlEncoder()
                    .encodeToString(encryptByte)
            } catch (e: Exception) {
                e.printStackTrace()
                ""
            }
        }
        //패딩이나 암호화 코드 없다고 Lint가 알려주는데, 추가하면 서버랑 통신시 오류 생겼던걸로..
        @SuppressLint("GetInstance")
        fun AESDecrypt(input: String, key: String): String {
            val secretKeySpec: Key = SecretKeySpec(key.substring(0, 16).toByteArray(), "AES")
            return try {
                val cipher: Cipher =
                    Cipher.getInstance("AES").apply { init(Cipher.DECRYPT_MODE, secretKeySpec) }
                val urlDecode = Base64.getUrlDecoder().decode(input.toByteArray())
                val decrypt: ByteArray = cipher.doFinal(urlDecode)
                String(decrypt)
            } catch (e: Exception) {
                ""
            }
        }
    }
}

• After : 암호화 / 복호화는 암호화할 대상, 키 두개의 파라미터는 공통적이므로 인터페이스로 묶은뒤 확장성을 가져옴
  AES 암호화를 사용할 수 있지만, 나중에는 RSA를 쓸 수도 있고, 패딩값이 바뀔수도 있는데, 이를 클래스로 분리해놓으면 기존 코드에 영향이 확연히 줄어들것으로 보임

/**
 * 암,복호화를 수행하는 인터페이스 (RSA, AES 지원)
 */
interface Crypto {

    /**
     * 암호화를 수행합니다.
     * @param message 암호화할 메시지
     * @param key 암호화에 사용될 키
     * @return 암호화된 결과값입니다.
     */
    fun encrypt(message : String, key : String) : String

    /**
     * 암호화를 수행합니다. Key 자체값을 사용합니다.
     * @param key 암호화에 사용될 키
     */
    fun encrypt(message : String, key : Key) : String

    /**
     * 복호화를 수행합니다.
     * @param encryptedMessage 암호화된 문자열
     * @param key 복호화에 사용될 키
     * @return 복호화된 결과입니다.
     */
    fun decrypt(encryptedMessage: String, key : String) : String


    /**
     * 복호화를 수행합니다. Key 자체값을 사용합니다.
     * @param key 복호화에 사용될 키
     */
    fun decrypt(encryptedMessage : String, key : Key) : String
}

/**
 * AES 구현체
 * @see Crypto.javaClass
 */
class AESCrypto : Crypto {

    companion object {
        private const val IV_LEN = 16
    }

    // AES의 key는 URL Encode 필요없이 문자열이면 됩니다.
    override fun encrypt(message: String, key: String): String {
        // AES는 16,24,32 바이트를 지켜야 하므로 키가 길경우 자름. - Exception 발생 가능성 있음.
        return encrypt(message, SecretKeySpec(key.substring(0, 16).toByteArray(), "AES"))
    }

    override fun encrypt(message: String, key: Key): String {
        return try {
            val iv = ByteArray(IV_LEN).also { SecureRandom().nextBytes(it) }
            val cipher: Cipher = Cipher.getInstance("AES/CBC/PKCS7Padding")
            // Android Keystore의 경우 자체 iv를 갖고 있으므로 에러가 발생하는경우 - 자체 IV로 변경
            kotlin.runCatching { cipher.init(Cipher.ENCRYPT_MODE, key, IvParameterSpec(iv)) }
                .onFailure { cipher.init(Cipher.ENCRYPT_MODE, key) }
            val encrypt = cipher.doFinal(message.toByteArray())
            // base64 encode
            Base64.getUrlEncoder().encodeToString(cipher.iv + encrypt)
        }
        catch (e: Exception) {
            Log.w(getClassName(), "Can't encrypt message : ${e.message}")
            ""
        }
    }

    // AES의 key는 URL Encode 필요없이 문자열이면 됩니다.
    override fun decrypt(encryptedMessage: String, key: String): String {
        return decrypt(encryptedMessage, SecretKeySpec(key.substring(0, 16).toByteArray(), "AES"))
    }

    override fun decrypt(encryptedMessage: String, key: Key): String {
        return try {
            // base64 decode
            val urlDecode = Base64.getUrlDecoder().decode(encryptedMessage.toByteArray())
            val iv = urlDecode.copyOfRange(0, IV_LEN)
            val encrypt = urlDecode.copyOfRange(IV_LEN, urlDecode.size)

            val cipher: Cipher = Cipher.getInstance("AES/CBC/PKCS7Padding")
                .apply { init(Cipher.DECRYPT_MODE, key, IvParameterSpec(iv)) }
            // decrypt
            val decrypt: ByteArray = cipher.doFinal(encrypt)
            String(decrypt)
        }
        catch (e: Exception) {
            Log.w(getClassName(), "Can't decrypt message : ${e.message}")
            ""
        }
    }

}

제네릭은 아직도 어렵다..

기존 SharedPreferences는 동시성 문제가 있어 PreferenceDataStore을 사용해보려 했는데, 얘는 Primitive Type이더라도 제네릭(Preferences.Key<Int>)을 사용해서 String형식의 키값을 사용하는 방식을 더이상 쓸 수 없었다.

기존에는 preferences.getString("KEY", "INSTEAD)로 가져왔다면 지금은 dataStore.data.map { pref -> pref[stringPreferenceKey("KEY")] } 형식으로 가져와야 하니..

물론 Companion Object에 키 설정하고, 가져오고 저장하면 되는 문제인데, 기존에 사용하던 인터페이스와 호환되지 않는 문제가 컸다.

/**
 * 로컬에 저장되는 데이터를 갖고 있는 스토리지
 */
interface LocalStorage {

    suspend fun delete(key: String)

    suspend fun putInt(key: String, value: Int)

    suspend fun getInt(key: String, defaultValue: Int): Int

    suspend fun putString(key: String, value: String)

    suspend fun getString(key: String, defaultValue: String): String

    suspend fun hasKey(key: String): Boolean

    suspend fun getBoolean(key : String, defaultValue: Boolean) : Boolean

    suspend fun putBoolean(key : String, value : Boolean)

}

간단하게나마 확장성을 가져오기 위해 로컬에 저장되는 데이터는 다음과 같은 인터페이스를 구현한다.
물론 기존 SharedPreferences를 기반으로 작성된거라 String형식의 키값에 의존한다는 문제가 있긴 했다.

이 인터페이스를 DataStore에 맞게 수정하는 방법도 있지만, 기존 방식이 좀더 사용하기 편한듯 하여, 이번에는 기존 인터페이스를 따라가기로 했다.

/**
 * PreferenceDataStore로 구현한 스토리지.
 * 기존 LocalDataStorage를 사용하기 위해 global한 keyStore 사용
 *
 * @property context application Context
 * @property globalKeyMap String을 key로 하는 글로벌한 키맵. 키의 제네릭 정보가 묻히긴 하지만, 적절하게 get할때 복호화 하기
 */
open class PreferenceDataStore(private val context: Context) : LocalStorage {

    //global key map
    private val globalKeyMap: MutableMap<String, Preferences.Key<*>> = mutableMapOf()

    init {
        // 20:54 - globalKeyMap은 메모리상에 존재하므로 앱 종료후 다시 시작되면 초기화됨..
        // class init시 파일에 저장된 키값을 메모리에 로드하기
        runBlocking {
            val keySet = context.dataStore.data.map { it.asMap().keys }
                .firstOrNull() ?: return@runBlocking //키 - 값 리스트 불러오기. 없을경우 return
            val keyPairMap = keySet.map { it.name to it } //name - key 매핑
            globalKeyMap.putAll(keyPairMap) //맵에 집어넣기
        }
    }

    override suspend fun delete(key: String) {
        val preferenceKey = globalKeyMap[key] ?: return
        context.dataStore.edit { preference -> preference.remove(preferenceKey) }
    }
 

    override suspend fun putString(key: String, value: String) {
        putObject(key, value)
    }

    override suspend fun getString(key: String, defaultValue: String): String {
        return getObject(key) ?: defaultValue
    }

    override suspend fun hasKey(key: String): Boolean {
        return true == context.dataStore.data.map { it.asMap().keys.map { key -> key.name } }
            .firstOrNull()
            ?.contains(key)
    }

    // object 가져올때 map에 저장된 키값 가져옴. 캐스팅 실패거나 값이 없을경우 null 리턴
    // unchecked cast... 제네릭정보가 런타임에는 날아가므로, Cast Exception 발생
    private suspend fun <T> getObject(key: String): T? {
        val preferenceKey: Preferences.Key<*> = globalKeyMap[key] ?: return null
        val result: T? = context.dataStore.data.map { preference ->
            // casting exception 발생한 이유. T가 Int인데 String값이 들어오면 map 함수에서도 Int로 캐스팅 시도 -> 에러.
            // 따라서 Any로 우선 받고, as?를 이용해서 안전한 캐스팅 수행
            // 17:23 - get할때 얘도 uncheckedCasting 수행함.
            val value = preference.get(preferenceKey as Preferences.Key<Any>)
            value as? T// 캐스팅 예외 대비 as? 사용
        }.firstOrNull()

        return result
    }

    // string key값을 value값에 따라 Preference Key로 적절히 변환한 후 global 한 키스토어 및 data store에 저장
    private suspend fun <T> putObject(key: String, value: T) {
        val preferenceKey: Preferences.Key<T> = providePreferenceKey(key, value) as Preferences.Key<T> //type recasting = 하지 않으면 Any로 받아서 수행
        context.dataStore.edit { preference -> preference[preferenceKey] = value } //update
        globalKeyMap[key] = preferenceKey //insert key
    }

    // value값에 따라 적합한 키 제공
    private fun <T> providePreferenceKey(key: String, value: T): Preferences.Key<*> {
        return when (value) {
            is Int -> intPreferencesKey(key)
            is Double -> doublePreferencesKey(key)
            is Float -> floatPreferencesKey(key)
            is Boolean -> booleanPreferencesKey(key)
            is String -> stringPreferencesKey(key)
            is Long -> longPreferencesKey(key)
            is List<*> -> stringSetPreferencesKey(key)
            else -> throw IllegalArgumentException("NOT SUPPORTED") //미지원 value 값
        }
    }
}

private val Context.dataStore: DataStore<Preferences> by preferencesDataStore(name = "mdm_storage")

뭔가 열심히 노력해서 제네릭 부분을 해결하려 했으나, 막상 타입정보는 런타임에 소거되기 때문에 여전히 몇몇 문제가 남아있긴하다.
(String 저장소에서 Int를 가져와달라 요청하면 as?를 넣었음에도 catch되지 않고 CastException 발생등등..)

• 이거는 다음날 프로젝트 진행하면서 inline과 reified 키워드를 붙여줘서 제네릭타입 살려줬더니 잘 작동함 (inlining되면서 제네릭 정보 살려준듯)

기본적으로는 넣을때는 Preferences Key에 알맞게 넣어주고, 가져올때는 Object형식으로 가져온 뒤, as? 연산자를 이용해 안전한 타입 캐스팅을 시도해 리턴하는 방식이다.
또한, 키값을 담고 있는 map도 일개 변수이므로 맨 처음 클래스가 로드될때 map에 다시 키값을 넣어주는 과정을 거치고 있다.
그래도 기존 인터페이스가 원하는 조건대로 작동하고 있어 마음에 들었다.

MVVM with Clean Architecture

소개는 이전 글에서도 많이 했으니 스킵

왜 쓰고 있냐

기존 리팩토링 전 코드를 보면 MVP는 커녕 MVC도 지키지 않은 코드들이 많았다.
MainActivity가 사실상 Controller로써 볼수는 있다만, 그래도 최소한 기능의 분리는 지켰어야 했다.

그렇게 코드를 어떻게 구성해야 할지 고민될때, MVVM과 클린 아키텍처를 보면서 최소한의 틀을 잡을 수 있었다.
기본적인 데이터, 주체를 나타내는 모델 / 사용자에게 나타나는 뷰 / 모델의 정보를 읽기/수정 및 뷰에게 전달하고 값을 가져오는 뷰모델

이 세가지를 토대로 설계되는 앱이 깔끔하게 보여 마음에 들었다.
추가로 클린 아키텍쳐의 UseCase - Repository - Data Layer 순으로 책임을 분리시킬 수 있는점도 좋았다고 해야할까

단편적인 예로 기존에는 RoomDB를 사용하고 있는데, 백엔드 개발자가 이제부터는 서버에서 관리하라고 말한다면 어떻게 해야할까?
기존 방식이라면 서버에 해당되는 Repository와 DAO를 새로 만들고, 기존에 사용되던 코드를 전부 고쳐야할 필요가 있었다.

하지만 클린 아키텍처를 사용한다면 UseCase 수준까지 분리를 수행했기 때문에, 그저 기존 인터페이스를 구현한 RemoteRepository와 DAO를 만들고, hilt 주입시 UseCase에 RemoteRepository만 제공한다면 끝난다.

단점

단점이라기 보단, 내가 개발하면서 느낀점을 꼽자면

개발시간이 좀 길어졌다.

기존 MVC의 경우 컨트롤러 (사실상 MainActivity..)에서 바로 모델에 접근하고 업데이트를 수행하기 때문에 상대적으로 바로바로 작성할 수 있었다.
하지만 MVVM은 뷰모델도 만들어야 하고, repository도 만들고 usecase도 만들어야 해서 코드 작성이 상대적으로 더뎌지게 느껴지긴 했다.
근데, 저렇게 바로바로 짜여진 코드는 나중에 수정할 일 생기면 빡세다는게 문제였다

지금 작성될 코드가 어떤 layer에 속해야할지 헷갈린다.

위에 FT도 이런 상황을 겪었는데, Activity, Fragment는 당연히 View에 속하지만, Service는 어떤 레이어에 속하는걸까?
당연히 Context를 가지고, 사용자의 입력/출력을 담당하는 Service이니 View로 봐야겠지? 라고 생각할 무렵, 다른 프로젝트에서는 GPS, 블루투스 서비스는 Dao로써 접근하고 Repository를 거쳐 사용했었다는 생각이 들었다.

그래서 이걸 View에 넣어야 할지 Dao로써 접근해야할지 한참 고민하곤 했었다.
결론은 일부 극 소수의 서비스 (GPS, 센서, 블루투스, WIFI)같이 정보를 가져오고 가공하는 경우에만 DAO로써 사용되고, 나머지는 뷰로써 보는게 맞다는 정보를 구글선생님과 GPT선생님으로부터 얻어냈다.

장점

테스트 용이성

요새 트렌드가 되어버린 TDD, 나도 하고싶다! 하면서 적용한 프로젝트는 맨 처음부터 테스트 코드 작성에서 큰 난황을 겪는다.

class CoolService {
	private val hotLogic = HotLogic()
    private val serveLogic = ServeLogic()
    
    fun service() : SpecificRet {
    	val discount = DiscountLogic()
        hotLogic.invoke(ColdLogic())
        return serveLogic.invoke(discount.sale())
    }
}

물론 이런 끔찍한 코드를 실제로 만나볼 기회는 거의 없다.. 아닌가?
이런 코드를 어떻게 유닛 테스트 할 수 있을까?
분명히 책이랑 고대 서적같은 tistory에서는 mock을 해서 모의 객체를 만들고, 얘를 호출하는지 확인하라 했었는데..
어디다 넣지...??

이런 문제가 빈번하게 일어나곤 했었다.
이 문제를 MVVM Clean Arcitecture에서는 해결이 가능하다.

class CoolService(private hot : HotLogic, private serve : ServeLogic) {
	
    fun service 
    ~~~~
}

• Module

@Provide
@Singleton
fun provideService(hot : HotLogic, serve : ServeLogic) : CoolService {
	return CoolService(hot, serve)
}

기존 Composition과 같이 내부에서 생성하는 방법보단, DI를 적용할 수 있는 생성자 주입을 통한 Aggregation 방식으로 수정하면 된다.
이러면 hot과 serve는 mockking이 가능하므로 테스트를 수행할 수 있게 된다. 물론 discount부분도 분리를 시도하거나, 로직을 변경해야 하겠지만.

이처럼 hilt를 이용해 DI를 사용한다면 테스트를 한결더 수행하기도 좋아지고, 추후 코드가 수정되더라도 composition에 비해 결합도가 낮은 방식이므로 수정이 용이하기도 하다.

근데 MVVM과 뭔 상관?

MVVM 클린 아키텍처는 위에서 말했다시피 usecase - repository - dao 순으로 각각 분리되어 있기도 하고, VM을 사용한다고 했다.
이 점이 테스트의 용이성을 엄청 증진시켜준다.
dao와 repository가 vm과 분리되어 있으므로, 각각의 책임을 분리해서 테스트할 수 있다.
예를 들어 repository는 단순히 서버로부터 api 요청 결과를 리턴하는 부분만 유닛 테스트 할 수 있다는 것이다. 뷰는 신경안쓰고.

그리고 vm은 이제 각 유닛 테스트가 완료된 repository - usecase를 이용해서 통합 테스트를 수행하면 되는 부분이다.
이때, hilt의 생성자 DI를 이용한다면? - mockking도 가능하다.

@HiltViewModel
class MyViewModel @Inject constructor(private val loadDataUseCase : LoadDataUseCase ...) 

확장성 있는 설계에 입문

기본적으로 클린 아키텍처는 repository 패턴을 이용해 확장성 있는 설계와, usecase를 이용한 변화에 대응하는 로직을 기본적으로 갖추고 있다.
또한, 생성자 DI를 이용하기 때문에, 각 클래스간 결합도가 기존 방식에 비해 낮다. (독립적인 LifeCycle)

이런 코드를 자주 접하다 보니, 기존 static / companion을 이용한 Utility class도 인터페이스로 분리하고, 구현체를 만드는 방식으로 설계할 순 없을까? 라는 생각이 종종 들고, 이러한 생각은 더 나은 코드의 결과물로써 나타나곤 했다. ( 위 NDM 암호화 부분등)

뷰의 입력값을 검증하더라도, '이 검증 로직은 추후에 VM에서도 쓰일것 같은데?', '이 검증하는 클래스는 인터페이스로 분리하면 다른 뷰 검증시에도 도움이 되지 않을까?' 라는 생각으로 확대되어 중복된 코드를 줄이고 더 보기 좋은 코드를 작성하는데에도 도움이 되었다.
그래서 나온 결과물이 PredictValidator와 제네릭을 활용한 ViewBindingValidator<T : ViewBinding>

결론

아직까지는 한참 부족한 개발자니까 좀더 자료도 많이 찾아보고, 여러 의견을 들어보면서 최선을 다한다면 뭔가 더 나아지지 않을까 라는 생각을 종종 하곤 한다.

그래도 2년전 코드를 보니 그 사이에 많이 발전했구나 싶기도 하다. ㅋㅋㅋ
대체 2년전의 나는 왜 그렇게 코드를 짠것인가