본문 바로가기
이슈와해결

리팩토링 회고 - 복잡한 코드, 중복 코드, 비효율적 코드 개선 경험

by Renechoi 2023. 12. 12.

0. 개요

본 글은 사내 자동화 툴의 사용성 확장에 대비하여 코드를 개선한 리팩토링 내용을 다룹니다. 처음 설계에서는 특정 도메인만을 대상으로 하였는데 요구사항이 확대되어 다른 도메인에서도 사용하게 된 경우입니다. 이와 같은 비즈니스 요구사항에 대한 대응으로 코드 레벨의 복잡성을 개선한 경험을 공유하고자 합니다.

 

  • 예시 코드는 실제 코드가 아닌 컨셉 코드로 대체하였습니다.

 

1. 목차

  1. 개요
  2. 목차
  3. 1차 리팩토링 - Deserializer를 일반화하여 41개의 중복 클래스를 통합하기
    1. 문제 정의
    2. 목표 설정
    3. 리팩토링 전략
    4. 결과 및 평가
  4. 2차 리팩토링 - enum의 제거와 대안
    1. 문제 정의
    2. 목표 설정
    3. 리팩토링 전략
    4. 결과 및 평가
  5. 결론

2. 1차 리팩토링

2.1 문제 정의

문제 상황

기존의 Deserializer 구현은 각 Payload 별로 분리되어 있어, 중복 코드와 복잡도가 높았습니다.

 

As-Is

  • Payload마다 별도의 Deserializer 클래스가 정의되어 있습니다.
  • 이를 테면 Deserializer1, Deserializer2, Deserializer3... 이런 식인 것이죠.
  • PayloadAction enum에서 Deserializer 클래스를 관리하며, 각 Payload 별로 다른 Deserializer 클래스를 사용하고 있었습니다.

 

public enum PayloadAction  {
    PayloadAction1(Deserializer1.class),
    PayloadAction2(Deserializer2.class),
    PayloadAction3(Deserializer3.class),
    PayloadAction4(Deserializer4.class),
    PayloadAction5(Deserializer5.class),
   // ...
}

 

세부 구현 예시는 다음과 같습니다.

 

@Component
public class Deserializer1 implements PayloadDeserializer<Payload1> {
    @Override
    public Payload1 deserialize(JsonObject json) {
        return GsonConverter.getGson().fromJson(json, Payload1.class);
    }
}

 

이러한 것이 41 개... !!!  

 

public class SampleFormatter implements Formatter {

    @Override
    public <P> Message<P> create(int messageTypeId, String uniqueId, PayloadActionResolver payloadActionResolver, JsonArray jsonArray, LogEvent previousLogEvent) {
        / ...
        return Message...
    }
}

 

문제점 분석

1. 중복 클래스

Payload 별로 별도의 Deserializer 클래스가 존재하며, 이로 인해 Payload 개수 만큼인 41개의 중복 클래스가 발생하였습니다.

 

Deserializer들은 동일한 코드 구조를 가지고 있으나, 각각 별도의 클래스로 정의되어 있어 중복이 발생합니다.

 

2. 유지보수성 저하

중복 코드로 인해 추가 Payload 발생 시 다수의 수작업이 요구되어 유지보수가 어려운 상황입니다. 새로운 Payload를 추가할 때마다 Deserialzer 클래스와 enum 상수를 추가해야 합니다.

 

3. 확장성 제한

중복 코드로 인해 Payload 또는 Deserialization 로직의 변경이 발생할 때 마다 모든 Deserializer 클래스를 수정해야 하므로 유지보수가 어렵죠.

 

이러한 문제점들로 인해 코드의 복잡성이 증가하였고, 이를 해결하기 위해 리팩토링이 필요한 상황이었습니다.

 

2.2 목표 설정

    • General Deserialzer를 구현하여 Payload 별 작성된 Deserializer 통합 -> 중복 코드 제거

 

 

2.3 리팩토링 전략

