“TDD 리팩토링 by 자바지기 박재성님”의 강연을 보고 리팩토링부분만 정리한 글이다.
리팩토링 연습 - 메서드 분리
오늘 다룰 메서드이다.
값이 null이거나 빈문자열일 때는 0을 반환하고 쉽표와 콜론dml 경우엔 split을 해서 for문돌면서 Int로 바꾸서 합을 구한다.
public class StringCalculator {
public static int splitAndSum(String text) {
int result = 0;
if (text == null || text.isEmpty()) {
result = 0;
} else {
String[] values = text.split(",|:");
for (String value : values) {
result += Integer.parseInt(value);
}
}
return result;
}
}
잠시 5분동안 위 코드를 스스로 리팩토링해보자. 여기서 더 뭘 줄이냐 싶을텐데. 할게 꽤 있다.
5분동안만 해보자.
5분이 지났으면 같이 리팩토링해보자. (내 경우엔 if 조건문을 메서드로 만드는 것밖에 생각못했다.)
한 메서드에 오직 한 단계의 들여쓰기(indent)만 한다.
public class StringCalculator {
public static int splitAndSum(String text) {
int result = 0;
if (text == null || text.isEmpty()) {
result = 0;
} else {
String[] values = text.split(",|:");
for (String value : values) { //
result += Integer.parseInt(value); // 여기가 Indent가 2다.
} //
}
return result;
}
}
==> 메서드를 분리하여 인덴트를 줄인다.
public class StringCalculator {
public static int splitAndSum(String text) {
int result = 0;
if (text == null || text.isEmpty()) {
result = 0;
} else {
String[] values = text.split(",|:");
result = sum(values); // sum 함수를 만들어서 인덴트를 줄였다.
} // Indent 가 1이 됐다.
return result;
}
private static int sum(String[] values) {
int result = 0;
for (String value : values) {
result += Integer.parseInt(value);
}
return result;
}
}
else 예약어를 쓰지 않는다.
public class StringCalculator {
public static int splitAndSum(String text) {
int result = 0; //
if (text == null || text.isEmpty()) { //
result = 0; //
} else { // else 를 쓰지 않는다.
String[] values = text.split(",|:"); //
result = sum(values); //
} //
return result;
}
private static int sum(String[] values) {
int result = 0;
for (String value : values) {
result += Integer.parseInt(value);
}
return result;
}
}
=> else 를 제거한다. else를 안쓰다보면 불필요한 로컬 변수 또한 사라지게 된다.
public class StringCalculator {
public static int splitAndSum(String text) {
if (text == null || text.isEmpty()) { //
result = 0; //
} //
String[] values = text.split(",|:"); //
return sum(values); //
} //
private static int sum(String[] values) {
int result = 0;
for (String value : values) {
result += Integer.parseInt(value);
}
return result;
}
}
else 문을 안쓰게 되면 불필요한 로컬 변수 또한 없앨 수 있다. 그리고 Indent까지 줄어든다.
메서드가 한가지 일만 하도록 구현하기
sum 함수가 계속 거슬린다. 하나의 일만하는거 같지가 않다.
public class StringCalculator {
public static int splitAndSum(String text) {
if (text == null || text.isEmpty()) {
result = 0;
}
String[] values = text.split(",|:");
return sum(values);
}
private static int sum(String[] values) { //
int result = 0; //
for (String value : values) { // sum 메서드가
result += Integer.parseInt(value); // string을 int로 변환도 하고
} // 더하기까지 한다.
return result; //
}
}
sum 메서드가 인트 변환뿐만아니라 더하기까지 해버린다. 두개의 메서드로 나누자.
public class StringCalculator {
public static int splitAndSum(String text) {
if (text == null || text.isEmpty()) {
result = 0;
}
String[] values = text.split(",|:");
int[] numbers = toInts(values);
return sum(numbers);
}
private static int[] toInts(String[] values) { // Int로 변환만하는 함수
int[] numbers = new Int[values.length];
for (int i = 0; i < values.length; i++) {
numbers[i] = Integer.parseInt(values[i]);
}
return numbers;
}
private static int sum(int[] numbers) { // 더하기만 하는 함수
int result = 0;
for (int number: numbers) {
result += number;
}
return result;
}
}
이 코드 보자마자 든 생각이 for문 한번 더도는데 이게 정말 맞아? 생각하고 있었는데 바로 강사분께서 서비스에서 for문 돌리는 데이터는 대부분 크지 않기 때문에 성능에 아무 영향이 없다고 한다.
메서드가 단 하나의 일만하므로 재활용하는데에 제약이 있다. 재활용을 위해 반드시 하나만 해야한다. 테스트를 위해서는 더더욱 하나의 일만해야한다.
로컬 변수가 정말 필요한가?
public class StringCalculator {
public static int splitAndSum(String text) {
if (text == null || text.isEmpty()) {
result = 0;
}
String[] values = text.split(",|:");
int[] numbers = toInts(values);
return sum(numbers);
}
}
values, numbers 로컬 변수가 필요없어보인다.
public class StringCalculator {
public static int splitAndSum(String text) {
if (text == null || text.isEmpty()) {
result = 0;
}
return sum(toInts(text.split(",|:"););); // 이런식으로 줄일 수 있다.
}
}
불필요한 로컬변수는 지워준다.
Compose Method Pattern
메서드(함수)의 의도가 잘 드러나도록 동등한 수준의 작업을 하는 여러 단계로 나눈다.
“추상화 레벨을 같게 한다.”
public class StringCalculator {
public static int splitAndSum(String text) {
if (isBlank(text)) { // 1단계 추상화
result = 0;
}
return sum(toInts(split(text)); // 1단계 추상화
}
private static boolean isBlank(String text) {
return text == null || text.isEmpty();
}
private static String[] split(String text) {
return text.split(",|:");
}
private static int[] toInts(String[] values) {
[...]
}
private static int sum(int[] numbers) {
[...]
}
}
코드를 전체적으로 같은 추상화 레벨로 만든다. if문 내의 조건식도 함수로 만들고, test.split()
또한 함수로 만들어준다. 굳이 이렇게 까지 해야하나 싶지만 바꿔놓고 보면 훨씬 코드가 깔끔해진다. 이것이 개발의 보람이랄까?
첫 코드와 리팩토링 결과 코드를 비교해보자
리팩토링 전
public class StringCalculator {
public static int splitAndSum(String text) {
int result = 0;
if (text == null || text.isEmpty()) {
result = 0;
} else {
String[] values = text.split(",|:");
for (String value : values) {
result += Integer.parseInt(value);
}
}
return result;
}
}
리펙토링 후
public class StringCalculator {
public static int splitAndSum(String text) {
if (isBlank(text)) { // 1단계 추상화
result = 0;
}
return sum(toInts(split(text)); // 1단계 추상화
}
private static boolean isBlank(String text) {
return text == null || text.isEmpty();
}
private static String[] split(String text) {
return text.split(",|:");
}
private static int[] toInts(String[] values) {
[...]
}
private static int sum(int[] numbers) {
[...]
}
}
사람마다 다르겠지만 다른 사람이 이 두 코드를 받게 된다면 리팩토링 후의 코드가 더 보기 쉽다고 본다.
다음 편은 클래스.