코드 리뷰 ABC - 반복문 코드 개선

코드리뷰 시리즈

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

반복문은 프로그램에서 대부분의 실행시간을 차지하는 곳이고 코드의 복잡도가 가장 높아지는 곳이다. 그러므로 반복문 코드를 잘 짜는 것은 프로그래밍에서 가장 중요하고 핵심적인 역량이다. 코드리뷰는 실행 상의 오류를 찾는 과정이 아니다. 문제없이 잘 동작하는 것이 확인된 코드에서 좀더 보기좋게, 간결하고 분명하게 코드를 고칠 수 있는 부분이 있는지 찾는 과정이라고 할 수 있다. 반복문 작성에서 주의할 점은 1) 루프 변수가 반복에 따라 어떻게 바뀌는지 잘 보여야 한다. 2) 종료조건과 반복회수가 알아보기 쉽게 작성되어야 한다. 3) 가능하면 for-each를 사용한다. 4) 반복과 상관없는 코드는 루프 밖으로 빼야 한다. 5) 관리, 반복 구조를 가능한 간결하고 분명하게 표현한다. 6) 반복의 핵심(시간복잡도를 결정하는 부분)이 되는 코드가 눈에 잘 띄고 알아보기 쉽게 구성되어야 한다. 

(1) for 문에서 루프 변수 사용의 주의할 점

앞에서 살펴본 것처럼 루프에서 계산을 반복하면서 저장되거나 변경될 변수는 루프 시작하기 전에 초기화 또는 설명문을 통해 밝혀 주는 것이 좋다. 그리고 필요하다면 루프 끝난 후에 어떤 값을 가지고 있는지 확인해 보는 것도 좋다. (print 문을 넣어서 검증)

for 문에서는 루프 인덱스 변수가 가장 중요하다. 루프의 반복을 제어하는 변수이고 반복에 따라 바뀌어가는 대표적인 변수다. 루프 인덱스 변수는 루프 내에서만 사용하는 것이 바람직하다. 그러므로 밖에서 선언된 변수를 쓰기보다는 for (int i = ...)과 같이 for 문 안에서 선언하는 것이 좋다. (루프를 이해하기 쉽고 루프 인덱스 변수가 외부와 관계를 갖지 않으므로 코드 가독성이 좋아짐)

for (int i = 0; i < 10; i++) {
	array[i] += 10;
}

루프 변수를 실행코드부 내에서 변경하는 것은 바람직하지 않다. 그러므로 알고리듬에서 그것이 필요하더라도 다른 방법을 찾아보는 것이 좋다.

int array[] = new int[10];
for (int i = 0; i < 10; i++) {
	array[i] = scan.nextInt();
	for (int j = 0; j < i; j++) {
		if (array[j] == array[i]) {
			i--;
			break;
		}
	}
}

위 자바 코드는 중복된 값이 입력되면 i를 감소하여 중복되지 않은 값만 저장한다. 이런 코드라면 다음과 같이 바꾸어 작성하는 것이 바람직하다. 배열의 0부터 i 사이에 중복값이 있으면 array[i]를 다시 입력받는다. 이것이 로직도 분명해지고 의도하는 바가 정확히 나타나는 바람직한 코드가 된다. 여기서 findDup 같은 보조 함수를 도입하는 것은 밑에서 설명하겠지만 중첩의 깊이가 너무 깊어지는 것을 막는 효과가 있다.

public static void main(String[] args) {
	int array[] = new int[10];
	Scanner scan = new Scanner(System.in);
	for (int i = 0; i < 10; i++) {
		array[i] = scan.nextInt();
		while (findDup(array[i], array, 0, i))
			array[i] = scan.nextInt();
		}
	}
	for (int i = 0; i < 10; i++) 
		System.out.printf("array[%d] = %d\n", i, array[i]);
}

그러므로 for 루프 내에서 루프 인덱스 변수를 감소 또는 증가시키는 것은 바람직하지 않다.

(2) 종료 조건 관리