GenericPayloadDeserializer 도입

기존의 Payload 별로 정의된 Deserializer 클래스들은 거의 동일한 로직을 포함하고 있으며, 이를 통합할 필요가 있었습니다. 따라서, 제너럴한 GenericPayloadDeserializer 클래스를 도입하여 기존의 Deserializer들을 하나로 통합하였습니다.

 

public class GenericPayloadDeserializer {
    public static Payload deserialize(JsonObject json, PayloadActionV2 payloadAction) {
        Class<? extends Payload> payloadClass = payloadAction.getPayloadClass();
        return GsonConverter.getGson().fromJson(json, payloadClass);
    }
}

 

PayloadAction 변경

 

원래의 PayloadActionDeserializer 클래스를 관리하고 있었으나, 리팩토링 과정에서 각 Payload의 클래스 정보만을 관리하도록 변경하였습니다. 이렇게 하여 Payload의 클래스 정보를 바탕으로 동적으로 Deserialization을 수행하도록 했고 중복된 클래스들을 제거할 수 있었습니다.

 

@Getter
public enum PayloadActionV2 {
    Payload1(Payload1.class),
    Payload2(Payload2.class),
    // ...
    ;

    private final Class<? extends Payload> payloadClass;

    PayloadActionV2(Class<? extends Payload> payloadClass) {
        this.payloadClass = payloadClass;
    }
}

 

실제 사용되는 로직

리팩토링의 적용으로 GenericPayloadDeserializer가 주요 로직을 처리하게 되었으며, 기존의 Deserializer를 사용하던 SampleFormatter1SampleFormatter2 클래스도 리팩토링이 가능해졌습니다.

 

public class SampleFormatter1 implements Formatter {
    @Override
    public <P> Message<P> create(int messageTypeId, String uniqueId, JsonArray jsonArray, LogEvent previousLogEvent) {
         // ...
        return Message....
    }
}

 

각각의 페이로드를 처리하는 별도의 클래스가 필요 없게 되었으며, 모든 페이로드가 GenericPayloadDeserializer를 통해 일관되게 처리됩니다. 이로 인해 코드의 복잡성이 크게 감소하고, 향후 페이로드가 추가되거나 변경될 경우에도 유연하게 대응할 수 있게 되었습니다.

2.4 결과 및 평가

코드 줄임

기존의 각 Payload 별로 정의된 41개의 Deserializer 클래스를 GenericPayloadDeserializer 하나로 통합하면서 인지 부담이 크게 감소한 건 말할 것도 없이 행복해집니다. 코드의 가독성과 관리 편의성에 크게 기여했습니다.

 

확장성 향상

리팩토링을 통해 새로운 Payload가 추가될 경우에도 PayloadActionV2 enum에만 해당 Payload 클래스를 추가하면 되므로, 쉽게 확장할 수 있는 구조가 되었습니다.

 

유지보수성 향상

리팩토링 전에는 각 Payload 별로 별도의 Deserializer 클래스를 관리해야 했으나, 이제는 일반화된 GenericPayloadDeserializer를 통해 모든 Payload를 처리할 수 있게 되므로 유지보수가 훨씬 쉬워졌습니다. 중복 코드로 인한 오류 발생 가능성도 줄어들었구요.

 

3. 2차 리팩토링

3.1 문제 정의

위의 리팩토링을 통해 중복 클래스를 제거하고 유지보수성과 확장성을 향상시켰지만 개선의 여지가 남아 있었습니다. 추가 리팩토으로서 PayloadAction enum 완전히 제거하고 Reflection을 이용해 동적으로 클래스를 얻고자 하였는데요. 그렇게하면 새로운 Payload를 추가해야할 때 다른 부가적인 코드 구현 없이 (기존은 enum 등록과 deserializer 구현) 순수하게 해당 Payload만 작성하면 됩니다.

 

문제 상황

