전체 보기
♻️

레거시 Exception Custom 로직 리팩토링(2)_중복되는 코드들(추상화, 다형성, tda)

작성일자
2024/01/09
태그
SPRING
프로젝트
FIS
책 종류
1 more property

5. 추상화 적용해 중복 코드 제거

(1) 문제점

도메인 별 exception을 인터페이스-구현체 형태로 분리해두긴 했는데,
public interface ErrorResult { HttpStatus getHttpStatus(); String getMessage(); }
Java
복사
@Getter @RequiredArgsConstructor public enum CenterErrorResult implements ErrorResult { NOT_EXIST_CENTER(HttpStatus.NOT_FOUND, "존재하지 않는 센터입니다."); private final HttpStatus httpStatus; private final String message; }
Java
복사
@Getter @RequiredArgsConstructor public enum UserErrorResult implements ErrorResult { NOT_EXIST_USER(HttpStatus.NOT_FOUND, "존재하지 않는 유저입니다."); private final HttpStatus httpStatus; private final String message; }
Java
복사
막상 이를 사용하는 쪽에선 인터페이스가 아닌 내부 구현체에 기반해 사용하는 형태를 보여주고 있다.
즉, 외부에서 내부 구현체를 알고 있는 형태다. 중복 코드도 굉장히 많다.
@ExceptionHandler({UserException.class}) public ResponseEntity<ErrorResponse> userException(UserException e) { ErrorResult errorResult = e.getErrorResult(); return ResponseEntity.status(errorResult.getHttpStatus()) .body(new ErrorResponse(errorResult.getHttpStatus(), errorResult.getMessage())); } @ExceptionHandler({CenterException.class}) public ResponseEntity<ErrorResponse> centerException(CenterException e) { ErrorResult errorResult = e.getErrorResult(); return ResponseEntity.status(errorResult.getHttpStatus()) .body(new ErrorResponse(errorResult.getHttpStatus(), errorResult.getMessage())); }
Java
복사
@Getter public class UserException extends RuntimeException { private UserErrorResult errorResult; public UserException() { super(); } public UserException(String message) { super(message); } public UserException(UserErrorResult errorResult) { super(); this.errorResult = errorResult; } } @Getter public class CenterException extends RuntimeException { private CenterErrorResult errorResult; public CenterException() { super(); } public CenterException(String message) { super(message); } public CenterException(CenterErrorResult errorResult) { super(); this.errorResult = errorResult; } }
Java
복사
얼핏 보기엔 사용하는 쪽인 Handler 가 인터페이스인 ErrorResult를 사용하는 것 같아 보이지만
자세히 뜯어보면 ErrorResult 구현체 하나 당 하나의 Exception 클래스를 가지고,
Handler는 ErrorResult 구현체의 개수만큼 존재하는 Exception 클래스들을 사용하고 있다.
ErrorResult 구현체가 추가되거나 변경되면
이에 일대일 대응하는 Exception 클래스 역시 추가되거나 변경될 것이고
결국엔 사용하는 쪽(Handler)이 그 영향을 받는다.
우리가 추상화, 다형성 등을 논하는 이유는 사용하는 쪽이 인터페이스만 알고 사용함으로써
구현체가 변경되는 것에 영향을 받지 않게 하기 위함인 데
지금의 코드는 인터페이스의 존재 의의가 퇴색된다.

(2) 해결책 : 추상화 적용

인터페이스를 제대로 사용해 추상화하자. 자연스레 중복 코드들도 없어질 것이다.
먼저 ErrorResult를 필드로 가지는 CustomException 이란 클래스를 만들어줬다.
@AllArgsConstructor public class CustomException extends RuntimeException { private ErrorResult errorResult; public ErrorResponse getErrorResponse() { return errorResult.getErrorResponse(); } }
Java
복사
그리곤 UserException, CenterException 등이 CustomException을 상속하게 해줬다.
public class CenterException extends CustomException { public CenterException(CenterErrorResult errorResult) { super(errorResult); } }
Java
복사
좀 더 덧붙이자면, 사실 현재 구조에선 CenterException을 쓰는 의미도 좀 퇴색된다고 본다.
물론 CustomException을 모든 곳에서 쓰는 것보다야 낫겠지만,
여전히 여러 곳에서 CenterException을 사용해야 하기에
굳이 클래스를 나누는 의미가 없어지는 느낌이다.
차라리 CustomException 하나만 사용하거나,
아싸리 에러 메시지 하나 당 하나의 Exception 클래스를 만들어 사용하거나 하는 식의
양극단 중 하나를 선택하는 게 낫지 않을까?
지금은 전자와 후자 사이 어딘가에 존재하는 애매한 코드다.
이에 대해선 아래 ‘고민한 부분’ 파트에서 후술하겠다.
다시 본론으로 돌아와서 CustomException 이 생겨서 얻는 이득은 뭘까?
ErrorResult를 사용하는 측인 Handler에서 이전처럼 ErrorResult 구현체들의 영향을 받지 않게 된다.
각 ErrorResult 구현체들을 다루는 각 Exception들을 따로 사용하지 않고,
CustomException이라는 부모 클래스만 사용하면 되기 때문이다.
@ExceptionHandler(CustomException.class) public ResponseEntity<ErrorResponse> CustomExceptionHandler(CustomException e) { ErrorResult errorResult = e.getErrorResult(); return ResponseEntity.status(errorResult.getHttpStatus()) .body(new ErrorResponse(errorResult.getHttpStatus(), errorResult.getMessage())); }
Java
복사

