우아한테크코스의 MVC미션을 진행하던 중 다음과 같은 리뷰를 받았다.

스트림의 forEach를 이용해서 로직을 수행하게 되면 가독성도 떨어지고 동시성을 보장할 수 없다.

또한, Stream은 부수 효과가 없는 함수형 프로그래밍을 위해 만들어진 것이므로 Stream의 의도와도 맞지 않다.

이펙티브 자바 아이템 46에서도 부작용 없는 순수 함수를 사용하라고 이야기하고 있다.

 

1. 기존 코드

문제의 코드를 보고 개선해보자.

private final Map<HandlerKey, HandlerExecution> handlerExecutions;

public void initialize() {
    ...
    final var methods = clazz.getMethods();
    Arrays.stream(methods)
            .forEach(method -> putHandlerExecutions(handler, method));
}

private void putHandlerExecutions(final Object handler, final Method method) {
    final var annotation = method.getDeclaredAnnotation(RequestMapping.class);
    final var handlerExecution = new HandlerExecution(handler, method);

    Arrays.stream(annotation.method())
            .map(requestMethod -> new HandlerKey(annotation.value(), requestMethod))
            .forEach(handlerKey -> handlerExecutions.put(handlerKey, handlerExecution));
}

 

putHandlerExecutions에서는 annotation.method() 배열의 원소 만큼

handler와 method로 Map<HandlerKey, HandlerExecution>의 Entry를 생성해서

인스턴스 변수인 handlerExecutions에 put을 하고 있다.

 

2. 개선된 코드

먼저 완성될 결과를 보자.

private final Map<HandlerKey, HandlerExecution> handlerExecutions;

public void initialize() {
            final var methods = clazz.getMethods();
            final var executionMap = createHandlerExecutionMap(handler, methods);
            handlerExecutions.putAll(executionMap);
}

Map<HandlerKey, HandlerExecution>을 생성하는 createHandlerExecutionMap으로 메서드로 추출하여 해당 메서드에서 Map을 완성한다.

이를 handlerExecutions.putAll로 맵의 모든 엔트리를 삽입해주었다.

 

핵심인 createHandlerExecutionMap을 살펴보자.

private Map<HandlerKey, HandlerExecution> createHandlerExecutionMap(final Object handler,
        final Method[] methods) {
    return Arrays.stream(methods)
            .map(method -> toHandlerExecutions(handler, method))
            .flatMap(map -> map.entrySet().stream())
            .collect(Collectors.toMap(Entry::getKey, Entry::getValue));
}

요소인 method마다 toHandlerExecutions(handler, method)를 통해 Map<HandlerKey, HandlerExecution>으로 만들어준다.

해당 Map이 여러개이기 때문에 flatMap으로 평탄화 하고 모든 Entry에 대한 스트림을 다시 만들어 준다.

결과의 key와 value를 다시 Map으로 collect 한다.

 

toHandlerExeutions는 다음과 같이 단순하게 파라미터를 통해 HandlerKey와 HandlerExecution을 만들어서 Map으로 맵핑해준다.

private Map<HandlerKey, HandlerExecution> toHandlerExecutions(
        final Object handler, final Method method) {
    final var annotation = method.getDeclaredAnnotation(RequestMapping.class);
    final var handlerExecution = new HandlerExecution(handler, method);

    return Arrays.stream(annotation.method())
            .map(requestMethod -> new HandlerKey(annotation.value(), requestMethod))
            .collect(Collectors.toMap(Function.identity(), handlerKey -> handlerExecution));
}

 

결론

스트림으로 만들기 어렵다고 무작정 forEach로 해결하거나 for-loop로 해결하기 보다 고민을 조금 해보면 충분히 해결할 수 있다. 위 방법은 여러개의 Map을 만들고 이를 합치는 과정을 통해 해결했다.

이 방식 이외에 method 원소 마다 createHandlerExecutionMap에서 Map을 생성하여 합치는 방식이 아닌 createHandlerExecutionMap에서 Stream<AbstractMap.SimpleEntry<HandlerKey, HandlerExecution>> 즉, 엔트리의 스트림을 반환하여 스트림의 중간 연산으로 활용할 수도 있다.

하지만 이러한 방법은 복잡도가 증가하고 Map의 key에 대한 중복 처리가 어떻게 될지 예상이 되지 않는다.

Map을 생성하여 합치는 방식보다 메모리가 효율적이기는 하나 그 정도까지 고려할 필요는 없다고 생각한다.

 

참고

+ Recent posts