1차 리팩토링에서 GenericPayloadDeserializer를 도입하면서 중복 클래스를 크게 줄였지만, PayloadActionV2 enum이 여전히 존재합니다. 이 enum은 각 Payload의 클래스 정보를 관리하고 있었으며, 새로운 Payload가 추가될 때마다 enum에 상수를 추가해야 하는 문제를 갖고 있죠.

 

As-Is

PayloadActionV2 enum이 다음과 같이 Payload와 그에 해당하는 응답 클래스를 관리합니다.

 

@Getter
public enum PayloadActionV2 {
    PAYLOAD1(Payload1.class),
    PAYLOAD2(Payload2.class),
    //...


    private final Class<? extends Payload> payloadClass;

    PayloadActionV2(Class<? extends Payload> payloadClass) {
        this.payloadClass = payloadClass;
    }
    //...
}

 

3.2 목표 설정

  • enum 제거: PayloadActionV2 enum을 제거하고 동적으로 클래스 정보를 얻어 처리하도록 개선합니다.
  • 유연성 향상: 새로운 Payload 추가 시 코드 변경 없이 자동으로 인식하도록 구조 개선합니다.

 

3.3 리팩토링 전략

PayloadClassResolver 도입

enum을 제거하면 상수로서 선언된 클래스명들을 어떻게 요청에 맞게 인지하여 매핑할 것이냐의 문제가 생깁니다.

 

이를 Reflections라이브러리를 이용하여 동적으로 패키지 내의 Payload 클래스를 자동으로 스캔하는 방식으로 해결합니다.

 

이때 매 로그 요청마다 Payload 클래스를 찾는 비용이 너무 크고 1회성으로 찾고 사용해도 되기 때문에 해당 클래스 정보를 캐시하여 관리하는 방식으로 구성합니다. 해당 클래스의 static 블록은 어플리케이션 시작시에 실행되며 요건을 충족하는 클래스를 찾아 캐시 메모리에 저장합니다.

 

public class PayloadClassResolver {

    private static final Map<String, Class<? extends Payload>> payloadClassCache = new HashMap<>();
    private static final Map<String, Class<? extends Payload>> responsePayloadClassCache = new HashMap<>();

