코드 리뷰 ABC - if 문 줄이기

코드리뷰 시리즈

• 프로그래밍 스타일 - 들여쓰기와 스페이스
• 프로그래밍 스타일 - 이름 붙이기
• 코드 리뷰 ABC - if 문 줄이기 (이글)
• 코드 리뷰 ABC - 반복문 코드 개선 

코드 리뷰는 개발자들이 서로의 코드를 보면서 의견을 나누고 내꺼 남의 꺼 구별없이 더 나은 코드가 될 수 있게 고쳐나가는 개발 과정의 한 단계다. 코드 리뷰를 통해 사소한 스타일 규칙부터 if 문이나 for 루프를 작성하는 방식, 메소드를 만드는 방식, 매개변수나 반환형의 타입 등 모든 부분에 대해 토론하고 서로 자기의 의견을 제시하고 더 나은 것을 수용하면서 코드를 더 좋게 만들게 된다. 이 때 프로그램이 올바로 동작하는 것은 당연한 전제조건이고 실행이나 논리상의 오류가 있다면 프로의 세계에서는 망신을 당하게 된다. 수준높은 개발자의 커뮤니티에서 살아남기 위해서는 수준높은 코드를 만드는 것은 물론이고 이러한 토론에 열린 마음으로 참가해야 하고 또한 다른 사람의 코드에 적극적인 의견을 개진해야 한다.

여기서는 초보자들이 간단히 점검해 볼 수 있는 몇가지 코드 개선 규칙을 살펴보려고 한다. 스타일 규칙에 관한 내용은 앞의 포스트에서 다루었으므로 여기서는 코드의 구성이나 문장 사용 방식을 중심으로 살펴본다.

(1) 연속된 if 조건문

사실 좋은 코드의 기본은 코드 중복이 없는 것이다. 크게는 소프트웨어 전체 차원에서 함수나 클래스 수준의 코드 중복을 없애기 위한 상속이나 리팩토링 기법 같은 고급 수준도 있지만, if문이나 수식 같은 작은 단위에서 코드의 중복을 없애는 것이 더 중요한 문제고 항상 몸에 익혀 두어야 하는 중요한 기준이다. 몇 가지 예를 통해 중복 코드가 어떻게 제거되는지 살펴보자. 

if (select == 1) System.out.println("하복 판매");
else if (select == 2) System.out.println("춘추복 판매");
else if (select == 3) System.out.println("동복 판매");

이런 코드는 문자열과 관련되어 코드 중복이 발생하는 경우이다. 3번 이상의 if가 같은 코드로 중복되는 경우 반드시 for 루프로 변환을 고려해야 한다. 이 때 사용할 수 있는 방법은 스트링을 배열로 저장하고 배열 인덱스를 이용해 처리하는 것이다. 다음 코드에서는 세 개의 문자열을 배열에 넣고 선택번호 select를 이용하여 해당 문자열을 가져와 출력하고 있다. 이렇게 되면 if와 else if 전체를 제거하고 하나의 출력문만으로도 같은 일을 할 수 있다.

String seasons[] = {"하복", "춘추복", "동복"};
System.out.println(seasons[select-1]) + " 판매");

반대 방향으로 문자열에 의한 중복이 일어나는 경우도 있다.

