[Review] ATDD 2주차 3차 PR

 

권승철 리뷰어 님께 감사의 말을 전하고 싶다!!

 

1. 질문

1-1) CustomException의 사용

이번에 CustomException을 좀더 세분화 하기위해, 기존의 SectionException을 SectionsAdd, SectionsDelete 별로 나눠 CustomException을 구현하게 되었습니다.

 

그보다 더 세부적인 내용들은 예외 안에서 메시지로 구별하였습니다!

우선 모든 예외가 공통적으로 상속받을 BusinessException을 구현하였습니다.

해당 class 안에는 HttpStatus를 저장 할 수 있습니다.

public class BusinessException extends RuntimeException {

    private final HttpStatus httpStatus;

    public BusinessException(String message, HttpStatus httpStatus) {
        super(message);
        this.httpStatus = httpStatus;
    }

    public HttpStatus getStatus() {
        return httpStatus;
    }
}

 

이후 이를 상속하는 세부적인 예외들은 다음과 같이 사용하였습니다.

public class SectionsAddException extends BusinessException {

    private static final String ALREADY_BOTH_STATION_REGISTER_EXCEPTION = "상행역과 하행역이 이미 노선에 모두 등록되어 있습니다";
    private static final String NOT_FOUND_BOTH_STATION_EXCEPTION = "상행역과 하행역 모두 찾을 수 없습니다";
    private static final String SECTION_DISTANCE_EXCEPTION = "기존의 구간 길이보다 긴 신규구간을 중간에 추가할 수 없습니다";

    private SectionsAddException(String message, HttpStatus httpStatus) {
        super(message, httpStatus);
    }

    public static SectionsAddException ALREADY_BOTH_STATION_REGISTER_EXCEPTION() {
        return new SectionsAddException(ALREADY_BOTH_STATION_REGISTER_EXCEPTION, HttpStatus.BAD_REQUEST);
    }

    public static SectionsAddException NOT_FOUND_BOTH_STATION_EXCEPTION() {
        return new SectionsAddException(NOT_FOUND_BOTH_STATION_EXCEPTION, HttpStatus.NOT_FOUND);
    }

    public static SectionsAddException SECTION_DISTANCE_EXCEPTION() {
        return new SectionsAddException(SECTION_DISTANCE_EXCEPTION, HttpStatus.BAD_REQUEST);
    }
}

 

이후 사용할 때는 다음과 같이 사용하였습니다.

    public void deleteSection(Station station) {
        if (isSizeLessThanOne()) {
            throw SectionsDeleteException.CANT_DELETE_LAST_ONE_SECTION_EXCEPTION();
        }

        // 생략...
    }

기존에 말씀해주신 것 처럼 ErrorCode한곳에서 메시지를 다 관리하기는 힘들것 같아, 해당 Exception 내부에서 관리하도록 변경하였습니다.

또한 factory method를 만들어 예외를 좀더 쉽게 사용하려 하였습니다!!

이번 변경된 저의 예외 사용방식에 대한 리뷰어 님의 의견이 궁금합니다!

 

답변: 

 

2. 리뷰 정리

2-1) CQRS

커맨드와, 쿼리 모두 나타나는 메서드라고 할 수 있다.

이를 분리해야 함은 알고 있었다. 이름도 유명한 CQRS 문제인 것 이다.

 

따라서 코드를 다음과 같이 변경하였다.

Section firstSection = getFirstSection();
if (firstSection.hasSameUpStation(station)) {
    sections.remove(firstSection);
    return;
}

Section lastSection = getLastSection();
if (lastSection.hasSameDownStation(station)) {
    sections.remove(lastSection);
    return;
}

코드가 상당하게 길어졌는데, 의미는 명확하게 전달된다 생각한다.

또한 command 부분과 query 부분또한 나뉘어 졌다 생각된다.

 

다만 위 부분을 메서드로 추출하여 좀더 리팩터링 할 수 있을것 같은데... 생각보다 쉽지가 않다...

 

2-2) 기능을 바꿀때는 메서드 이름도 다시 고려해보기

 

기존의 메서드는 노선의 맨 뒤 구간만 삭제가 가능해서 메서드 이름이 위과 같았다.

 

하지만 변경후 해당 메서드는 노선의 앞, 중간, 뒤 모두에서 구간이 삭제 가능하게 기능이 변경되었다.

따라서 이에 맞도록 이름 또한 "deleteSection"으로 변경해 주었다.

 

2-3) Early Return

원래 코드는 다음과 같았다.

private void deleteMiddleStation(Station station) {
    Section sameUpStationSection = findSameUpStationSection(station);
    Section sameDownStationSection = findSameDownStationSection(station);
    if (sameUpStationSection != null && sameDownStationSection != null) {
        Section newSection = Section.combineOf(sameDownStationSection, sameUpStationSection);
        sections.remove(sameUpStationSection);
        sections.remove(sameDownStationSection);
        sections.add(newSection);
    }
}

이를 다음과 같이 변경하였다.

private void deleteMiddleStation(Station station) {
    Section sameUpStationSection = findSameUpStationSection(station);
    Section sameDownStationSection = findSameDownStationSection(station);
    if (sameUpStationSection == null || sameDownStationSection == null) {
        return;
    }

    Section newSection = Section.combineOf(sameDownStationSection, sameUpStationSection);
    sections.remove(sameUpStationSection);
    sections.remove(sameDownStationSection);
    sections.add(newSection);
}

 

이번 리뷰에서 핵심 정리할 부분은 좀 짧았던것 같다? ㅎㅎ