리팩토링 일기: SRP 최대한 준수해보자

1. 엔티티에 도메인 로직 위임하기

변경전: rejectOrder, getOrder, getOrdersByShop

68~128줄 차지

검증, 저장, 레포지토리 호출, 값 변환... 오만 거 다 했다.

 

public void validateIsPending() {
    if (status != Status.PENDING) {
        throw new InvalidRequestException(ErrorCode.CANNOT_CHANGE_STATUS);
    }
}

public void validateIsNotCompleted() {
    if (status == Status.COMPLETED) {
        throw new InvalidRequestException(ErrorCode.ALREADY_COMPLETED);
    }
}

public void validateIsCompleted() {
    if (status != Status.COMPLETED) {
        throw new InvalidRequestException(ErrorCode.ALREADY_COMPLETED);
    }
}

public Shop getShop() {
    return orderMenus.stream()
        .findFirst()
        .map(orderMenu -> orderMenu.getMenu().getShop())
        .orElseThrow(() -> new InvalidRequestException(ErrorCode.SHOP_NOT_FOUND));
}

public void validateOwnership(AuthUser authUser) {
    if (!Objects.equals(user.getId(), authUser.id())) {
        throw new ForbiddenException(ErrorCode.FORBIDDEN_OPERATION);
    }
}

해결: 엔티티 계층에 검증 로직과 orderMenu로 shop을 추출하는 등 프로젝트 전반의 서비스 계층에서 사용할 수 있는 메서드를 담았다.

 

변경후: 64~101줄(대충 주석 4줄도 더해야되긴 한다)

엔티티가 자신의 상태를 스스로 검증하거나 처리하게 해서, 상태와 관련된 로직이 분리되지 않고 하나로 모여있게 했다. 따라서 코드의 응집도가 높아졌다.

같은 엔티티 검증 로직을 다른 서비스 계층에서도 사용할 수 있어 재사용성도 증가했으며 프로젝트 전반의 서비스 계층 가독성이 증가했다.

 

하지만 엔티티 계층이 꽤 복잡해져서, 그 부분은 더 고민해야 할 과제이다.

 

 

 

2. 너무너무 과한 책임 분리하기

변경전: 모두 createOrder 하나만을 수행하는 로직이다..

장바구니가 비어있는지 확인하고, 메뉴가 올바른지 확인하고, 가게가 운영중인지 확인하고, 최소주문금액을 맞췄는지 검증하고, order 객체를 생성하고, 캐시를 삭제하고(이 때는 캐시 담당자님이랑 소통이 덜 됐을 때라 그나마 코드 짧은 것..), save한다.

그냥 진짜 엄청 길다.. 처음엔 어떻게 분리할지 감도 안 잡혔던 것 같다.

 

변경후:

OrderCartService: 카트(캐시)에서 정보를 가져온다.

OrderCartValidator: 카드(캐시)의 값들이 유효한지 검증한다.

OrderFactory: Order 객체를 생성하고 함께 매핑되어있는 OrderMenu 엔티티도 추가한다.

결과적으로 createOrder는 해당 클래스와 분리된 메서드들을 이용해 orderRepository에 값을 저장하고, dto로 변환하게 되는 책임만 갖게 되었다.