라인의 코드 품질 개선 기법 리포트를 읽고

 단순한 것들도, 단순하지 않은 것들도 클린 코드를 위한 내용이 담겨 있는거 같아서 아티클을 보며 정리했다.

개선 1편 : 한 번 엎지른 error는 다시 주워 담지 못한다

링크

문제가 있는 코드

에러 전파 방법은 호출자가 해당 에러를 어떻게 처리하는지 따라 달라진다.

@Throws(IllegalArgumentException::class)  
fun parseFooValue(inputText: String): FooParseResult {  
    val matchResult = FOO_FORMAT_REGEX.matchEntire(inputText)  
    requireNotNull(matchResult) // Throws if `inputText` format is invalid  
  
    val fooIntValue = matchResult.groupValues.getOrNull(1)?.toIntOrNull()  
    return if (fooIntValue != null) {  
        FooParseResult.Success(fooIntValue)  
    } else {  
        FooParseResult.InvalidRegexError  
    }  
}

/** Result model of parsing "FOO" integer from string value. */  
sealed class FooParseResult {  
    /** A result representing "FOO" data is correctly parsed as an integer. */  
    class Success(val value: Int) : FooParseResult()  
  
    /** An error result representing the parsing regex implementation is incorrect. */  
    data object InvalidRegexError : FooParseResult()  
}

매우 방어적인 코드, 누군가 Regex 를 잘못 작성하는 경우까지 처리를 해주는 것 ( "foo:\d{1,6}" 와 같이 그룹이 없지만, 정규식을 만족하게 할 때 )

• 일반적으로 호출자가 제공한 인수는 신뢰할 수 없다 라고 생각해야 더 견고한 코드를 작성할 수 있다. -> 사용자가 입력하거나, 외부 시스템에 제공 받은 것일 수 있기 때문

• 호출자는 에러를 신경 쓸 필요 없으며, 복구 할 수 없는 에러여야 한다. -> 정규 표현식 실수는 로직 에러이나, sealed 클래스를 통해 호출자 처리하도록 강제

=> 에러 처리하는 방법과 에러 표현하는 방법이 서로 맞지 않다!

복구 (불)가능 수준

복구 가능

0. 기본값
1. 단순 도메인 에러
2. 에러값 포함한 합 타입, 널어블(nullable), 에러값
3. 확인된 예외
4. 확인되지 않은 예외
5. 캐치할 수 없는 에러

복구 불가능

기본값 : 호출자가 에러 발생 여부 파악하지 않아도 되는 경우 사용

"", 0, 1 등등

userProvider.getUsers() // If some error happens, `getUsers` returns empty

단순 도메인 에러 : 에러가 발생한다는 사실은 알아야 하지만, 에러 내용까지 알 필요 없을 시

null, Optional, undefined 등등

val user = userProvider.getUser(id) ?: return // If some error happens `getUser` returns null.

반환값이 없는 경우에는 진위값을 사용 가능 - false 로 에러, true 로 성공 판단 타입 안정성이 확보된 도메인 에러 사용 ( Kotlin 의 null 등 )

에러값 포함한 합 타입이나 널러블 에러값

Either, Result sealed 등등

sealed class FooParseResult {  
    class Success(val value: Int) : FooParseResult()  
  
    data object InvalidRegexError : FooParseResult()  
}

// val result = FooParseResult() 가 불가능

여기서 open 으로 바꾸면, 외부에서 생성 가능해서 sealed 를 사용해야 한다.

정상적인 상태에서 반환값이 필요 없는 경우라면, 널러블 에러값 반환하는 경우도 있으나, 합 타입을 통해 좀 더 명확하게 표현 가능하다.

확인된 예외

이론적으론 합 타입에서 사용하는 것과 동일하나 ( InvalidRegexError ) 실제 구현에선 타입을 엄격히 다루기 어렵다.

합 타입보다 더 복구하기 어려운 유형에 사용해야 한다.

-> Java 는 Exception 이라는 예외 아래 스택 정보등 많은 정보를 포함시켜 사용한다.

확인되지 않은 예외