루프에서 가장 중요한 것은 어떻게 언제 끝낼 것이냐다. 즉 반복 회수, 또는 종료 조건이다. 루프를 작성할 때는 반복회수나 종료 조건이 잘 드러나게 작성하는 것이 좋다. 예를 들어 반복하는 회수를 나타내는 수식이 복잡하거나 break하는 조건식이 지나치게 복잡하다면 간단하게 고치는 방법을 찾아 보아야 한다. (함수 독립, 변수에 미리 계산 등)

또한 인덱스 변수와 마찬가지로 for 루프 내에서 상한값을 변경하는 것은 매우 바람직하지 않다. 예를 들어 다음과 같은 코드를 보자.

public static void main(String[] args) {
    int array[] = new int[10];
    int count=1, sum=0;
    Scanner scan = new Scanner(System.in);
    for (int i = 0; i < count; i++) {
        array[i] = scan.nextInt();
        if (sum + array[i] < 100)
            count++;		// 합계가 100이 넘지 않으면 더 입력받음
        sum += array[i];
    }
    for (int i = 0; i < 10; i++) 
        System.out.printf("array[%d] = %d\n", i, array[i]);
}

이 코드에서 의도하는 바는 count를 합계가 100이 될 때까지 늘려서 배열에 입력값을 저장하겠다는 것이다. 그러나 루프 실행부에서 count 값을 증가시키는 것은 첫째 무한루프의 위험성이 크다. 또한 루프의 반복에 대해 변하는 종료조건(moving target)을 가지므로 이해하기 어렵다. 코드를 이렇게 짤 일이 없을 것 같지만 루프 상한값을 변경하는 코드가 생각보다 자주 있다. 이것은 사실 if break를 이용해 중간에 종료하거나 while 루프로 짜야 할 코드로 볼 수 있다. 

break나 continue를 이용해서 종료 및 루프 제어를 관리하는 것이 바람직하다. 그리고 루프의 종료나 제어를 결정하는 제어변수가 숨겨져 있는 것은 바람직하지 않다. 같은 레벨에서 루프의 반복을 제어하는 로직이 표현되어 있어야 하고, 하위 레벨에 숨겨져 있으면 안된다는 것이다. 바깥 루프를 끝낼지 말지가 중첩된 안쪽의 루프 계산에서 결정되는 구조가 그런 경우이다.

자바에서는 다단계 break 또는 다단계 continue로 여러 개의 루프를 한꺼번에 벗어나는 기능이 있다. 다음 코드는 입력을 한줄씩 읽어 단어를 wordList에 모으는데, $가 나오면 그 줄을 끝내고 다음 줄로 넘어가고 end가 나오면 입력을 종료한다.

Outer:
while (true) {
	line = scan.nextLine();
	String tokens[] = line.split();
	for (String tok: tokens) {
		if (tok == "$")
			continue Outer;
		if (tok == "end")
			break Outer;
		wordList.append(tok);
	}
}

for 루프는 반복할 회수가 정해져 있고 차례로 증감하는 인덱스 변수가 필요하고 예외적으로 break를 통해 벗어날 수 있는 경우에 적합한 것이다.

(3) for each 문의 사용

위에서 얘기한 바람직한 for 루프의 형태는 배열이나 콜렉션의 요소를 차례로 (인덱스 변수를 사용해서) 방문하는 경우가 대부분이다. 이런 경우는 사실 for each (for 콜론) 문으로 나타낼 수 있다. 자바에서 for (int i = 0; ...) 대신 for each로 바꿀 수 있다면 당연히 for each 문을 써야 한다. for each 문은 이해하기 쉽고 간결하고 보기좋은 코드이다. 게다가 성능 효율성도 좋아진다. 가능한 경우라면 반드시 써야 하는 좋은 기능이다.

int array = new int[N];
for (int i = 0; i < array.length; i++)
    print(array[i]);
    
=>
for (X a: array)
	print(a);

ArrayList<Integer> numList = new ...
for (int i = 0; i < numList.size(); i++)
	sum += numList.get(i);
=>
for (int a: numList)
	sum += a;

단, for each 문을 사용할 수 없는 경우를 주의해야 한다. for each 문에서 사용하는 변수는 배열이나 콜렉션의 요소를 차례로 하나씩 지정받게 된다. 지정된 이 변수를 이용해서 값을 읽어오거나 계산하는 것은 문제가 없지만 값을 변경하거나 지정하는 경우는 배열이나 콜렉션의 요소는 변동이 없고 변수에 들어있는 값이 바뀌게 된다. 

