Skip to content

Conversation

@mintcoke123
Copy link

@mintcoke123 mintcoke123 commented Nov 13, 2025

안녕하세요! 정다빈 리뷰어님!
저번 자바 미션에 이어 오랫만에 스프링 미션으로 뵙게 되었네요!
이번 미션도 잘부탁드립니다!

미션 진행 전에 사과드립니다.

crlf를 이용해 자동 eol에러 방지를 적용하려고 했습니다만,
lf에서 crlf로 변경하는 과정에서 잘못된 레퍼런스를 참고해
git add . --renormalize 명령어를 실행하는 바람에
모든 파일에 불필요한 diff가 발생하고 말았습니다.

찾아본 결과, 이를 원상태로 되돌리기 위해서는
초기 코드를 다시 받아 구현 과정을 모두 재작성해야 하는데,
그렇게 되면 제가 기능을 구현 의도와 흐름이 사라져
오히려 더 혼란을 줄 것이라 판단했습니다.

혹시 리뷰를 진행하실 때 Hide whitespace changes 옵션을 켜주신다면
제가 실제로 구현한 부분만 확인하실 수 있을 것 같습니다.
번거롭게 해드려 정말 죄송합니다.
가능한 한 리뷰에 불편함이 없도록 이후에는 이런 문제가 발생하지 않도록 하겠습니다.


🚀 미션 내용

이번 3,4단계 미션은 어플리케이션에 예약 추가/취소 기능 추가와, 예외처리를 구현하는 내용이었습니다.
저는 다음과 같은 순서로 코드를 구현하였습니다.

📊 구현 순서

▶3단계

  1. 기능 추가에 따른 클래스의 패키지 분리
  2. 객체 불변성을 위한 RestaurantList 도메인객체 생성
  3. ResponseDto 생성

▶4단계

  1. RestaurantList 에서 exception을 throw 하도록 변경
  2. throw를 받아 예외처리코드 작성

🤔 시행착오

RestController사용

처음에는 ReservationController를 단순 @Controller로 작성했으나,
JSON 형태로 데이터를 주고받는 로직을 구현하면서 @RestController가 더 적합하다고 판단해 수정했습니다.
@Controller는 주로 View 반환에 사용되는 것으로 알고있지만, 이번 미션 뿐만이 아니라 대부분의 백엔드 로직에서는 API 응답이 중심이기 때문에
@ResponseBody를 반복해서 붙이는 대신 @RestController로 변경하여 응답 의도를 더 명확히 표현했습니다.

ResponseEntity 사용

에러 응답을 기존 방식 그대로 반환하면 상태 코드를 원하는 대로 반환하지 못했습니다.
그래서 예외 상황에서는 ResponseEntity를 사용해 HTTP 상태 코드와 응답 바디를 명확하게 설정할 수 있도록 변경했습니다.
이를 통해 에러 코드의 응답값을 테스트코드의 요구사항에 맞게 분리하고, 컨트롤러에서 검증한 에러의 의도를 더 잘 나타낼 수 있었습니다.

Copy link

@70825 70825 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 동현님~ 로또 미션 이후로 벌써 한달만이군요 저도 잘 부탁드려요~!

미션 진행 전에 사과드립니다.

저는 Hide whitespace changes로 코드 편하게 볼 수 있으면 딱히 상관 없습니다 😄

@controller는 주로 View 반환에 사용되는 것으로 알고있지만, 이번 미션 뿐만이 아니라 대부분의 백엔드 로직에서는 API 응답이 중심이기 때문에
@responsebody를 반복해서 붙이는 대신 @RestController로 변경하여 응답 의도를 더 명확히 표현했습니다.

👍
생각해보니 저는 팀에서 어드민 페이지도 FE 기술스택을 사용해서 @Controller를 안쓴지 엄청 오래됐네요
요즘 어드민 페이지도 작업하고 있는데, 동현님 코드리뷰하니까 대학생때 FE 조금이라도 해볼걸이라는 생각이 들어요 🫠


요번 미션도 1, 2단계랑 비슷하기 때문에 코드 보면서 궁금한 부분 + 찾아보시면 좋은 내용으로 코멘트 남겼습니다~

}

