최근 간단 리팩토링 사례

요즘 코딩 하다가 일단 돌아가게 만들고 리팩토링 한 코드가 있어서 공유해 본다. 실제 완전한 코드는 공개할 수 없어 일부 이름을 변경했다.

대충 시작한 코드는 아래와 같다. 아래 코드는 "접두어:포맷"으로 구성된 홀더파트(holderPart)에서 필요한 값을 추출한다.

while(leftCur != -1) {

    ...

    String holderPart = ...;

    if (holderPart.startsWith("date:")) {

        String holderFormat = holderPart.substring("date:".length());

        result.add( new HolderFormat(HolderType.DATE, holderFormat) );

        regEx += "(.{" + holderFormat.length() + "})";

    }

    ...

}

위 코드는 홀더파트의 접두어가 "date"면, HolderType.DATE에 해당하는 HolderFormat을 생성한다. 그리고, 관련 정규 표현식으로 "(.{포맷길이})"를 추가한다.

HolderType은 접두어에 해당하는 타입 목록을 정의한 enum 타입으로 코드는 다음과 같다.

public enum HolderType {

    DATE("date"), NAME("name"), TS("ts"), ANY("any");

    private String name;   

    

    private HolderType(String name) {

        this.name = name;

    }

    public String getName() {

        return name;

    }

}

필요한 기능을 점진적으로 구현하다보니 코드가 다음과 같이 바뀌었다.

while(leftCur != -1) {

    ...

    String holderPart = ...;

    if (holderPart.startsWith("date:")) {

        String holderFormat = holderPart.substring("date:".length());

        result.add( new HolderFormat(HolderType.DATE, holderFormat) );

        regEx += "(.{" + holderFormat.length() + "})";

    } else if (holderPart.startsWith("name:")) {

        String holderFormat = holderPart.substring("name:".length());

        result.add( new HolderFormat(HolderType.NAME, holderFormat) );

        regEx += "(" + holderFormat + ")";

    } else if (holderPart.equals("ts")) {

        result.add(new HolderFormat(HolderType.TS, ""));

        regEx += "([0-9]+)";

    } else if (holderPart.equals("any")) {

        result.add(new HolderFormat(HolderType.ANY, ""));

        regEx += "(.*)";

    } else {

        throw new IllegalArgumentException(String.format("not supported holder '%s'", holderPart));

    }

    ...

}

if-else로 구성된 코드를 보면 각 if 블록이 다음과 같이 구성된 것을 알 수 있다.

• 홀더 파트가 특정 접두어에 해당하는지 검사하고,
• 접두어에 따라 포맷 부분을 생성
• 접두어에 해당하는 정규 표현식을 생성

정규 표현식을 생성할 때 포맷 부분을 사용하는 경우도 있고, 아닌 경우도 있다. 예를 들어, 접두어가 date나 name인 경우는 포맷 부분을 이용해서 정규 표현식을 생성하지만, ts인 경우는 포맷 부분과 상관없이 정규 표현식으로 "([0-9]+)"를 추가한다. 또한, 포맷 부분을 사용하더라도 접두어가 date인 경우는 포맷의 길이를 정규 표현식에 사용하는 반면 접두어 name인 경우는 포맷 부분을 그대로 정규 표현식에 사용한다.

즉, if 블록의 코드는 동일한 행위(접두어 비교, 포맷 생성, 정규 표현식 생성)를 하는 코드인데, 실제 행위의 구현은 접두어에 따라 달라지는 코드이다. 여기서 접두어는 HolderType에 해당하므로, 실제 행위는 HolderType에 따라 달라진다고 할 수 있다.

HolderType에 따라 기능이 달라진다는 것은 그 기능이 HolderType에 있어야 할 가능성이 높아 보인다. 그래서, if 블록의 코드를 HolderType으로 이전하기 시작했다. 이전을 위해 작성한 첫 번째 코드는 다음과 같다.

public enum HolderType {

    DATE("date") {

        @Override

        public FormatAndRegex extract(String holderValue) {

            String regex = "(.{" + holderValue.length() + "})";

            return new FormatAndRegex(holderValue, regex);

        }

    },

    NAME("name"), TS("ts"), ANY("any");

    private String nameVal;

    private HolderType(String name) {

        this.nameVal = name;

    }

    public String getName() {

        return nameVal;

    }

    public boolean matchName(String holderName) {

        return this.nameVal.equals(holderName);

    }

    public FormatAndRegex extract(String holderValue) { return null; }

    public static class FormatAndRegex {

        public final String format;

        public final String regex;

        public FormatAndRegex(String format, String regex) {

            this.format = format;

            this.regex = regex;

        }

    }

}

if 블록에서 수행한 기능 세 가지를 위해 추가한 코드는 다음과 같다.

• 홀더파트가 특정 접두어에 해당하는지 비교 -> matchName() 메서드
• 접두어에 해당하는 포맷 생성, 정규 표현식 생성 -> extract() 메서드
• 포맷과 정규 표현식을 담기 위한 FormatAndRegex 클래스 구현