ArrayList<Integer> numList = new ...
for (int a: numList)
	a = scan.nextInt();

그러므로 변수의 값을 지정하거나 변경하는 루프에서는 기존처럼 인덱스 변수를 이용하는 for 루프, 또는 이터레이터를 이용해야 한다.

(4) 반복과 상관없는 코드를 밖으로 빼기

반복되어야 할 필요가 없는 코드를 반복문 안에 넣는 것도 많이 생기는 실수다. 입력이나 print 문이라면 반복문 안에 잘못 들어간 경우 바로 실행에서 오류를 발견할 수 있다. 그러나 한번만 해도 되는 계산이나 변수 지정이 잘못 반복문 안에 들어갔다면 그것은 실행에는 큰 영향을 미치지 않지만 성능이 떨어지는 효과가 날 것이다. 이런 것을 루프 불변값 또는 불변식(invariant)이라고 한다. 사실 컴파일러에서 이런 코드를 찾아서 루프 밖으로 내보내주는 기능이 코드 최적화의 일환으로 들어있는데 그렇더라도 코드 작성 단계에서 그런 경우를 찾아 밖으로 꺼내주는 것이 바람직하다.

불변값 또는 불변식은 루프가 반복되면서 변경되지 않는 값이다. 즉 루프의 전이나 중간이나 끝난 후에 계속 같은 값을 가진다. 그러므로 루프 시작전에 한번, 또는 끝나고 한번만 하면 되는 수식 계산이나 변수 지정을 반복회수만큼 반복하는 것은 바보짓이다. 자주 생길 것 같지 않지만 의외로 많이 발견되는 실수이다.

아래와 같은 코드에서 "복잡한 계산식..."이 불변식인 경우가 그런 예이다. 이것은 루프 밖으로 옮기면 된다.

for (int i = 0; i < M; i++) {
    루프의 계산부...
    value = 복잡한 계산식...
    루프의 계산부...
}

(4) 반복 구조를 가능한 간결하고 분명하게 표현한다.

한 함수의 단위에서 전체적으로 반복 구조가 어떻게 되는지 한눈에 파악할 수 있어야 한다. 이것은 코드를 읽을 때도 중요한 기법이다. 좋은 구조라면 반복이 너무 중첩되지 않고 반복부가 다른 부분과 얽히지 않아야 한다. 중첩에 관한 규칙은 비교적 분명하다. 어떤 형태든 들여쓰기가 네 단계 이상 들어가는 것은 좋은 코드가 아니다. 루프 중첩은 두 단계(while-for, for-for, for-while) 정도가 적합하며, if 문 안에 또다른 루프가 중첩되는 것은 바람직하지 않다. 그런 코드가 생기는 부분은 코드 리뷰 단계에서 함수를 독립시키거나 if 문의 위치를 바꾸는 방법(아닌 경우를 break나 continue로 미리 제외)이 있다. 

또한 한 함수 안에 반복부의 개수는 3개를 넘지 않는 것이 좋다. 보통 함수는 하나의 기능을 중점으로 작성하는 것이 바람직하므로 메인 반복부가 하나 있고 그것을 위한 전후 처리를 해주는 루프 정도가 들어갈 수 있다.

루프는 어떤 일을 반복하여 배열이나 변수에 값을 저장하는 것이 목적이다. 그러므로 루프 들어가기 전에 변수값 설정, 루프가 끝난 후에 바뀐 값이 무엇인지 확인하는 것이 중요하다., 반복부 작성에 좋은 기법 중 하나는 반복부 바로 앞에 변수의 초기화 또는 값설정 코드들을 넣어주는 것이다. 예를 들어 다음의 중간 루프는 array 배열에서 10점 이상인 것만 scores 배열로 옮기고 최대값을 계산한다. 이를 위해 그 루프에서 필요로 하는 scores 배열이나 k나 max1의 초기화를 루프 들어가기 전에 모아 놓았다. 이렇게 하면 그 루프에 들어가기 전에 변수의 값들이 무엇인지를 분명히 해주어 코드를 읽기 쉽고 작성도 쉬워 진다. 이 때 지정이 어렵다면 설명문으로 표시해 주는 것도 좋은 방법이다.

