Legacy code에 Lombok 적용은 지양하자

 

소개

오래 전부터 워낙 유명해서 Lombok을 사용하지 않는 곳이 없을 정도이다.

그러나 이번에 오랜 코드의 보일러 플레이트 코드들을 제거하기 위해서 Lombok을 도입했다.

하는 김에 Setter 구문을 제거하는 작업도 진행하였다.

코드만 보기에는 너무 깔끔하고 유지/보수가 편하다고 생각했었다. 

그러나 QA 기간이 들어가면서 생각지도 못한 부분에서 에러들이 계속 발생하고 나를 괴롭혔다..

만약 Legacy Code에 Lombok을 도입하고 싶은 분들이라면 아래 내용을 참고해서 고민해봤으면 한다..

 

문제상황 1: Boolean 형식의 DTO

아래의 예시를 보면 boolean 형식의 get 메서드는 어떤 형태로 생성될까?

@NoArgsConstructor
@AllArgsConstructor
@Getter
public class ArgumentM {
    private String name;
    private String value;
    private boolean isEncrypted;
}

 

필드명을 isEncrypted, encrypted 둘 다 isEncrypted로 생성된다. 이게 무슨 문제가 될까?

기존은 Legacy Code에선 아래와 같이 Getter를 만들어 사용했다.

public boolean getIsEncrypted() {
    return isEncrypted;
}

 

그러나 Lombok 도입으로 Getter가 달라져 @RequestBody에서 문제가 된다.

기존 코드에서 아래의 요청을 보내면 "true"로 받을 것이다.

그러나 Lombok 적용 코드에서는 FE 에서의 요청은 똑같지만 계속 "false"가 나올 것이다. 

이건 RequestBody에서 Getter를 이용해서 Binding하는데 Getter가 달라져서 바인딩이 안되서 생기는 문제이다.

boolean, Boolean의 Getter를 사용할 땐 주의하자.

{
    "name": "key",
    "value": "value",
    "isEncrypted": true
}

 

또 Tip으로 Boolean 변환에 관한 내용이다.

isEncrypted는 getIsEncrypted, encrypted는 getEncrypted로 해석되어진다.

결론적으로 Primitive Type 변환과 Refence Type 변환이 다르게 된다.

참고 : https://projectlombok.org/features/GetterSetter

 

문제상황 2: Reflection 및 @NoArgsConstructor, @Setter 사용

대부분 Hibernate 사용 시 @NoArgsConstructor에 PROTECTED를 붙여 생성자 접근 제한을 한다.

왜냐하면 Proxy만 생성하면 되기 때문이다.

그리고 Setter를 제거하는 방향이 Immutable한 객체를 유지할 수 있는 방법이다.

물론 위와 같은 제약이 꼭 필요하다. 그러나 레거시 코드들은 상당히 어렵다.

어딘가에서 Reflection 코드를 사용하고 있을 수 있다.

우리는 Entity - Model Mapping을 RestModelMapper라는 클래스를 가지고 변환한다.

내부적으로 Reflection을 사용한다.

public class RestModelMapper {
    private static final Logger logger = LoggerFactory.getLogger(RestModelMapper.class);

    private int features;

    public enum Feature {
        SKIP_ARRAY(false),
        SKIP_COLLECTION(false),
        SKIP_ARRAY_OR_COLLECTION(false),
        USE_RECURSIVE_MAPPING(false),
        ONLY_PRIMITIVE_OR_WRAPPER_OR_STRING(false),
        DIFFERENT_PARAMETERIZED_TYPE_COLLECTION_MAPPING(false);
		...
    }
 
    private <T, X> T convert(X source, Class<T> targetClass, TreeSet<Class> convertingClasses) {
            Class<?> sourceClass = source.getClass();

            //check cyclic dependencies
            if (convertingClasses.contains(sourceClass)) {
                return null;
            }

            convertingClasses.add(sourceClass);

            T targetValue = targetClass.newInstance();

            Field[] sourceFields = sourceClass.getDeclaredFields();
			....
	}
}

 

아래와 같은 에러가 발생한다.. 

@NoArgsConstructor의 PROTECTED도 좋지만 PUBLIC으로 해서 오류를 줄이는 방법이 더 낫다고 생각했다.

 

문제상황 3: @Builder 사용

 

빌더패턴은 좋은 패턴이다. 그러나 문제가 될 수 있는 부분도 있다.

많은 필드를 가진 DTO이다.

Legacy Code 특징 중 하나는 DTO에 무분별하게 많은 필드들이 생성된다는 것이다.

그러다 보니 진짜 많은 필드를 가진 DTO는 아래와 같은 형태를 가진다.(대략 60개 정도로 보인다...)

여기 빌더를 사용하면? 런타임 NPE 가 발생하기 가장 쉬운 형태이다.

이것을 알지못하고 빌더는 무조건 필드명을 알기 쉽고 코드를 짤 때 쉽게 사용할 수 있다고 사용해왔다.

public class TableDetailM implements Bookmarkable {
	private String dbName;
    private TableType tableType;
    private String storageType;
    private String name;
    ..(20개 생략)
    private String outputFormat;
    private boolean compressed;
    private Integer buckets;
    private String storageHandler;

    /*For SparkSql Table*/
    private String sparkSqlDatasourceType;
    /*For SparkSql Table*/
    private int bucketCount;
    private boolean deleteDataFileWhenDrop;

	...(20개 생략)
}

 

근본적으로 DTO를 변경해야하지만 계속되는 요구사항 및 안전성을 위해서 10개 이상의 필드를 가진 DTO는 유지하는게 좋아보인다.

만약 새로운 기능에서 DTO를 사용한다면 그에 맞는 생성자를 새로 만드는 방식이 나을 수 있다는 생각이 든다.

요즘은 아래 사진과 같이 IDE가 잘 동작하기에 생성자를 사용해도 문제없다고 생각한다.

 

결론

정답은 없다.

물론 Lombok 및 Legacy코드에 대한 이해도가 깊은 분이 사용한다면 문제가 발생 안 할 수도 있다.

테스트 코드가 없거나 부족하다면 순수 자바로 생성자, Getter, Setter를 지정해주는 방식이 좋다.