첫 번째로 이전한 기능이 DATE와 관련되어 있으므로 if-else 블록에서 "date" 부분을 다음과 같이 변경했다.

while(leftCur != -1) {

    ...

    String holderPart = ...;

    String[] holderElements = holderPart.split(":", 2);

    String holderName = holderElements[0];

    String holderValue = holderElements.length == 1 ? "" : holderElements[1];    

    if (HolderType.DATE.matchName(holderName)) {

        FormatAndRegex formatAndReg = HolderType.DATE.extract(holderValue);

        result.add( new HolderTypeFormat(HolderType.DATE, formatAndReg.format) );

        regEx += formatAndReg.regex;

    } else if (holderPart.startsWith("name:")) {

        String holderFormat = holderPart.substring("name:".length());

        result.add( new HolderTypeFormat(HolderType.NAME, holderFormat) );

        regEx += "(" + holderFormat + ")";

    } else if (holderPart.equals("ts")) {

        result.add(new HolderTypeFormat(HolderType.TS, ""));

        regEx += "([0-9]+)";

    } else if (holderPart.equals("any")) {

        result.add(new HolderTypeFormat(HolderType.ANY, ""));

        regEx += "(.*)";

    } else {

        throw new IllegalArgumentException(String.format("not supported holder '%s'", holderPart));

    }

    ...

}

나머지 홀더타입과 관련된 코드도 차례대로 HolderType으로 이관하고 그에 맞춰 if-else 블록 코드를 수정해 나갔다.

while(leftCur != -1) {

    ...

    String holderPart = ...;

    String[] holderElements = holderPart.split(":", 2);

    String holderName = holderElements[0];

    String holderValue = holderElements.length == 1 ? "" : holderElements[1];    

    if (HolderType.DATE.matchName(holderName) {

        FormatAndRegex formatAndReg = HolderType.DATE.extract(holderValue);

        result.add( new HolderTypeFormat(HolderType.DATE, formatAndReg.format) );

        regEx += formatAndReg.regex;

    } if (HolderType.NAME.matchName(holderName) {

        FormatAndRegex formatAndReg = HolderType.NAME.extract(holderValue);

        result.add( new HolderTypeFormat(HolderType.NAME, formatAndReg.format) );

        regEx += formatAndReg.regex;

    } if (HolderType.TS.matchName(holderName) {

        FormatAndRegex formatAndReg = HolderType.TS.extract(holderValue);

        result.add( new HolderTypeFormat(HolderType.TS, formatAndReg.format) );

        regEx += formatAndReg.regex;

    } if (HolderType.ANY.matchName(holderName) {

        FormatAndRegex formatAndReg = HolderType.ANY.extract(holderValue);

        result.add( new HolderTypeFormat(HolderType.ANY, formatAndReg.format) );

        regEx += formatAndReg.regex;

    } else {

        throw new IllegalArgumentException(String.format("not supported holder '%s'", holderPart));

    }

    ...

}

위 if-else 블록은 enum 타입 값만 다를 뿐 완전 동일하다. 그래서, 위 코드를 다음과 같이 for 문을 이용해서 변경했다.

while(leftCur != -1) {

    ...

    String holderPart = ...;

    String[] holderElements = holderPart.split(":", 2);

    String holderName = holderElements[0];

    String holderValue = holderElements.length == 1 ? "" : holderElements[1];    

    

    HolderType matchHolderType = null;

    for (HolderType holderType : HolderType.values()) {

        if (holderType.matchName(holderName)) {

            matchHolderType = holderType;

            break;

        }

    }

    if (matchHolderType == null) throw .....

    FormatAndRegex formatAndReg = matchHolderType.extract(holderValue);

    result.add( new HolderTypeFormat(matchHolderType, formatAndReg.format) );

    regEx += formatAndReg.regex;

    ...

}

이후 몇 번의 리팩토링을 거쳐 코드를 조금씩 개선해 나갔다. 사소한 것 하나를 언급하자면 while에서 사용한 leftCur 변수는 실제로는 "{ .... }" 형식의 문자열에서 "{"의 위치를 뜻하는데, "{"는 홀더파트를 열고 "}"는 홀더파트를 닫는다는 의미를 갖는다. 그래서, leftCur를 openCur로 이름을 바꿨다.

while(openCur != -1) {

    ...

}

그리고, (openCur != -1)은 실제로는 홀더파트를 여는 괄호를 찾았는지를 의미하므로, 코드를 다음과 같이 바꿨다.

while(foundOpenCur()) {

    ...

}

private boolean foundOpenCur() {

    return openCur != -1;

}

점진적으로 메서드를 추출하면서 while() 부분이 다음과 같이 바뀌었다.

while (foundOpenCur()) {

    ...

    findCloseCurAfterOpenCur();

    parseHolderPartAndAddToResult();

    ...

}