2.4.4.3 숫자 두개를 쉼표(,) 구분자로 입력할 경우 두 숫자의 합을 반환한다

HINT)
String[] numbers = text.split(",");

 

public class StringCalculatorTest {

    ...

    @Test
    public void add_쉼표구분자() throws Exception {
        assertEquals(3, cal.add("1,2"));
    }

}

 

public class StringCalculator {

    public int add(String text) {
        if (text == null || text.isEmpty()) {
            return 0;
        }
        
        if (text.contains(",")) {
            String[] values = text.split(",");
            int sum = 0;
            for (String value : values) {
                sum += Integer.parseInt(value);
            }
            return sum;
        }

        return Integer.parseInt(text);
    }

}

 

쉼표(,) 구분자를 가지는 문자열 처리까지 완료했다. 위와 같은 로직을 구현할 때 보통 if / else if / else 형태로 구현하는 것이 일반적이지만, else를 가능한 사용하지 말라는 원칙에 따라 else를 사용하지 않고 구현했다.

구현을 완료하고 결과까지 확인했더니 모든 테스트가 성공했다. 이제 리팩토링할 부분을 찾아본다.

많은 기능을 구현하지 않았음에도 불구하고 소스코드의 복잡도는 증가하고 있다. 숫자가 하나인 경우와 쉼표 구분자를 포함하는 경우를 분기해서 처리해야 하는 부분을 고쳐본다.

String의 split() 메서드에 숫자 하나를 가지는 문자열을 전달하면 숫자 하나가 담긴 String[]을 반환하면 이 부분의 분기문을 제거할 수 있다. 이를 확인하기 위한 테스트를 추가하고 확인한다.

 

import static org.junit.Assert.*;
import org.junit.Test;

public class SplitTest {
    @Test
    public void split() {
        assertArrayEquals(new String[] {"1"}, "1".split(",));
        assertArrayEquals(new String[] {"1", "2"}, "1,2".split(","));
    }
}

위와 같이 테스트 코드를 추가했더니 테스트가 성공한다. 다음과 같이 리팩토링을 진행한다.

 

public class StringCalculator {

    public int add(String text) {
        if (text == null || text.isEmpty()) {
            return 0;
        }
        
        String[] values = text.split(",");
        int sum = 0;
        for (String value : values) {
            sum += Integer.parseInt(value);
        }
        return sum;
    }

}

 

if 절을 하나 제거했더니 깔끔해졌다. 추가적으로 리팩토링할 부분을 살펴보면, add() 메서드의 복잡도가 증가하고 있다. add() 메서드를 분리하기 위해 숫자의 합을 구하는 부분을 별도의 메서드로 분리하기 위해 다음과 같이 리팩토링 한다.

 

public class StringCalculator {

    public int add(String text) {
        if (text == null || text.isEmpty()) {
            return 0;
        }
        
        String[] values = text.split(",");
        return sum(values);
    }

    private int sum(String[] values) {
        int sum = 0;
        for (String value : values) {
            sum += Integer.parseInt(value);
        }
        return sum;
    }

}

 

각 메서드가 하나의 책임만 가지고 있는지 살펴본다. sum() 메서드가 두 가지 일을 하고 있다. 문자 배열을 숫자로 변환하는 작업과 숫자 배열의 합을 구하는 두 가지 일을 하고 있음을 알 수 있다. 메서드는 한 가지 책임만 가져야 한다는 원칙에 따라 이 두 가지 작업을 다음과 같이 분리한다.

 

public class StringCalculator {

    public int add(String text) {
        if (text == null || text.isEmpty()) {
            return 0;
        }
        
        String[] values = text.split(",");
        return sum(toInts(values));
    }

    private int[] toInts(String[] values) {
        int numbers[] = new int[values.length];
        for (int i = 0; i < values.length; i++) {
            numbers[i] = Integer.parseInt(values[i])
        }
        return numbers;
    }

    private int sum(int[] numbers) {
        int sum = 0;
        for (int number : numbers) {
            sum += number;
        }
        return sum;
    }

}

 

간단한 로직을 구현하는 데 소스코드 복잡도가 감소하는 것이 아니라 오히려 복잡해진 느낌이 든다. 이렇게 극단적으로 리팩토링을 하는 이유는 연습이기 때문이다. 현장에서 프로젝트를 할 때 모든 소스코드에 대해 이 같은 리팩토링을 진행할 수 없지만, 이렇게 작은 코드로 극단적으로 연습해야 리팩토링할 부분을 더 쉽게 찾아 개선할 수 있다.

이와 같이 리팩토링을 한 뒤, 주의 깊게 봐야할 부분은 public으로 공개하고 있는 add() 메서드가 얼마나 읽기 쉽고 좋은가가 가장 중요하다. 지금 단계보다 더 극단적으로 리팩토링 한다면 다음 단계까지도 할 수 있다.

 

public class StringCalculator {

    public int add(String text) {
        if (isBlank(text)) {
            return 0;
        }
        
        return sum(toInts(split(text)));
    }

    private boolean isBlank(String text) {
        return text == null || text.isEmpty();
    }

    private String[] split(String text) {
        return text.split(",");
    }

    private int[] toInts(String[] values) {
        int numbers[] = new int[values.length];
        for (int i = 0; i < values.length; i++) {
            numbers[i] = Integer.parseInt(values[i])
        }
        return numbers;
    }

    private int sum(int[] numbers) {
        int sum = 0;
        for (int number : numbers) {
            sum += number;
        }
        return sum;
    }

}

 

add() 메서드를 처음 읽는 개발자 입장이라면 리팩토링하기 전이 읽기 좋을지, 후가 읽기 좋을지 생각해본다. 이렇게 극단적으로 리팩토링한 이유는 소스코드를 읽을 때 이 메서드가 무슨 일을 하는 메서드인지 최대한 쉽게 파악할 수 있도록 하기 위함이다. 즉, 세부 구현은 모두 private 메서드로 분리해 관심사에서 제외하고 add() 메서드가 무슨 일을 하는지에 대한 전체 흐름을 쉽게 파악할 수 있도록 하기 위함이다. add() 메서드가 무슨 일을 하는지 글로 표현해보면 "text 값이 비어 있으면 0을 반환, 비어 있지 않으면 구분자로 분리(split), 숫자로 변환(toInts)한 다음 이 숫자의 합(sum)을 구한다." 라고 파악할 수 있다. 세부 구현에 집중하기 보다 논리적인 로직을 쉽게 파악할 수 있도록 구현하는 것이 읽기 좋은 코드다.

소스코드를 통해 의도를 명확히 드러내는 것은 쉽지 않다. 이 한 가지 예로 모든 것을 설명할 수 없지만, 깔끔하고 읽기 좋은 코드를 구현하기 위해 끊임 없이 고민하고 연습해야 한다. 가끔 이렇게 극단적인 연습을 한다면 리팩토링 실력도 키우고, 깔끔한 코드를 구현하는데 많은 도움을 받을 수 있다.

참고도서 : https://roadbook.co.kr/169

[신간안내] 자바 웹 프로그래밍 Next Step