    static {
        Reflections reflections = new Reflections("kr.co.......payload");
        Set<Class<? extends Payload>> allPayloadClasses = reflections.getSubTypesOf(Payload.class);

        allPayloadClasses.forEach(clazz -> {
            String payloadClass = clazz.getCanonicalName();
          /// 캐시에 담는 로직 
    }

    public static Class<? extends Payload> resolvePayloadClass(String actionString) {
        return ;
    }

    public static Class<? extends Payload> resolveResponsePayloadClass(String actionString) {
        return ;
    }
}

 

Payload 클래스 정보는 어플리케이션 실행 동안 변경되지 않으므로, 최초 스캔 후 결과를 캐시 메모리에 저장하여 재사용합니다. 이로써 성능이 향상되며, 로그 요청마다 클래스를 찾는 비용을 절약할 수 있습니다.

 

단, 이 Resolver가 제대로 작동하기 위해서는 Payload 클래스 이름 규칙이 정확하게 정의되어 있어야 합니다.

 

본 프로젝트에서는 예를 들어 "Authorize" 작업에 대한 요청 클래스는 AuthorizePayload이고 응답 클래스는 AuthorizePayload.Response로 정의하였습니다.

GenericPayloadDeserializerV2 수정

GenericPayloadDeserializerV2에서 위에서 구현한 PayloadClassResolver를 사용합니다. 필요한 Payload 클래스를 동적으로 가져와 처리합니다.

 

3.4 결과 및 평가

enum 제거

리팩토링을 통해 PayloadActionV2 enum을 완전히 제거하였으며, 이로 인해 Payload 추가 시 별도의 코드 변경이 필요 없게 되었습니다. 아주 큰 성과죠 !

 

유연성과 확장성 향상

새로운 Payload가 추가되더라도 PayloadClassResolver가 해당 클래스를 자동으로 인식하고 처리하므로, 코드의 유연성과 확장성이 크게 향상되었습니다.

 

유지보수성 향상

동적 클래스 정보 관리를 통해 코드의 복잡성이 감소하고 유지보수가 훨씬 쉬워졌습니다.

 

4. 결론

로그 구조화 작업에서 Payload를 생성하는 기존 로직은 꽤 복잡했습니다. 사실 제네릭 타입을 제대로 사용하지 못해서 한 번 늘어나기 시작한 중복코드가 눈덩이처럼 불어났던 상황인데요. Payload의 개수가 많고 메시지 타입도 종류가 여러 개라 Json 구조를 객체로 매핑하는 과정에서 여러 클래스들이 필요했습니다. 이 리팩토링 작업을 통해 동적으로 클래스 정보를 관리하는 방법을 도입함으로써 Payload 생성의 플로우를 간결하고 효율적으로 만들었습니다. 중복된 코드가 눈에 띄게 줄어 인지 비용을 크게 줄였고 새로운 Payload 작성시에도 부담이 크게 줄어듭니다.

 

이 작업을 하면서 다음과 같은 추가 고민들도 해보았습니다.

 

enum에 대한 생각

자바의 enum은 상수 값의 그룹을 표현하는 데 매우 유용합니다. 코드 내에서 사전 정의된 상수 집합을 사용하면, 코드의 안정성과 가독성을 높일 수 있으나 확장성 측면에서 제약이 있는 것은 또 분명합니다. 예를 들어, 새로운 상수 값을 도입하거나 기존 상수 값을 변경하려면 해당 enum 클래스를 직접 수정해야 하는 것이죠. 이러한 변경은 컴파일 시점에서 확인되기 때문에, 유연한 구조 변경이 어렵습니다. 리팩토링 과정에서 enum을 제거하고 동적으로 클래스 정보를 관리함으로써 이러한 제약을 해결하였는데 확장성vs안정성 구도에서 적절한 조율이 필요할 수 있음을 생각해볼 수 있었습니다.

 

static 블록을 사용해 어플리케이션 구동 초기시 여러가지 설정을 해두는 방식에 대한 생각

어플리케이션 시작 시 필요한 초기화 로직을 static 블록 내에서 처리하는 방식은 특정 클래스나 리소스가 처음 로드될 때 한 번만 실행되는 로직을 정의하는 데 유용합니다. 예를 들어, 리팩토링 과정에서 도입한 PayloadClassResolver의 경우, static 블록을 사용하여 어플리케이션 시작 시 한 번만 Payload 클래스를 스캔하고 캐시에 저장할 수 있게 해줍니다. 이렇게 하면 로그 요청마다 클래스를 찾는 비용을 절약할 수 있습니다. 사실 이 비용 절감 효과는 매우 크다고 생각합니다. 스프링 빈 컨테이너도 목적은 좀 다를 수 있지만 원리는 비슷하구요.

 

그럼에도, 이와 같은 방식 역시 최선일까 라는 의문은 있습니다. 예를 들어, 해당 클래스는 그렇다면 어떻게 테스트할 것인가? 실제 개발 과정에서는 디버깅 모드를 통해 확인했는데요. 테스트 코드를 작성하려면 통합 테스트가 되어야 할 것이고 검증 코드를 작성하는 부담도 적지 않을 것 같습니다. 그도 그렇지만 근본적으로는 결국 '정적 vs 동적' 컨셉에 대해 사용되는 로직이 어느 쪽으로 풀어야 맞는 것인가에 대한 문제라고 봅니다. 즉, 로직이나 객체의 본질적인 목적? 속성? 같은 것이 동적이냐 정적이냐 이를 결정하는 문제일 것이고 이 부분은 어떤면에서는 코드 레벨에서만 생각해볼 문제는 아닐 것이기 때문이겠죠.

 

아무튼... 정적인 접근의 이점과 취약점도 함께 잘 따져봐야 할 것 같다는 생각을 했습니다.

반응형