처리 시스템 에러나 로직 에러 같이 복구 불가능 한 에러에 사용해야 한다. ( 호출자가 에러 발생한 가능성 자체를 간과할 수 있기 때문 )

-> Java 의 RutimeException

캐치할 수 없는 에러

확인되지 않은 예외와 거의 유사하나, 빠른 실패를 엄격히 구현

엎지른 물을 다시 주워 담지 말자

fun parseFooValue(inputText: String): Int? {
    val matchResult = FOO_FORMAT_REGEX.matchEntire(inputText)
        ?: return null
    return matchResult.groupValues[1].toInt()
}

• 잘못된 입력에 대해선 null 반환
• 정규 표현식 구현 실수에는 확인되지 않은 예외 사용
  • getOrNull -> get
  • toIntOrNull -> toInt ( 이미 검증후, return 으로 빠른 실패 )

에러 복구 가능 여부는 호출자의 코드와 에러 처리 범위에 따라 달라진다. ( 쿼리 로직 자체는 복구 불가능하나, 서버 프로세스 관점에선 복구 가능일시 )

호출자 코드가 결정되지 않고, 복구 가능 여부 판단할 수 없으면 다루기 쉬운 방식으로 반환 후 -> 다른 에러로 변환하는 것도 고려 가능하다.

개선 2편 : 확인 여부를 확인했나요?

링크

문제가 있는 코드

fun caller() {
    ... // snip
    val cappedProgress = progress.coerceAtMost(1F)
    showProgressBar(cappedProgress)
}

fun showProgressBar(progress: Float) {
    val progressInRange = progress.coerceAtLeast(0F)
    ... // snip
}

값이 [0,1] 범위인지 확인하는 소재가 명확하지 않다.

상한값은 호출자가 지정, 하한값은 호출 대상이 지정 -> 잘못 사용하기 쉬워지고, 버그 발생도 높아진다.

믿을 수 있는 건 자기 자신

호출 대상 내에서 확인하는 것 확인되지 않은 상태를 타입 안전하게 처리할 수 없는 상황에서 효과적

여전히 제공되는 인수는 신뢰할 수 없는 것

fun showProgressBar(progress: Float) {
    val progressInRange = progress.coerceIn(0F, 1F)
}

호출 대상이 명확히 보장한다.

fun showProgressBar(progress: Float): Boolean {
    if (progress !in 0F..1F) {
        return false
    }
    return true
}

제대로 처리되었는지 여부를 알려주는 것도 가능하다.

타입 안전 ‘검사 필증’

값이 특정 범위에 속한다는 걸 보장하는 타입을 만들어 올바른 값만 전달되도록 만드는 것

class ProgressRatio(rawValue: Float) {
    val value = rawValue.coerceIn(0F..1F)
}

fun showProgressBar(progressRatio: ProgressRatio)

오류 처리를 호출자에서 하고 싶다면?

class ProgressRatio private constructor(val value: Float) {
    companion object {
        fun of(value: Float): ProgressRatio? =
            if (value in 0F..1F) ProgressRatio(value) else null
    }
}

타입 안전한 값을 사용해 견고한 코드를 만든다. null 및 처리를 통해 명확하게 사용 가능하다. ( 확인되지 않은 예외를 사용하면, 오히려 자세한 내용을 알지 못하면 사용할 수 없는 클래스가 된다. )

개선 3편 : 전략 없는 전략

링크

문제가 있는 코드

interface Loggable {
    val logType: LogType
    val logLevel: LogLevel
    val logDescription: String
    val timestamp: Long
    val codeLocation: StackTraceElement
    ...
)

로그 출력 정보를 담고 있는 인터페이스

enum class LogAttribute { LOG_TYPE, LOG_LEVEL, LOG_DESCRIPTION, ...}

