코드 품질 개선 기법 11편: 반복되는 호출에 함수도 지친다

이 글은 2024년 2월 1일에 일본어로 먼저 발행된 기사를 번역한 글입니다.

안녕하세요. 커뮤니케이션 앱 LINE의 모바일 클라이언트를 개발하고 있는 Ishikawa입니다.

저희 회사는 높은 개발 생산성을 유지하기 위해 코드 품질 및 개발 문화 개선에 힘쓰고 있습니다. 이를 위해 다양한 노력을 하고 있는데요. 그중 하나가 Review Committee 활동입니다.

Review Committee에서는 머지된 코드를 다시 리뷰해 리뷰어와 작성자에게 피드백을 주고, 리뷰하면서 얻은 지식과 인사이트를 Weekly Report라는 이름으로 매주 공유하고 있습니다. 이 Weekly Report 중 일반적으로 널리 적용할 수 있는 주제를 골라 블로그에 코드 품질 개선 기법 시리즈를 연재하고 있습니다.

이번에 블로그로 공유할 Weekly Report의 제목은 '반복되는 호출에 함수도 지친다'입니다.

반복되는 호출에 함수도 지친다

어떤 서비스에서 사용자끼리 '친구'가 되는 기능을 구현하려고 합니다. 현재 사용자 입장에서 다른 사용자가 친구인지 확인하거나 새로 친구를 추가하는 기능을 구현하기 위해 다음과 같은 UseCase 클래스를 사용한다고 가정해 봅시다.

class FriendStateUseCase(
    private val currentUserId: UserId,
    ...
) {
    fun isFriend(userId: UserId): Boolean {
        ...
    }

    fun markAsFriend(userId: UserId) {
        ...
    }
}

호출자 측 코드는 다음과 같습니다.

if (!friendStateUseCase.isFriend(someUserId)) {
    friendStateUseCase.markAsFriend(someUserId)
}

위 코드에 문제가 있을까요?

여러 번 묻지 않기

호출자 측 코드의 구조를 추출하면 if (receiver.a()) { receiver.b() }가 됩니다. 즉, '현재 수신 객체의 상태에 따라 수신 객체를 변경'하는 코드가 되는데요. 이렇게 '현재 수신 객체의 상태에 따라 수신 객체를 변경'하는 경우 상태를 변경하는 함수 자체에서 확인하도록 만드는 것이 더 좋은 경우가 많습니다.

위 코드에서 friendStateUseCase.markAsFriend의 경우 이미 !isFriend를 충족하는 것으로 가정하는 것 같습니다. 만약 사용자가 이미 친구인 상태에서 markAsFriend를 호출하면 예외가 발생하는 등 !isFriend를 꼭 확인해야 하는 경우에 확인이 누락된다면 버그가 발생할 것입니다. 이를 해결하려면 !isFriend 확인을 markAsFriend 내부에서 수행하는 것이 좋습니다.

fun markAsFriend(userId: UserId) {
    if (isFriend(userId)) {
        return
    }

    // Actual logic to mark `userId` as a "friend".
}

이미 친구 상태일 때 markAsFriend가 아무것도 하지 않는 것은 자연스러운 동작으로 보이는 경우가 많습니다. 하지만 '아무것도 하지 않는다'는 것을 특별히 강조해야 한다면 함수 이름을 markAsFriendIfNotYet으로 바꾸거나 주석으로 조건을 설명해야 합니다.

이렇게 수신 객체에게 자신의 상태를 확인하도록 하면 굳이 보여줄 필요가 없는 상태를 숨기고 겉으로 드러나는 상태 전이를 단순화할 수 있습니다.

반환값으로 물어보기

다음 코드를 리팩터링하는 것을 생각해 봅시다.

if (!friendStateUseCase.isFriend(someUserId)) {
    friendStateUseCase.markAsFriend(someUserId)
    showEventPopup(Event.NEW_FRIEND) // To notify user the operation was successfully finished.
}

위 코드를 다음과 같이 리팩터링하는 것은 부적절합니다.

class FriendStateUseCase(...) {

    fun markAsFriend(
        userId: UserId,
        onSucceeded: () -> Unit
    ) {
        if (isFriend(userId)) {
            return
        }

        ... // Actual logic to mark `userId` as a "friend".
        onSucceeded()
    }
}

// Caller 
friendStateUseCase.markAsFriend(someUserId) {
    showEventPopup(Event.NEW_FRIEND)
}

FriendStateUseCase.markAsFriend의 리팩터링이 부적절한 이유는 onSucceeded 콜백으로 인해 불필요한 의존성 순환이 발생하기 때문입니다. 또한 언뜻 보기에는 onSucceeded가 동기식 콜백인지 비동기식 콜백인지 알 수 없다는 점도 문제입니다.

이런 경우 markAsFriend를 고차 함수로 만드는 것보다 반환값으로 결과를 반환하는 것이 좋습니다. 현재 예시의 경우에는 진위값(Boolean)으로 충분합니다.

/** ... */
fun markAsFriend(userId: UserId): Boolean {
    if (isFriend(userId)) {
        return false
    }

    ... // Actual logic to mark `userId` as a "friend".
    return true
}

// Caller
val isNewlyMarkedAsFriend = friendStateUseCase.markAsFriend(someUserId)
if (isNewlyMarkedAsFriend) {
    showEventPopup(Event.NEW_FRIEND)
}

이렇게 하면 showEventPopup(Event.NEW_FRIEND)이 언제 호출되는지 명확하게 알 수 있습니다. 다만 이렇게 함수 이름에서 반환값을 언급하지 않는 경우에는 문서에서 반환값을 설명해야 합니다(자세한 내용은 Code readability: Session 3 문서를 참고하세요). 또한 필요하다면 호출자가 반환값을 확인하도록 강제하세요(코드 품질 개선 기법 2편: 확인 여부를 확인했나요?의 첫 번째 방법 참고).

---

한 줄 요약: 수신 객체의 상태를 확인하는 코드는 해당 수신 객체의 함수 내에서 실행하는 것이 더 좋은 경우도 있다.

키워드: state check logic, responsibility, implicit precondition