(3) 고민한 부분 : 예외 클래스의 구체화 정도

@AllArgsConstructor public class CustomException extends RuntimeException { private ErrorResult errorResult; public ErrorResponse getErrorResponse() { return errorResult.getErrorResponse(); } }
Java
복사
public class CenterException extends CustomException { public CenterException(CenterErrorResult errorResult) { super(errorResult); } }
Java
복사
위 구조에서 사용할 수 있는 아래 두 코드 각각의 장단점에 대해 고민했다.
// a throw new CustomException(CenterErrorResult.NOT_FOUND_CENTER); // b throw new CenterException(CenterErrorResult.NOT_FOUND_CENTER);
Java
복사
그러다 문득, 꼭 메시지를 인자로 넣어주어야 하나란 생각이 들었고
기존 구조를 리팩토링한 후 아래처럼 쓰는 것도 좋겠다 생각했다.
// c throw new CenterNotFoundException(); // d (c에 싱글톤 적용 시) throw CenterNotFoundException.EXCEPTION;
Java
복사
spring-boot 공식 레포지토리에서 찾아봤을 땐, 대체로 2와 3을 많이 쓴다.
내가 참고한 코드
각각의 장단점에 대해 정리해보겠다.
장점
단점
a
유연성과 재사용성 좋음
디버깅 시 에러 코드 만으로 예외 원인 파악하기 어려울 수 있음
b
-
재사용성이 3번 보단 좋지만 1번 보단 안 좋음 예외 파악이 1번 보단 쉽지만 3번 보단 안 좋음
c
예외 파악하기 쉬움
재사용성이 부족하고 많은 예외 유형을 처리하려면 클래스가 과하게 많아질 수 있음
d
예외 자주 발생하는 상황에서 메모리 효율이 좋음
스택 트레이스가 최초 발생 시점에 고정되어 예외가 발생한 정확한 위치 추적하기 어려울 수 있음
물론 정답은 없다. 각자의 상황과 취향에 맞는 선택을 하면 될 거 같다.
나 같은 경우 일단 확실히 현재 레거시의 구조인 b 방식은 명확한 장점이 없다 판단했고,
c 방식으로 리팩토링하기로 결정했다.
public class CenterNotFoundException extends CustomException { public CenterException() { super(CenterErrorResult.NOT_FOUND_CENTER); } } // throw new CenterNoutFoundException();
Java
복사

(4) 클린코드 : tda 적용