fun createLogMessage(loggable: Loggable, attributesToLog: Set<LogAttribute>): String {
  ...

입력된 LogAttribute 에 따라 어떤 속성을 사용해 메시지를 구성할지 결정한다.

여기서, 로그 레벨은 메시지 시작 부분에 위채햐아 한다. 라는 규칙이 있다면?

val ORDERED_ATTRIBUTES_TO_LOG: List<LogAttribute> = listOf(
  LOG_LEVEL,
  LOG_TYPE,
  LOG_DESCRIPTION,
  ...
)

fun createLogMessage(loggable: Loggable, attributesToLog: Set<LogAttribute>): String =
    ORDERED_ATTRIBUTES_TO_LOG.asSequence()
        .filter(attributesToLog::contains)
        .map { attribute ->
            when (attribute) {
                LogAttribute.LOG_LEVEL -> getLogLevelText(loggable)
                LogAttribute.LOG_TYPE -> getLogTypeText(loggable)
                ...
            }
        }.joinToString()

ORDERED_ATTRIBUTES_TO_LOG 를 만들어서 로그 메시지를 구성한다.

반복문 내부에 분기가 있고, 분기가 기껏해야 한번만 사용된다. ( 반복되는 ‘고도의 유연성’ )

• 분기를 반복문 내부에 직접 작성해서 함수 흐름을 파악하기 위해선 각 분기를 파악해야 한다.
• 타입의 순서와 분기를 대응시키기 어렵다.

첫 번째 방법: 반복문 제거하기

val message = StringBuilder()
if (attributesToLog.contains(LogAttribute.LOG_LEVEL)) {
    message.append(getLogLevelText(loggable))
}
if (attributesToLog.contains(LogAttribute.LOG_TYPE)) {
    message.append(getLogTypeText(loggable))
}
...

타입을 나타내는 컬렉션이 충분히 작을 시에는 if로 직접 분기

val message =
    loggable.getAttributeTextOrEmpty(attributesToLog, LogAttribute.LOG_LEVEL, ::getLogLevelText) +
    loggable.getAttributeTextOrEmpty(attributesToLog, LogAttribute.LOG_TYPE, ::getLogTypeText) +
    ...

private fun Loggable.getAttributeTextOrEmpty(
    attributesToLog: Set<LogAttribute>,
    targetAttribute: LogAttribute,
    attributeTextCreator: (Loggable) -> String
): String = if (targetAttribute in attributesToLog) attributeTextCreator(this) else ""

보조 함수를 만들어서, 명확히 연결

하지만, 단점 두가지가 발생한다.

• 모든 타입이 포함되었는지, 단위 테스트 작성하기 어려움 ( if 문을 통해 처리하므로, IDEA가 경고 띄워주지 않음 )
• joinToString 과 같은 컬렉션 유틸리티 함수 사용 불가능

  두 번째 방법 : 분기 추출

fun createLogMessage(loggable: Loggable, attributesToLog: Set<LogAttribute>): String =
    ORDERED_ATTRIBUTES_TO_LOG.asSequence()
        .filter(attributesToLog::contains)
        .map { attribute -> attribute.getLogText(loggable) }
        .joinToString()

private fun LogAttribute.getLogText(loggable: Loggable): String = when(this) {
    LogAttribute.LOG_TYPE -> ...
    ...
}    

조건 분기를 보조 함수로 추출

가독성은 개선되나, ORDERED_ATTRIBUTES_TO_LOG 와 getLogText 간 대응이 여전히 어렵다. 추가로, 분기에 else 사용시 포괄성 보장을 하지 못한다. ( 추가되어도, 경고 및 예외 발생시키지 않음 )

세 번째 방법 : 로직을 타입에 포함하기

전략 패턴이나 유사한 구조를 적용해 각 타입에 고유 로직 포함 가능

enum class LogAttribute {
  LOG_TYPE {
    override fun getLogText(loggable: Loggable) = ...
  },
  LOG_LEVEL {
    override fun getLogText(loggable: Loggable) = ...
  },
  ...;

  abstract fun getLogText(loggable: Loggable)
}

fun createLogMessage(loggable: Loggable, attributesToLog: Set<LogAttribute>): String =
    ORDERED_ATTRIBUTES_TO_LOG.asSequence()
        .filter(attributesToLog::contains)
        .map { attribute -> attribute.getLogText(loggable) }
        .joinToString()

분기가 보이지 않으므로 함수 흐름 쉽게 파악 가능 ( + 포괄성 보장 ) 로직 구현이 누락되면 컴파일 오류 발생 + ORDERED_ATTRIBUTES_TO_LOG 가 모든 요소 포함하는지 단위 테스트도 쉽게 작성 가능

네 번째 방법 : 관계를 명시하는 튜플 만들기

class AttributeTextModel(val attributeType: LogAttribute, val textCreator: (Loggable) -> String)

타입, 타입 순서, 로직 연관성 명시를 위해 튜플을 만든다.

val ORDERED_ATTRIBUTES_TO_LOG: List<AttributeTextModel> = listOf(
    AttributeTextModel(LogAttribute.LOG_LEVEL, ::getLogLevelText),
    AttributeTextModel(LogAttribute.LOG_TYPE, ::getLogTypeText),
    ...
)

fun createLogMessage(loggable: Loggable, attributesToLog: Set<LogAttribute>): String =
    ORDERED_ATTRIBUTES_TO_LOG.asSequence()
        .filter { attributesToLog.contains(it.attributeType) }
        .map { it.textCreator(loggable) }
        .joinToString()

타입 순서가 정의되어 있다면 로직 attributesToLog 도 구현되어 있는걸 보장 가능하다.

널리 사용되는 타입에도 기능 고유 로직을 연결 가능하다. ( AttributeTextModel 이 대신 받아주므로 )

개선 4편 : 한 번 엎지른 error는 다시 주워 담지 못한다

링크

class IntAdder {
    private var currentSum: Int = 0

    fun add(value: Int) {
        currentSum += value
    }

    fun flush(): Int {
        val result = currentSum
        currentSum = 0
        return result
    }
}

열린 창문만 이용해 테스트하기

내부 세부적인 작동보다 관찰 가능한 작동이 사양과 일치하는지 검증하는 것이 중요하다.

• 함수의 반환값 및 예외
• 외부에서 제공되는 객체와의 상호작용 ( 실제 인수로 입력된 객체, 생성자 인수로 입력된 객체 등 )

// Unit test code
adder.add(100)
adder.add(200)
assertEquals(300, adder.flush())
assertEquals(0, adder.flush())

currentSum 값을 직접 검증하는게 아닌, 함수 반환값을 통해 검증하자.

외부에서 제공되는 객체와의 상호작용 테스트

class IntAdder(private val transactionLogger: TransactionLogger) {
    ...

    fun inTransaction(action: Adder.() -> Unit) { 
        ... // Use `transactionLogger` here
    }
}

val logger: TransactionLogger = mock()
val adder = IntAdder(logger)

adder.inTransaction { add(100) }

inOrder(logger) {
    verify(logger).write(100)
    verify(logger).commit()
}

당연히 외부 제공 객체를 무조건 mock 으로 만들 필요는 X

개선 5편 : # 나쁜 열거가 좋은 계층을 몰아낸다

링크

enum class AccountType { FREE, PERSONAL, UNLIMITED }

이와같은 열거형이 정의 되어 있을 때 로컬 저장소, DB, 네트워크 통한 API 를 사용해 해당 값 읽고 쓰는 경우 컨버터,매퍼 같은 매커니즘을 사용해 언어별 객체, 프로토콜 정의된 바이트 열 등 상호 변환이 가능하다.

class AccountTypeConverter {
    @TypeConverter
    fun fromStringValue(typeString: String): AccountType = AccountType.valueOf(typeString)

    @TypeConverter
    fun toStringValue(type: AccountType): String = type.name
}

class AccountTypeConverter {
    @TypeConverter
    fun fromIntValue(typeInt: Int): AccountType = AccountType.entries[typeInt]

    @TypeConverter
    fun toIntValue(type: AccountType): Int = type.ordinal
}

이와같이 Converter 를 구현하면?

enum class AccountType { FREE, PERSONAL, BUSINESS, UNLIMITED }

값이 추가됨에 따라 ordinal 은 변경이 되게 된다.

ordinal과 name 을 사용하면 열거 사용측의 편의를 위한 간단한 리팩토링에도 의도치 않은 버그가 발생하게 된다.

부패하지 않도록 랩 씌우기

enum class AccountType(val dbValue: String) {
    FREE("free"),
    PERSONAL("personal"),
    UNLIMITED("unlimited");

    companion object {
        val DB_VALUE_TO_TYPE_MAP: Map<String, AccountType> =
            entries.associateBy(AccountType::dbValue)
    }
}

외부에서 사용하는 값과 열거형 선언을 분리하자. 열거형 이름이나 순서 변경으로부터 외부 사용 값을 보호 가능하다.

예외: 그 자리에서 먹으면 부패하지 않는다

열거형이 외부 사용하는 값으로 변환이 아닌, 임시 변환이라면 사용해도 상관 없다. 단, 이 경우에도 타입 안전성 이점 누리기 위해 가능한 name, oridnal 이 아닌 열거형 자체를 사용하자.

개선 6편 : # 나쁜 열거가 좋은 계층을 몰아낸다

링크

fun ...(contact: ContactModel): ReturnValue? {
    val friendName = (contact as?
            ContactModel.Person)?.takeIf {
        it.isFriend
    }?.let { normalizeEmoji(it.displayName) } ?: return null

    // snip...
    // snip...
}

• ?.takeIf 값이 null 일 시, 아무것도 안하고 null 반환
• ?.takeIf 값이 null 이 아닐 시, 반환값을 사용해 normalizeEmoji 호출후 결과 반환

자르기 전 칼을 갈자.

val friendName = (contact as? ContactModel.Person)
	?.takeIf { it.isFriend }
	?.let { normalizeEmoji(it.displayName) }
	?: return null

부적절한 줄 바꿈 부터 없애자. 로직을 전혀 손 대지 않고, 줄 바꿈 위치 바꾸는 것만으로 가독성을 높인다.

이게, 꽤나 중요한거 같다. ( 제이슨의 강의때도 느꼈지만, 코드를 바로 건드리는건 오히려 더 큰 나비효과를 부를 수 있다. )

의미가 크게 구분되는 곳에서 줄을 바꿔라.

    val friend = (contact as? ContactModel.Person)
        ?.takeIf { it.isFriend }
        ?: return null
    val friendName = normalizeEmoji(friend.displayName)

비로소 normalizeEmoji 를 메소드 체인 외부로 이동해도 괜찮은지를 판단할 수 있다.

다양한 방법의 자르기

메소드 체인 / 폴백 체인

도트 연산자 ( . ) 나 세이프 콜 ( ?. ) 등을 이용한 메소드 체인 또는 엘비스 연산자 ( ?: ) 등 이용한 폴백 체인시 코드 세부적 부분보다 로직의 구조 흐름이 더 중요한 경우도 많다.

위 연산자들 바로 앞에 줄 바꿈을 넣자.

    val ... = nullable?.value
        ?: fallback.value
        ?: another.fallback(value)

    val ... = nullable?.value
        ?: fallback.shortcut
        ?: another.fallback(value)
...
private val Fallback.shortcut: ...? get() =
    value.with(long.long.long.long.long.long.long.argument)

인수가 길어지는 경우, 인수 짧아지게 해주는 보조 함수 및 확장 함수 사용해 줄 바꿈 위치도 역시 조정 가능하다.

연산자 우선순위

valueWithLongName1 - valueWithLongName2 ==
    valueWithLongName3 + valueWithLongName4

당연히, '==' 에서 줄 바꾸는 것이 더 가독성이 좋은 코드

valueWithLongName1 *
    (valueWithLongName2 + valueWithLongName3)

'()' 에서 줄 바꾸는 것이 더 가독성 좋은 코드

    val nonNullValue = some.nullable.value.with(parameter)
        ?: return someReturnValue

코드의 영향이 국소적으로 국한된게 아니면, 영향이 강조되도록 줄 바꿈 위치를 신중히 결정해야 한다.

return과 throw 사용시에는 코드 왼쪽에 나타나게 두어 강조할 수 있다.