[Refactoring] if문

Martin Fowler의 저서 Refactoring에는 다양한 "코드의 악취(Code Smell)"에 대해 설명되어 있습니다. Martin Fowler는 코드의 악취만 제거해도 썩 괜찮은 코드가 된다고 하고, 반대로 코드에서 악취가 느껴진다면 이는 시스템 내에 더 깊은 문제가 있음을 의미한다고 이야기합니다. 그러므로 리팩토링은 코드를 작성한 이후 반드시 수행해야 하는 절차라고 말합니다.

 

간혹 리팩토링을 수행하면 성능이 떨어진다고 주장하는 개발자들을 만날 수 있습니다. 리팩토링의 결과로 객체가 분리되거나 메서드 호출의 횟수가 증가하게 되며, 그 결과 실제로 메모리 사용량과 CPU 사용량이 모두 증가하기 때문입니다.

하지만 성능에 치명적인 영향을 주는, 예를 들어 파일 입출력이나 데이터베이스 작업과 같은 I/O 처리가 아닌 한 실제 성능에는 큰 영향을 주지 않습니다. 이에 대해 리팩토링을 하는 것이 더 이득이라고 주장하는 쪽에서는 다음과 같이 설명합니다.

Does an increase in the number of methods hurt performance, as many people claim? In almost all cases the impact is so negligible that it's not even worth worrying about.

Plus, now that you have clear and understandable code, you are more likely to find truly effective methods for restructuring code and getting real performance gains if the need ever arises. - https://sourcemaking.com/refactoring

많은 사람들이 말하는 것과 같이, 메소드 호출 횟수가 증가하면 성능에 악영향을 주지 않을까? (하지만) 거의 모든 경우에 그 영향은 걱정할 필요가 없을 정도로 무시할 만하다.

더욱이, (리팩토링의 결과로) 이제 명확하고 이해 가능한 코드를 얻게 되었기 때문에, 코드를 재구성했을 때 효과적인, 혹은 성능 향상에 더 효과적인 코드가 어떤 것인지 훨씬 찾기 쉬워질 것이다.

 

이 글에서는 if문을 최소화 혹은 구조화하여 읽기 쉽고 유지 보수하기 쉬운 코드를 만드는 방법에 대해 다룹니다. if문은 본질적으로 하나의 메서드 내에 여러 개의 주요 로직을 포함하게 하고(SRP 위반), 코드를 장황하게 하며, 그래서 결과적으로 읽기도 유지 보수하기도 어렵게 합니다. 이 글에서는 if문을 제거하거나 효과적으로 사용하는 가장 단순한 방법들에 대해 다룰 것입니다. if문이 사용된 코드는 잠재적으로 좋지 않은 것으로 간주되는데, 그에 대해서는 Stackoverflow에 소개된 내용을 참고하시기 바랍니다.

 

1. Return with boolean

boolean 논리 값을 리턴하는 메서드의 경우 if-else를 이용하지 않는 것이 바람직합니다. if문에 사용할 조건을 그대로 리턴 값으로 사용할 수 있습니다.

 

[Non-compliant Code]

private boolean isValidName(String name){
    if(name != null && !name.isEmpty()){
        return true;
    } else{
        return false;
    }
}

[Compliant Code]

private boolean isValidName(String name){
    return name != null && !name.isEmpty();
}

 

비슷한 용법으로 boolean 값에 의해 리턴되는 value가 결정되는 경우 if-else 대신 삼항 연산자(Ternary Operator)를 이용하는 것이 바람직합니다.

 

[Non-compliant Code]

private String getUserPhoneNumber(User user){
    if(user.getPhoneNumber() != null){
        return user.getPhoneNumber();
    } else{
        return user.getMobileNumber();
    }
}

[Compliant Code]

private String getUserPhoneNumber(User user){
    return user.getPhoneNumber() != null ? user.getPhoneNumber() : user.getMobileNumber();
}

위의 코드는 아래와 같이 지역변수를 사용하도록 변경할 수 있습니다.

with Local Variable

1 2 3 4

private String getUserPhoneNumber(User user){     String phoneNumber = user.getPhoneNumber();     return phoneNumber != null ? phoneNumber : user.getMobileNumber(); }