if (kwd.equals("하복") price = (int) (20000 * 1.1);
else if (kwd.equals("춘추복") price = (int) (40000 * 1.1);
else if (kwd.equals("동복") price = (int) (50000 * 1.1);

위와 같이 수식 계산이 중복되는 경우도 다음과 같이 고칠 수 있다.

String seasons[] = {"하복", "춘추복", "동복"};
int prices[] = {20000, 40000, 50000};
int index = findIndexOf(seasons, kwd);
price = (int) (prices[index] * 1.1);

여기서 findIndexOf 메소드는 직접 만들어야 한다. 이런 경우 메소드까지 만들면서 굳이 배열로 할 필요가 있느냐는 질문이 있을 수 있다. 코드가 오히려 길어지지 않았느냐고... 그런데 문제는 하복, 춘추복, 동복이 여기서 한번만 쓰이는 문자열이라면 그냥 if 문을 세번 써도 되지만, 교복 회사나 교복 판매점에서 쓰는 프로그램이라면 아마도 이 문자열들이 프로그램 전체에 걸쳐서 계속 등장할 것이다. 그 경우 우리는 스트링 배열을 만들어 두고 이 값들에 대해 for 루프를 돌릴 수 있게 하고 각 계절 별로 번호나 이름을 정해 두고 코드 전체에서 일관되게 사용하는 것이 반드시 필요하다.

사실 한 단계 더 나아가서 정말 이 문자열들이 많이 쓰인다면 1이 하복, 2가 춘추복, 3이 동복이라고 정해두고 코드 중간중간에 1, 2, 3이 등장하는 것은 바람직하지 않다. 이것에 대해서는 따로 살펴보아야 되겠지만 많이 쓰이는 기법은 C의 #define 이나 자바의 static final int 처럼 이름을 정의하여  쓰는 방법과 enum을 통해 여러 개의 이름 그룹을 정의하는 방법이다. enum은 이런 면에서 매우 중요하고 좋은 테크닉이므로 잘 익혀두어야 한다. enum에 대해서는 따로 포스트를 만들 계획이다.

또 많이 사용되는 패턴으로 구간을 검사하여 대응하는 다른 값을 찾는 다음과 같은 코드를 생각해 볼 수 있다.

for (int i = 0; i < 10; i++) {
   if (score[i] >= 90) grade = "A";
   else if (score[i] >= 80) grade = "B";
   else if (score[i] >= 70) grede = "C";
   else if (score[i] >= 60) grade = "D";
   else grade = "F";
}

이 코드에서의 if else 중복을 해결하기 위해서는 마찬가지로 구간값을 가진 배열을 이용할 수 있다. 구간 경계값인 90, 80, 70, 60을 배열에 저장하고 이것을 이용해서 각 값을 비교하는 루프를 작성한다. 여기서 큰 값과 비교하여 더 크면 break하므로 80과 비교할 때는 90보다 크거나 같은 값은 제외되었음을 확인할 수 있다. 또한 if 문을 4번 수행해서 한번도 만족되지 않은 경우는 디폴트 값으로 "F"를 그대로 가지게 된다. 

int interval[] = {90, 80, 70, 60};
char* gradeStr[] = {"A", "B", "C", "D", "F"};
for (int i = 0; i < 10; i++) {
   grade = "F";
   for (int j = 0; j < 4; j++) {
       if (score[i] >= interval[j]) {
           grade = gradeStr[j];
           break;
       }
   }
}

한편 함수의 중복을 해결하는 방법도 많이 있는데, 이것은 좀더 복잡한 주제이므로 별도의 포스트에서 다루어야 할 것 같다.

(2) boolean을 리턴하는 메소드

클래스에서는 boolean을 반환하는 메소드를 많이 쓰게 된다. 비교나 조건의 검사를 해서 true, false를 돌려주는 메소드에서 나타나는 패턴이 있다.

boolean match(String kwd) {
    boolean result = false;
    if (name.equals(kwd) || Integer.toString(id).equals(kwd))
        result = true;
    return result;
}

별 문제가 없어 보일 것이다. 그러나 이 코드는 다음과 같은 return 문으로 바뀌어야 한다.

   return (name.equals(kwd) || Integer.toString(id).equals(kwd));

변수 선언이나 if 문이 필요하지 않은 경우다. 물론 비교나 검사 로직이 더 복잡해 지면 return 문 하나로 어려운 경우도 있다.

boolean match(String kwd) {
    if (name.equals(kwd))
        return true;
    else if (Integer.toString(id).equals(kwd))
        return true;
    else if ...
    else return false;
}

위 코드에서 개선되어야 하는 것은 else 키워드가 모두 불필요하다는 점이다. return 후에는 다음 코드로 넘어가지 않으므로 다음의 else는 없어야 한다. 그리고 boolean을 반환하는 메소드에서 대부분의 경우 boolean result 같은 결과값을 저장할 변수는 필요하지 않다. 물론 아주 복잡한 조건의 검사여서 지역변수에 임시 결과를 저장해야 하는 경우가 있을 수 있으나 반환한 결과만 저장된다면 바로 return을 하는 것이 맞다.

또한 흔히 나타나는 초보자들의 실수가 ArrayList에 어떤 객체가 들어있는가의 검사를 for 루프를 돌리는 메소드로 만드는 경우이다. ArrayList의 contains 메소드가 이런 일을 해 준다. 이외에도 라이브러리 함수에는 boolean을 돌려주는 검사 메소드가 여러 개 있으므로 확인해 보고 사용하는 것이 좋다. 대부분의 컬렉션 클래스들은 isEmpty(), contains() 메소드를 포함한다.

(3) 간단한 이항 선택 수식

앞에서 살펴본 예제들과 비슷하지만 출력에서 다음과 같은 if 문을 쓰는 경우를 많이 보게 된다.

if (seasonOff)
     System.out.printf("%s (%d벌) %d원 [50% 세일]\n", category, copies, price/2);
else System.out.printf("%s (%d벌) %d원\n", category, copies, price);

이것을 어떻게 코드 중복 없이 작성할 수 있을까? price 계산을 먼저 임시 변수에 하는 방법도 있지만 다음과 같은 ?: 연산자 사용도 생각해 볼 수 있다. ?: 연산자는 수식에서 편리하게 사용해서 if 문이나 임시 변수를 제거할 수 있는 방법이다. (코드가 너무 복잡해 진다면 가독성이 떨어지는 요인이 될 수도 있으므로 간단한 경우에만 사용하는 것이 좋다)

System.out.printf("%s (%d벌) %d원", category, copies, (seasonOff) ? price/2 : price);
if (seasonOff) System.out.print(" [50% 세일]");
System.out.println();

(4) if 조건식에 의한 실행의 조건부가 길어지는 경우

if 문과 관련해서 일반적인 주의사항으로 조건부로 둘러싼 부분이 너무 길어지거나 그 안에 다시 중첩해서 중첩의 깊이가 깊어지는 것을 경계해야 한다는 점이다. 초보 프로그래머일수록 if 문 안에 if, 또 그 안에 for로 중첩에 중첩이 깊어지는 코드를 많이 작성한다. 이것은 프로그래머가 로직을 생각하면서 경우의 수를 나누는데 한 방향으로 계속 들어가면서 조건에 조건을 계속 붙여나갈 때 생기는 현상이다. 이렇게 만들어진 코드는 가독성이 떨어질 뿐 아니라 

초보 프로그래머들에게서 많이 발견되는 실수 중 하나로 반복부나 함수 안에서 if 문으로 긴 코드 부분을 둘러싸는 것이다. 특히 그 if 문 이외에 다른 부분이 없다면 그 코드는 불필요하게 중첩 깊이만 한번 더 들어가서 불필요하게 복잡한 코드가 된다.

int matches(String kwd) {
    if (kwd != null) {
       if (names.equals(kwd))
           return true;
       if (dept.equals(kwd))
           return true;
    }
    return false;
}

여기서 if 문을 (kwd != null) 대신 if (kwd == null) return false;으로 바꾸면 그 아래 긴 if 문 안의 코드를 다 {  } 중첩 없이 바로 쓸 수 있고 들여쓰기 한번을 줄일 수 있다. 그러면 그 안에 다시 들여쓰기한 중첩의 깊이가 깊어지는 것을 막을 수 있다.

반복문의 경우도 마찬가지다. 입력을 검사해서 그에 따라 여러 가지 처리를 해야 하는 다음 코드에서 잘못된 입력과 종료조건 등 루프의 반복부 제어에 관련된 코드를 continue, bread, return 등을 적절히 이용해서 루프 코드의 가독성을 올리는 것은 매우 중요하다.

while (true) {
   num = scan.nextInt();
   if (num != 0) {
      if (num >= 0 && num < 10) {
          // num에 관한 처리부가 길어짐
          //
          //
      } else {
          System.out.println("잘못된 입력입니다.");
      }
   } else
       break;
}

위와 같은 코드는 break와 continue를 이용하면 아래와 같이 바꿀 수 있다.

while (true) {
   num = scan.nextInt();
   if (num == 0) break;
   if (num < 0 || num >= 10) {
       System.out.println("잘못된 입력입니다.");
       continue;
   }
   // num에 관한 처리부가 길어짐
   //
   //
}

이렇게 바꾸면 몇 가지 점에서 코드가 좋아진다. 

• 루프 전체적으로 { ... } 중첩의 깊이가 얕아져 보기가 훨씬 좋다.
• 루프의 메인 부분 // 코드가 깊이 중첩되지 않고 나타난다. 그 결과 코드가 훨씬 간결해 보이고 이해하기도 쉬워진다.
• 루프가 종료되거나 입력을 무시하고 넘어가야 하는 경우에 대해 루프의 앞부분에서 처리하므로서 루프의 메인 부분 코드를 읽을 때 그런 조건을 고려하지 않아도 된다.

루프나 함수에서 주요 계산부 로직이 나오기 전에 제외되어야 할 값이나 무시해야 하는 경우를 continue로 미리 걸러주는 것이 좋은 방법이다. 이것은 오류 처리나 계산값의 가정을 미리 체크하는 방법으로도 좋은 코딩 습관이다.