기존 구조에선 ErrorResult(enum) 값을 토대로 ErrorResponse(dto)를 생성하고 있다.
취지는 좋은데, 코드를 더럽게 만드는 건 ErrorResponse를 생성하는 위치다.
현재엔 ErrorResult를 사용하는 쪽에서 ErrorResult의 각 필드를 getter로 빼가서 ErrorResponse를 만들고 있다.
public record ErrorResponse(HttpStatus httpStatus, String error) { }
Java
복사
public interface ErrorResult { HttpStatus getHttpStatus(); String getMessage(); }
Java
복사
@Getter @RequiredArgsConstructor public enum CenterErrorResult implements ErrorResult { NOT_EXIST_CENTER(HttpStatus.NOT_FOUND, "존재하지 않는 센터입니다."); private final HttpStatus httpStatus; private final String message; }
Java
복사
@Getter @AllArgsConstructor public class CustomException extends RuntimeException { private ErrorResult errorResult; }
Java
복사
@ExceptionHandler(CustomException.class) public ResponseEntity<ErrorResponse> CustomExceptionHandler(CustomException e) { return ResponseEntity.status(e.getErrorResult().getHttpStatus()) .body(new ErrorResponse(e.getErrorResult().getHttpStatus(), e.getErrorResult().getMessage())); }
Java
복사
키워드가 하나 떠오르지 않는가? tda! (묻지 말고 시켜라)
현재 구조는 ErrorResult한테 ErrorResponse(dto)를 만들라 시키는 게 아닌,
ErrorResult한테 ErrorResut의 각 필드(status, messag)를 물어보고 있다.
그렇다면, tda를 적용하면 코드가 어떻게 바뀔까?
ErrorResult한테 ErrorResponse를 만들라 시키게 바꿈으로써
디미터 법칙을 지킬 수 있게 되었으며,
ErrorResult와 CustomException의 필드를 getter로 열어두지 않아도 되게 됐다.
(ErrorResponse는 dto라서 getter 열어두어도 됨)
결과적으로,
Handler가 CustomException과 ErrorResult의 모든 필드를 getter를 통해 알고 있던 상황에서,
Handler는 CustomException의 메서드 하나만 알면 되게 변경 되었다. (Handler는 사용하는 쪽)
이를 통해 객체 간 결합도를 낮출 수 있게 되었고 유지보수성과 가독성이 좋아졌다.
예를 들면 ErrorResponse dto의 필드가 변경될 때 기존엔 최소한 ErrorResult와 Handler가 영향을 필수로 받게 되었었는데, 변경된 코드에선 ErrorResult만 영향 받는다.
public interface ErrorResult { ErrorResponse getErrorResponse(); }
Java
복사
@AllArgsConstructor public enum CenterErrorResult implements ErrorResult { NOT_EXIST_CENTER(HttpStatus.NOT_FOUND, "존재하지 않는 센터입니다."), ; private final HttpStatus httpStatus; private final String message; @Override public ErrorResponse getErrorResponse() { // dto 생성 return new ErrorResponse(httpStatus, message); } }
Java
복사
@AllArgsConstructor public class CustomException extends RuntimeException { private ErrorResult errorResult; public ErrorResponse getErrorResponse() { // dto 전달 return errorResult.getErrorResponse(); } }
Java
복사
@ExceptionHandler(CustomException.class) public ResponseEntity<ErrorResponse> CustomExceptionHandler(CustomException e, HttpServletRequest request) { ErrorResponse errorResponse = e.getErrorResponse(); return ResponseEntity.status(errorResponse.httpStatus()) .body(errorResponse); }
Java
복사
한 걸음 더 나아가서, 아래 코드가 맘에 들지 않는 분이 계셨을 수도 있다.
@AllArgsConstructor public enum CenterErrorResult implements ErrorResult { NOT_EXIST_CENTER(HttpStatus.NOT_FOUND, "존재하지 않는 센터입니다."), ; private final HttpStatus httpStatus; private final String message; @Override public ErrorResponse getErrorResponse() { // dto 생성 return new ErrorResponse(httpStatus, message); } }
Java
복사
ErorrResult가 dto의 필드를 알고 있다보니, dto 필드가 변경됐을 때 ErorrResult의 모든 구현체들이 다 영향을 받기 때문이다.
유지보수를 더 용이하게 하기 위해 아래와 같이 변경할 수 있지 않나 질문할 수 있다.
@AllArgsConstructor public enum CenterErrorResult implements ErrorResult { NOT_EXIST_CENTER(HttpStatus.NOT_FOUND, "존재하지 않는 센터입니다."), ; private final HttpStatus httpStatus; private final String message; @Override public ErrorResponse getErrorResponse() { // dto 생성 return ErrorResponse(this); } }
Java
복사
public record ErrorResponse(HttpStatus httpStatus, String error) { public static ErrorResponse of(ErrorResult errorResult) { return new ErrorResponse(errorResult.getHttpStatus, errorResult.getMessage); } }
Java
복사
ErrorResult가 dto를 아는 게 아니라
dto가 ErrorResult를 알고 있게 변경해준 것이다.
하지만, 위 코드엔 함정이 있다. ErrorResult가 인터페이스기 때문이다.
(실제로 getHttpStatus 부분에 빨간줄이 뜬다)
인터페이스-구현체 구조가 아니라면,
model이 dto를 아는 것보단 dto가 model을 알게 만들어주는 방식이 좋겠지만
현재 구조 상으론 그렇게 하려면 결국 dto에 모든 구현체에 대한 정적팩토리메서드를 각각 만들어줘야 하고,
그렇게 된다면 유지 보수는 여전히 어렵다.
이 부분은 본인 취향에 맞게 선택하면 될 거 같다.
dto에 각 구현체에 대한 여러 정적 팩토리 메서드 만들기 vs 각 구현체에 메서드 만들기
메서드 개수는 똑같기에 한 파일에 모여있느냐 아니냐 정도의 차이인데 이건 정말 취향 차이인거 같다.
dto에 각 구현체에 대한 여러 정적 팩토리 메서드 만들기
public record ErrorResponse(HttpStatus httpStatus, String error) { public static ErrorResponse of(CenterErrorResult errorResult) { return new ErrorResponse(errorResult.getHttpStatus(), errorResult.getMessage()); } public static ErrorResponse of(CommentErrorResult errorResult) { return new ErrorResponse(errorResult.getHttpStatus(), errorResult.getMessage()); } // ... 이하 생략 }
Java
복사
각 구현체에 메서드 만들기 ← 나는 이 방식을 택했다.
public enum CenterErrorResult implements ErrorResult { NOT_EXIST_CENTER(HttpStatus.NOT_FOUND, "존재하지 않는 센터입니다."), ; private final HttpStatus httpStatus; private final String message; @Override public ErrorResponse getErrorResponse() { return new ErrorResponse(httpStatus, message); } }
Java
복사