@GetMapping("/reservations")
public List<ReservationResponse> findAll() {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

List<ReservationResponse> 👍

}

@DeleteMapping("/reservations/{id}")
public ResponseEntity<Void> delete(@PathVariable int id) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

코드 수정은 안해도 되지만 int형으로 할 경우의 문제점은 무엇이 있다고 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

크게 생각을 안한 부분인 것 같습니다,
Integer같은 래퍼 클래스를 사용하는 이유는 컬랙션이나 제네릭을 사용할 수 있기 때문으로 알고 있습니다.

추가로 알아본 결과 클라이언트가 잘못된 URL을 요청한다면, 기본값은 커스텀 메세지를 만들 수 없기 때문에 controller 메서드에 들어오기 전 400 Bad Request를 리턴해 버린다는 것을 알았습니다,
또한, REST API는 기본값은 null값을 리턴할 수 없지만.
래퍼 클래스는 null 값을 리턴할 수 있기 때문에 여러 상황들을 리턴할 수 있군요!

  1. 경로 변수 누락
  2. optional 한 파라미터의 예외 처리

래퍼 클래스의 존재 의의를 이제야 제대로 이해한 것 같습니다.
감사합니다!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

헙 사실 코멘트 달은 목적은 Int형이 데이터 저장을 다 담기에는 작을 수도 있는 것이었어요
서비스는 한 번 만들어진 이상 계속해서 운영되기 떄문에 int형은 턱없이 작더라구요
ex. 제 팀에 가장 많이 적재되는 데이터는 한달반이면 id값이 int형 범위를 넘어감

여기 상황에서는 null 값이 들어오면 안되긴하는데, 조회 API쪽에서는 null값이 필요한 상황이 생각보다 좀 있어서 그 상황에 사용하면 도움이 될거라 생각해요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 그렇군요ㅋㅋㅋ
프론트엔드에서는 long자료형을 사용할 이유가 많이 없어 간과하고 있었습니다.
long의 래퍼클래스인 Long 자료형을 사용하는 것이 좋아보입니다,
수정했습니다!

Comment on lines 14 to 15
@Component
public class ReservationList {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. @Component를 붙인 이유가 있나요?
  2. ReservationList가 domain 안에 있는 이유가 궁금해요

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

패키지를 살펴보니 ReservationList만 존재하군요?!
Reservation이 없는 이유가 있을까요?

Copy link
Author

@mintcoke123 mintcoke123 Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. @Component를 붙인 이유는 해당 클래스를 빈으로 사용해야 동시성 문제가 생기지 않는다는 레퍼런스를 참고했기 때문입니다.
    찾아보니 서버에서는 같은 사용자가 데이터를 입력했을 때 동시성 문제가 생긴다는 것을 알았고, 그 때문에 사용하였습니다.

  2. ReservationList는 ‘예약 데이터들의 집합’을 관리하는 하나의 객체라고 판단했습니다. 그래서 “도메인 객체는 스스로의 행동을 책임진다”는 기준에 따라 ReservationList 전체를 domain 레이어에 두었습니다.
    하지만 Reservation을 도메인 엔티티로 두게 된다면, ReservationList가 담당하는 역할은 더 이상 도메인 자체의 책임이라기보다는 엔티티들을 관리하는 서비스적 역할에 가깝습니다.
    따라서 이 경우에는 ReservationList를 domain이 아닌 service 레이어로 이동시키는 것이 더 적절하다고 판단했습니다.

  3. 초기에 Reservation은 DTO 형태로만 존재했습니다. 클라이언트에 반환되는 ReservationResponse DTO를 기준으로 구현을 진행하다 보니, 예약 데이터를 실제로 보관하는 구조가 ReservationList 하나뿐이었고, 자연스럽게 이 클래스만 존재하게 되었습니다.
    코드를 작성할 때에는 ReservationList 자체를 데이터의 모임인 하나의 객체로 보았습니다만, 다시 생각해보니 Restaurant객체를 따로 관리하는 것이 좋아보입니다!

Copy link

@70825 70825 Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 빈으로 등록해야 동시성 문제가 생기지 않는다는게 구체적으로 어떤 상황이 일어날 때 발생하는 문제인건가요? 이거는 어떤 상황에 동시성 문제가 발생하냐에 따라 코드가 달라질 것 같아서요

2-1) Reservation을 엔티티로 두어도 예약 데이터들의 집합’을 관리하는 하나의 객체로써는 그대로 동작할 수 있지 않을까요? 일급 컬렉션이라고 생각해도 될 것 같아가지구요
2-2) service 레이어는 어떤 역할을 한다고 생각하시나요?

  1. 아하 이해했습니다 여기는 데이터베이스가 없으니 지금 상황에서는 ReservationList만 두어도 상관없겠네요. 요거는 편하신대로 수정하시면 됩니다