루프가 연속해 있을 때는 이전 반복부에서 얻어진 값이 다음 루프에 사용된다. 아래 경우 max1 같은 변수가 그런 역할을 한다. 이런 변수에 대해서는 간단하게 설명문을 달아주는 것도 좋다.

int max1=0. sum1, k;
int count=0
for (int i = 0; i < n; i++) {
   	if (array[i] >= 10)
   		count++;
}
int scores[] = new int[count];
k = 0; 
max1 = array[0];
for (int i = 0; i < n; i++) {
	if (array[i] >= 10)
		scores[k++] = array[i];
	if (array[i] > max1)
		max1 = array[i];
}
// max1 : scores에 있는 점수 중에 최대값
sum1 = 0;
for (int i = 0; i < count; i++) {
   scores[i] = 100.0 * scores[i] / max1;
   sum1 += scores[i];
}

(5) 반복의 핵심부가 눈에 잘 띄게 구성해야 한다.

반복부 안에서는 무엇을 반복하는지(어떤 값이 반복해서 바뀌는지)가 잘 드러나게 코드를 작성해야 한다. 너무 여러 가지 일을 해서 복잡해 지면 안된다. 특히 최근에는 스트림 방식이 각광을 받으면서 루프가 하는 일은 하나의 연산, 또는 함수호출 처럼 단순한 것이 바람직하다. "단순한 것은 단순하게"가 코드의 원칙이라고 할 수 있다. 물론 반복부의 로직이 아주 복잡해서 간단치 않은 경우도 있는데, 그런 경우도 복잡한 계산부를 가능한 한 독립시켜 함수로 만들면 훨씬 구조가 간단해 진다. 예를 들어 다음 코드를 생각해 보자.

for (Item item: itemList) {
    if (item.hasOptions()) {
       for (Option op: item.options) {
          if (!item.hasStock(op))
             nSoldOut++;
       } 
    }
}

이 코드는 몇가지 개선의 여지가 있다. if 안에 중첩된 for 문을 해결하기 위해 다음과 같이 고쳐볼 수 있다. 이렇게 해서 중첩의 깊이를 하나 줄였고 if 문 안에 for 루프가 들어가는 문제도 해결되었다.

for (Item item : itemList) {
    if (!item.hasOptions()) 
    	continue;
    for (Option op : item.options) {
        if (!item.hasStock(op))
           nSoldOut++;
    }
}

반복부는 반복하는 제어의 흐름 구조를 표현하는데 중요 역할이 있다. 그러므로 그 안에 너무 복잡한 로직을 집어넣지 말고 반복이 되는 동작이 무엇인지가 알아보기 쉽게 코드를 작성해야 한다.

다음으로 객체지향적으로 봤을 때 아이템의 옵션 별 품절 여부나 개수는 아이템에서 하는 것이 적합하다. 이것을 다음과 같이 고친다면 훨씬 좋은 구조가 될 수 있다. 물론 여기서 클래스에 또 이 기능에 관련된 메소드를 만들어야 하는 문제가 있으나 객체지향에서는 그 클래스의 내부 데이터를 필요로 하는 일은 클래스 메소드에 넣는 것이 바람직하다.

for (Item item: itemList) { 
    nSolOut += item.getSoldOutOptionCount();
}

제어 구조와 객체지향은 서로 긴장 관계에 있다. 이 두 가지를 잘 결합하는 것이 진짜 프로그래머의 능력이다. 객체지향은 코드가 길어지고 메소드가 많아지고 타이핑할 것도 많아진다. 가능한 한 로직을 숨기고 추상화의 높은 차원에서 그냥 글을 읽듯이 알아보기 쉽게 하는 것이 목적이라고 볼 수 있다.

물론 그렇다고 세상의 모든 코드가 단순하고 간단해야 하는 것은 아니다. 분명 정말 복잡하고 복잡해질 수 밖에 없는 코드 부분도 있을 것이다. 그리고 코드 관리 등의 이유로 한 곳에 알고리듬이 모여 있어야 하는 경우도 있을 수 있다. 그런 경우에 개선의 여지가 있는지 꼼꼼히 검토하는 것이 더욱 중요하다.