6. 지워서 개선하기 (StringUtil, MapUtil)

리팩토링을 하고 싶었으나..

개발자 G는 주도적으로 개선할 수 있는 업무를 찾기 시작했다. 개선해야 할 대상과 방법은 너무나도 다양하겠지만, G는 당장 해낼 수 있으면서도 장기적으로 도움이 될 수 있는 것에 집중하기로 했다. 그 방법은 당장 개선할 수 있는 범위 찾기이다. 인프라 레벨, 프로덕트 레벨, 개개의 서버 레벨.. 전부 범위도 크고 결재도 나지 않을 것들이다. G는 소스 레벨의 개선하기로 목표를 세운다.

소스 레벨의 개선 프로젝트 중 가장 눈에 띈 것은 바로 '미사용 코드를 제거하거나, 불필요한 공통화 코드를 리팩토링하는 것'이다. 지금까지 업무를 진행하면서 들어오던 부분인데, 실제 소스를 보는 순간 G는 한숨이 나왔다.

Wrapping Method

공통화된 유틸 클래스에 isEmpty() 라는 메서드가 있다.

public static boolean isEmpty(String input) {
        return input == null || input.equals("");
    }

input String 을 파라미터로 받아서 빈 값인지를 체크하는 간단한 메서드이다. 로직상 오류는 없지만, 아래의 코드와 비교해본다.

    public static boolean isEmpty(CharSequence cs) {
        return cs == null || cs.length() == 0;
    }

전자는 프로덕트 내부에 구현된 isEmpty() 이고, 후자는 org.apache.commons.lang3.StringUtils 라는 아파치 표준 라이브러리의 isEmpty() 이다. 즉 프로덕트의 소스가 StringUtils 를 사용하지 않고 자체적으로 구현한 동일한 내용의 메서드를 공통으로 사용하겠다는 이야기인데, 라이브러리 의존성(?)을 높이지 않고 싶었던 것인지.. 전부 후자를 사용하도록 교체한다.

Integer 의 경우에도 동일한 문제점을 가지고 있다.

public static int getInteger(String input) { 
	int result = -1;
    if (!input.equals("")) result = Integer.parseInt(input);
	return result;
}

String 으로 들어오는 input 의 빈 문자열 체크를 하고 싶은 의도이지만, 결국 사용은 Integer.parseInt() 를 wrapping 하는게 전부인 불필요한 코드이다. 심지어 -1 이 반환될 수도 있으므로 의미마저 모호하다. 전부 Integer.parseInt() 로 교체하고 NumberFormatException 에 대한 예외처리를 추가한다.

Hard Coding

다음은 세션에서 값을 받아와 map 에 저장하는 메서드이다.

public static void setSessionVal(HttpServletRequest request, Map map) {
    if (map == null) map = new HashMap();
    HttpSession session = request.getSession();
    map.put("A", session.getAttribute("A"));
    map.put("B", session.getAttribute("B"));
    map.put("C", session.getAttribute("C"));
    ...
}

반환 타입이 void 여서 map 이 무엇을 하는 것인지도 모르고, map 이 비어있는 경우에는 new 키워드로 초기화까지 수행한다. 그리고 세션과 map 의 key-value 를 전부 String 으로 하드코딩한다. 필요한 곳에서 session.getAttribute() 를 따로 사용해서 값을 얻도록 수정한다.

Not Refactoring But Deleting

G는 위와 같은 코드의 wrapping 을 제거하거나 hard coding 을 제거하면서 수천줄의 코드를 삭제했고, 서버는 동일한 기능을 이상없이 수행했다. 이것은 리팩토링인가? 리팩토링은 소프트웨어 공학에서 '결과의 변경 없이 코드의 구조를 재조정함' 을 의미한다(출처: 위키백과). 결과의 변경은 없고, 코드의 구조가 바뀐 것도 맞다. 하지만 G는 이 변경사항을 커밋하면서 리팩토링이라는 단어를 사용하지 않았다. 이것은 그저 불필요한 코드를 지운 것에 불과하다고 생각했기 때문이다. 불필요한 코드를 지우지 않는 개발. 머리아픈 날들의 연속이다.