분명히 user.getPhoneNumber() 메서드를 두 번 호출하는 것보다 메소드 호출 횟수가 줄어들기 때문에 성능의 향상이 있습니다만 그 효과는 그리 크지 않습니다. "Refactoring"에서는 성능의 큰 향상이 없는 경우 지역변수의 사용을 자제하라고 합니다. 너무 잦은 지역변수의 사용은 결과적으로 리팩토링을 하는 데 도움을 주지 않으며 그것 자체로 코드의 악취를 만드는 것으로 간주됩니다.

다만, user.getPhoneNumber() 메서드의 호출이 데이터베이스나 외부 API를 호출하는 경우에는 반드시 1회만 호출하도록 제어해야 합니다.

 

---

2. Fast Exit - 빠른 탈출

특정 조건에 부합되는 경우에만 메소드의 기능을 수행하고 싶은 경우, "해당 조건에 부합되지 않으면 기능을 수행하지 않는다"로 표현하는 것이 바람직합니다. 

 

[Non-compliant Code]

private void insertUser(User user){
    if(isValidName(user.getName())){
        userDAO.insert(user);
    }
}

[Compliant Code]

private void insertUser(User user){
    if(!isValidName(user.getName())){
        return;
    }
     
    userDAO.insert(user);
}

아래 코드는 윗 코드와 완벽히 동일한 방식으로 동작하며, 유일한 차이점은 기능의 주요 로직이 1 depth(깊이)에서 수행된다는 것입니다. 주요 비즈니스 로직은 항상 낮은 depth에서 수행하도록 하는 것이 바람직합니다.

 

---

3. Single Exit - 단일 탈출

if - else if - else 구문의 처리의 결과로 리턴 값이 결정되는 경우, 단 하나의 return 문만 사용하는 것이 바람직합니다.

 

[Non-compliant Code]

class Bird {
    double getSpeed() {
        if(EUROPEAN.equals(type)){
            return getBaseSpeed();
        } else if(AFRICAN.equals(type)){
            return getBaseSpeed() - getLoadFactor() * numberOfCoconuts;
        } else if(NORWEGIAN_BLUE.equals(type)){
            return (isNailed) ? 0 : getBaseSpeed(voltage);
        } else{
            throw new IllealStateException("No Such Type:" + type);
        }
    }
}

[Compliant Code]

class Bird {
    double getSpeed() {
        double speed = 0D;
        if(EUROPEAN.equals(type)){
            speed = getBaseSpeed();
        } else if(AFRICAN.equals(type)){
            speed = getBaseSpeed() - getLoadFactor() * numberOfCoconuts;
        } else if(NORWEGIAN_BLUE.equals(type)){
            speed = (isNailed) ? 0 : getBaseSpeed(voltage);
        } else{
            throw new IllealStateException("No Such Type:" + type);
        }
         
        return speed;
    }
}

반복되는 if / else if 구문 내에서 return과 같은 주요 처리를 수행하지 않는 것이 바람직합니다. return 처리가 되어 있지 않는 else if 코드를 읽을 때, 그것이 개발자의 의도에 의한 것인지 아니면 단순히 실수로 빼먹은 것인지 구별하기 어렵습니다.

 

위에서 언급했던 두 지침- Fast Exit와 Single Exit-은 서로 상충되는 것처럼 보일 수 있습니다. 하지만 조금만 생각해보면 이 두 가지 중 어느 것을 선택할지 쉽게 판단할 수 있습니다.

• 주요 비즈니스 로직이 여러 가지 조건에 의해 구분되는 경우 Single Exit를 이용하는 것이 바람직합니다.
• 주요 비즈니스 로직이 특정 조건에 의해 실행되거나 실행되지 말아야 하는 경우 Fast Exit를 이용하는 것이 바람직합니다.

원칙적으로 Single Exit를 이용한다 하더라도 하나의 메서드 내에 여러 가지 주요 비즈니스 로직이 등장한다면 그것 자체가 리팩토링 대상입니다. (SRP 위반) 이 글에서 직접 다루지는 않겠지만 디자인 패턴을 이용하여 이에 대한 처리를 단순화할 수 있습니다. 이에 대한 상세한 설명은 다형성을 이용한 조건 분기 처리(Replace Conditional with Polymorphism)를 읽어보시기 바랍니다.

 

if문은 "기획자의 말"에는 적합할 수 있지만 "개발자의 코드"로는 적합하지 않은 경우가 많습니다. 정말로 필요한 경우에만 if문을 제대로 사용하고 그렇지 않은 경우 다른 방식을 이용하는 것이 바람직합니다.