이 글은 2024년 6월 20일에 일본어로 먼저 발행된 기사를 번역한 글입니다.
LY Corporation은 높은 개발 생산성을 유지하기 위해 코드 품질 및 개발 문화 개선에 힘쓰고 있습니다. 이를 위해 다양한 노력을 하고 있는데요. 그중 하나가 Review Committee 활동입니다.
Review Committee에서는 머지된 코드를 다시 리뷰해 리뷰어와 작성자에게 피드백을 주고, 리뷰하면서 얻은 지식과 인사이트를 Weekly Report라는 이름으로 매주 공유하고 있습니다. 이 Weekly Report 중 일반적으로 널리 적용할 수 있는 주제를 골라 블로그에 코드 품질 개선 기법 시리즈를 연재하고 있습니다.
이번에 블로그로 공유할 Weekly Report의 제목은 '(투명한) 운명의 붉은 실'입니다.
(투명한) 운명의 붉은 실
다른 사용자가 보낸 '메시지'를 표시하는 기능을 구현한다고 가정해 봅시다. 다음 MessageModel과 MessageContent는 해당 '메시지'를 표현하는 데이터 모델입니다. 이 데이터 모델들은 UI 이외의 레이어에서도 사용하므로 UI만을 위해 정의를 변경하는 것은 부적절합니다.
class MessageModel(
val content: MessageContent,
val timestampMillis: Long,
val senderUserId: UserId,
...
)
sealed class MessageContent {
data object Empty: MessageContent()
data class Error(val errorType: ErrorType): MessageContent()
data class Text(val text: String): MessageContent()
data class Event(val eventType: EventType): MessageContent()
}
메시지를 표시하는 UI 레이아웃은 다음과 같이 구현했습니다(참고로 error는 런타임 에러를 발생시키는 Kotlin 표준 함수입니다).
fun bindToViews(message: MessageModel) {
val isVisible = isContentValid(message.messageContent)
layoutContainer.isVisible = isVisible
if (!isVisible) {
return
}
timestampTextView.text = formatter.format(message.timestampMillis)
... // 기타 뷰 업데이트 로직
contentTextView.text = getMessageText(message.content)
}
private fun isContentValid(content: MessageContent): Boolean = when (content) {
MessageContent.Empty,
is MessageContent.Error -> false
is MessageContent.TextMessage,
is MessageContent.EventMessage -> true
}
private fun getMessageText(content: MessageContent): String = when (content) {
MessageContent.Empty,
is MessageContent.Error -> error("Invalid message type as `messageText`")
is MessageContent.TextMessage -> content.text
is MessageContent.EventMessage -> content.eventType.toMessageString()
}
위 코드에 문제가 있을까요?
한 배를 탔다면 함께 하자
위 코드에서는 getMessageText 구현이 암묵적으로 isContentValid와 연관돼 있어 다음과 같은 문제가 발생합니다.
isContentValid가true를 반환할 때만getMessageText를 호출할 수 있다고 전제하고 있지만, 이를 위반해도 런타임 에러로만 감지할 수 있다.- 타입이 추가되거나 업데이트될 때
isContentValid와getMessageText간의 동작 일관성을 확인해야 한다.
bindToViews 구현만 보면 isContentValid와 getMessageText의 연관성을 눈치채지 못할 수 있는데요, 이 연관성을 놓치면 리팩터링이나 기능 확장 시 getMessageText를 잘못 사용해도 알아차리기 어렵습니다.
이와 같은 함수 간의 암묵적 연관성은 함수를 하나로 합쳐 해결할 수 있습니다. 현재 코드에서 isContentValid는 필터로, getMessageText는 변환으로 구현돼 있지만 근본적인 로직은 공통적입니다. 따라서 이 두 가지 역할을 getMessageText에 통합해 보겠습니다.
/**
* 메시지 콘텐츠[content]를 표시 가능한 텍스트 표현으로 변환하여 반환한다.
* 단 표시할 수 없는 콘텐츠 유형(error/empty)인 경우 null을 반환한다.
*/
private fun getMessageText(content: MessageContent): String? = when (content) {
MessageContent.Empty,
is MessageContent.Error -> null
is MessageContent.TextMessage -> content.text
is MessageContent.EventMessage -> content.eventType.toMessageString()
}
호출자는 다음과 같습니다.
fun bindToViews(message: MessageModel) {
val messageText = getMessageText(message.messageContent)
layoutContainer.isVisible = messageText != null
if (messageText == null) {
return
}
timestampTextView.text = formatter.format(message.timestampMillis)
... // 기타 뷰 업데이트 로직
contentTextView.text = messageText
}
이번에는 유효하지 않은 콘텐츠 유형을 나타내기 위해 null을 사용했지만, 이 사양은 getMessageText가 널리 사용될 경우 혼란을 초래할 수 있습니다. 그런 경우에는 isContentValid를 재정의하는 것이 좋습니다. 다만 그때는 isContentValid와 getMessageText의 연관성을 명시하는 방식으로 구현해야 합니다. 예를 들어 isContentValid는 독립된 함수가 아닌 getMessageText를 사용해 구현할 수 있습니다.
private fun isContentValid(content: MessageContent): Boolean =
getMessageText(content) != null
/** ... */
private fun getMessageText(content: MessageContent): String? = when (content) {
MessageContent.Empty,
is MessageContent.Error -> null
is MessageContent.TextMessage -> content.text
is MessageContent.EventMessage -> content.eventType.toMessageString()
}
이렇게 하면 단독으로 getMessageText를 사용해도 안전하고, 더불어 isContentValid와 getMessageText 간의 일관성을 보장할 수 있는 구현이 됩니다.
한 줄 요약: 함수 간에 암묵적인 연관성이 있을 경우 함수를 하나로 합치거나 연관성이 명확한 구현으로 변경한다.
키워드:
conditional branch,function precondition,implicit dependency