@@ -0,0 +1,7 @@
package roomescape.exception;

public class InvalidReservationRequestException extends RuntimeException {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • checked exception
  • unchecked excpetion
  • runtime exception
  • illegal argument exception

에 대해서 찾아보시면 좋아보입니다

자바 미션에서 이미 공부하신 내용이라면 checked / unchecked exception은 생략하고, runtime exception vs illegal argument exception중에 어떤게 더 좋을지만 답변주셔도 돼요~

Copy link
Author

@mintcoke123 mintcoke123 Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

자바에서 예외는 Throwable 클래스를 기준으로 Error, Exception 클래스로 나뉘어집니다.
그리고 그 Exception 클래스에서

  • Checked Exception
  • Unchecked Exception(RuntimeException 하위)
    이 나누어지게 됩니다.

1. checked exception

checked exception은, runtime exception의 하위 클래스가 아니면서 Exception의 하위 클래스인 것들으로써,
특징으로는 반드시 예외 처리를 해야 하고, 컴파일러가 try/catch 또는 throws로 처리했는지 “체크”하는 특징을 가지고 있습니다,

2. unchecked exception

runtime exception의 하위 클래스 전부를 unchecked exception이라고 이해했습니다!
특징으로는 예외처리를 강제하지 않고, 런타임 도중에 발생하는 예외들이라는 특징을 가지고 있습니다.

3. runtime exception

runtime exception은 unchecked exception의 대표적인 부모 클래스라고 이해했습니다.
컴파일 시점에는 강제적인 예외 처리를 요구하지 않고, 실제 런타임 과정에서 잘못된 로직이나 실수로 발생하는 예외들을 포괄합니다.

4. illegal argument exception

illegal argument exception은 runtime exception의 하위 클래스 중 하나로,
“전달된 인자 값이 잘못되었다”는 상황을 명확하게 표현할 수 있는 예외라고 이해했습니다.
예를 들어 메서드 호출 시 null이 오면 안 되는 자리에서 null이 들어오거나, 유효 범위를 벗어난 값이 들어온 경우가 있습니다,
동작적인 측면에서는 runtime exception과 동일하지만, “전달된 인자가 잘못되었다”는 것을 이름으로 명시하는 역할을 합니다.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    public ReservationResponse create(ReservationRequest request) {
        if (request.name() == null || request.name().isBlank()
                || request.date() == null || request.date().isBlank()
                || request.time() == null || request.time().isBlank()) {
            throw new InvalidReservationRequestException("이름,날짜,시간이 모두 입력되어야 합니다.");
        }

현제 해당 예외사항을 사용하는 경우는 “전달된 인자 값이 잘못되었다”를 나타내고 있습니다.
즉, 해당 에러는 RuntimeException 이 아닌 IllegalArgumentException 를 상속받도록 하는 것이 적절하다고 생각합니다!
수정하겠습니다!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍👍

Comment on lines +7 to +8
@RestControllerAdvice
public class ReservationExceptionHandler {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

            InvalidReservationRequestException.class,
            NotFoundReservationException.class

위 두 에러 말고도 동현님이 생각하지 못한 에러가 발생한다면 문제가 생길 것 같아요
요거는 찾아보신다음 적용하면 좋아보입니다

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

몇가지 기본적으로 사용된다고 하는 핸들러들을 추가했습니다!

IllegalArgumentException
잘못된 인자값이 메서드로 전달될 때 발생하는 일반 런타임 예외.

HttpMessageNotReadableException
HTTP 요청 바디(JSON 등)를 역직렬화할 수 없을 때 발생.

MethodArgumentTypeMismatchException
컨트롤러 파라미터 타입과 전달값이 매칭되지 않을 때

MissingServletRequestParameterException
필수 요청 파라미터가 누락된 경우

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

    @ExceptionHandler(Exception.class)
    public ResponseEntity<Void> handleUnexpected(Exception e) {
        return ResponseEntity.status(500).build();
    }

👍 리뷰의 의도는 위에 Exception.class를 통해 생각하지 못한 에러들을 방어하기 위함인데요. 요거는 처음에 ControllerAdvice 만들 떄 필수적으로 추가하시면 된다고 생각하시면 되어요

코멘트 주신 HttpMessageNotReadableException, MethodArgumentTypeMismatchException, MissingServletRequestParameterException 같은 경우에는 필요하다면 이미 스프링에서 ResponseEntityExceptionHandler라는 곳에서 기본 제공하고 있으니 나중에 필요할 때 요거 사용하시면 됩니다

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

“생각 못한 에러” 방어는 Exception.class로 이미 충족 가능하군요!
다음부터는 범용성이 넓은 ResponseEntityExceptionHandler 사용하도록 하겠습니다!

Comment on lines 12 to 15
NotFoundReservationException.class
})
public ResponseEntity<Void> handleBadRequest(RuntimeException e) {
return ResponseEntity.badRequest().build();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. 에러 클래스명은 404 NotFound인데 실제 응답 상태코드는 400이군요?! 개선이 필요해보이네요
  2. 에러 발생할 때 로그 적용해보시면 좋아보여요~

Copy link
Author

@mintcoke123 mintcoke123 Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

말씀 주신 부분 확인했습니다!
로그 적용해보니 기존에는 NotFoundReservationException 을 다른 요청 검증 예외들과 함께 handleBadRequest 에 묶어두는 바람에, 클래스명은 NotFound인데 실제 응답 코드는 400으로 내려가고 있었습니다.
기존 테스트코드에서도 400을 검사하고 있었는데, NotFoundReservationException 의 경우에는 404를 리턴하는 것이 옳다고 판단하여 테스트코드를 수정하였습니다.
피드백 반영해서 NotFoundReservationException 을 별도 예외 핸들러로 분리하고, 404 상태 코드를 반환하도록 수정했습니다.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

앗 로그는 @Slf4j같은걸 이야기한건데요. 이거는 지금 간단하게 의존성 추가해서 적용해보시거나, 프로젝트때 해보시면 Logback까지 찾아보시는걸 추천드려요

아래 코드는 java.util.logging을 사용한 방법인데요. java.util.logging은 잘 사용하지 않는 거긴한데 어떻게 로그가 나오는지만 살펴봐도 도움이 될 것 같아요


@RestControllerAdvice
public class ReservationExceptionHandler {

    private static final Logger logger = Logger.getLogger("테스트용");

...

    @ExceptionHandler(NotFoundReservationException.class)
    public ResponseEntity<Void> handleNotFound(NotFoundReservationException e) {
        String stackTrace = Arrays.stream(e.getStackTrace())
                .map(StackTraceElement::toString)
                .collect(Collectors.joining("\n"));
        logger.log(Level.INFO, stackTrace);
        return ResponseEntity.status(404).build();
    }

...

이후 포스트맨 같은거로 없는 id를 DELETE 요청하면 로그가 나와요

스크린샷 2025-11-16 오후 5 59 20

Copy link
Author

@mintcoke123 mintcoke123 Nov 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아 그렇군요!
logger 사용법 익힐 겸 작성해주신대로 적용해보았습니다!
이후 미션부터 적용해 볼 예정입니다!

Comment on lines 36 to 41
public void delete(int id) {
if (!idToReservation.containsKey(id)) {
throw new NotFoundReservationException("해당 분실물을 찾을 수 없습니다: " + id);
}
idToReservation.remove(id);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

삭제에 대한 생각이 두가지 관점으로 나눌 수 있을 것 같아요

  1. 데이터가 존재하지 않는게 목적이니 삭제 요청이 계속 들어오면 이미 없는 데이터여도 성공적으로 삭제 했다고 응답한다
  2. 삭제 요청이 계속 들어오면 두번째 요청부터는 삭제할 리소스가 없다고 판단해 에러를 반환한다

두가지 관점중에 어떤게 더 나은 방식인 것 같나요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이번 구현에서는 “잘못된 id로 삭제 요청이 들어왔다면 버그일 가능성이 높다”는 쪽에 무게를 두고, 두 번째 방식(존재하지 않으면 NotFound 처리)을 선택했습니다!

1번 관점이 존재할 이유 자체를 이해하지 못했었는데, 찾아보니 멱등성이라는 개념이 있군요!
실사용에서는 사용자가 여러 번 클릭하거나 네트워크 문제로 삭제 요청이 여러 번 전송될 때 오류를 뱉으면 흐름이 부자연스러울 수도 있을 것 같습니다.
해당 상황를 대비하여 1번 관점으로 구현할 수도 있을 것 같습니다.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

멱등성 빌드업으로 이야기 꺼냈는데 먼저 찾아주셨군요 😁

1번, 2번 둘 다 맞는 코드이긴한데, 만약 2번으로 하게 된다면 삭제 됐다, 저장된 리소스가 없다고 명확하게 구분지을 수 있다는 장점이 있지만, 단점으로는 FE분들이 404 에러 케이스도 핸들링 해야한다는 귀찮음이 있긴해요 ㅋㅋ

if (request.name() == null || request.name().isBlank()
|| request.date() == null || request.date().isBlank()
|| request.time() == null || request.time().isBlank()) {
throw new InvalidReservationRequestException("이름,날짜,시간이 모두 입력되어야 합니다.");
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

에러 메시지를 남겨두는 이유는 나중에 에러가 발생 했을 때 무엇이 문제였는지 찾아보기 위함인데요. 그런 의미로 아래 NotFoundReservationException처럼 에러 추적에 필요한 파라미터 정보들을 모두 담아두면 좋아보입니다~

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

나중에 로그로 봤을 때 “어떤 값이 들어와서 이 예외가 났는지”를 남기기 위해, 해당 방식으로 코드를 수정하겠습니다!

    if (request.name() == null || request.name().isBlank()
            || request.date() == null || request.date().isBlank()
            || request.time() == null || request.time().isBlank()) {
        throw new InvalidReservationRequestException(
                "잘못된 예약 요청입니다. " +
                "name=" + request.name() +
                ", date=" + request.date() +
                ", time=" + request.time()
        );
    }

public class ReservationList {

private final Map<Integer, ReservationResponse> idToReservation = new HashMap<>();
private int index = 0;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nextstep 3단계에 AtomicLong에 대한 내용이 있는데 찾아보시면 좋아보여요~

Copy link
Author

@mintcoke123 mintcoke123 Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

AtomicLong을 사용하면 두 스레드가 동시에 값에 접근해도 값 증가가 원자적이기 때문에 서로 덮어쓰지 않고 자연스레 증가하는군요!
int를 사용하면 동시성 문제가 생기는 것을 알게 되었습니다.
index를 사용하는 모든 파트를 AtomicLong으로 대체했습니다!

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

넵 요거는 더 궁금하시다면 CAS 알고리즘 찾아보시면 좋아보여요

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

메모리 위치 M의 값이 예상 값 A와 같으면 새 값 B로 업데이트하고, 그렇지 않으면 아무 작업도 수행하지 않는 알고리즘이군요!

public class AtomicInteger extends Number implements java.io.Serializable {

        private static final Unsafe U = Unsafe.getUnsafe();
        private static final long VALUE = U.objectFieldOffset(AtomicInteger.class, "value");
    private volatile int value;

        public final int incrementAndGet() {
                return U.getAndAddInt(this, VALUE, 1) + 1;
        }
}

public final class Unsafe {
        @HotSpotIntrinsicCandidate
        public final int getAndAddInt(Object o, long offset, int delta) {
                int v;
                do {
                        v = getIntVolatile(o, offset);
                } while (!weakCompareAndSetInt(o, offset, v, v + delta));
                        return v;
        }
}

위 예제코드에서 CAS 알고리즘이 왜 필요한지 이해했습니다!
현재 제 코드에는 incrementAndGet() 메소드 내부에서 CAS 알고리즘의 로직을 구현하고 있어서 비동기적 처리가 가능하다는 것을 알았습니다.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants