Skip to content

Conversation

@kimsky247-coder
Copy link

@kimsky247-coder kimsky247-coder commented Nov 20, 2025

안녕하세여 경환님! 이번 Spring JDBC 미션에 매칭된 김하늘입니다. 잘 부탁드립니다🙇

프로젝트 구조

현재 프로젝트 구조입니다.

controller/
- HomeController.java: 메인 페이지 GET 요청을 처리합니다.
- ReservationController.java: 예약 관련 API 요청을 처리합니다.
- ReservationViewController.java: 예약 관리 페이지 요청을 처리합니다.

domain/
- Reservation.java: 예약의 핵심 데이터를 정의하는 도메인 객체.

dto/
- ReservationRequest.java: 예약 생성 요청 데이터를 담는 DTO.
- ReservationResponse.java: API 응답 데이터를 담는 DTO.
- ErrorResponse.java: 예외 발생 시 에러 메시지를 담아 반환하는 DTO.

exception/
- BadRequestReservationException.java: 400 Bad Request에 해당하는 커스텀 예외.
- NotFoundReservationException.java: 404/400에 해당하는 커스텀 예외.
- GlobalExceptionHandler.java: 모든 컨트롤러의 예외를 전역적으로 처리하는 핸들러.
- ErrorMessage.java: 에러 메시지와 상태 코드를 중앙 관리하는 Enum.

repository/
- ReservationRepository.java: JdbcTemplate을 사용하여 H2 데이터베이스와 통신하며 예약 데이터를 저장/조회/삭제하는 저장소 계층.

service/
- ReservationService.java: 비즈니스 로직을 담당하며, Repository를 호출하여 데이터를 처리하는 서비스 계층.

궁금한 점

이번 미션에서 H2 데이터베이스의 AUTO_INCREMENT를 사용하여 ID 생성을 DB에 위임했습니다. AUTO_INCREMENT 에 대해 학습하는 과정에서 INSERT가 실패해도 이미 증가한 AUTO_INCREMENT 카운트는 줄어들지 않아 ID 값에 구멍이 생긴다는 것을 알게 되었습니다.예약 번호가 반드시 연속적이어야 하는지 아니면 동시성 성능을 위해 ID의 불연속성을 허용해도 되는 것인지 궁금합니다!

Copy link

@dooboocookie dooboocookie left a comment

Choose a reason for hiding this comment

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

안녕하세요, 하늘.
리뷰어 루카입니다.

늘 Spring MVC 미션은 리뷰를 드리기가 쉽지 않은데요.
일단 서로 맥락의 차이를 좀 줄이고자 질문위주로 리뷰를 드렸습니다.

stream이나 메서드 참조를 사용하신 것이
JAVA 문법에 많이 익숙해보이시네요 굿입니다.

🤓 질문에 대한 답변

이번 미션에서 H2 데이터베이스의 AUTO_INCREMENT를 사용하여 ID 생성을 DB에 위임했습니다. AUTO_INCREMENT 에 대해 학습하는 과정에서 INSERT가 실패해도 이미 증가한 AUTO_INCREMENT 카운트는 줄어들지 않아 ID 값에 구멍이 생긴다는 것을 알게 되었습니다.예약 번호가 반드시 연속적이어야 하는지 아니면 동시성 성능을 위해 ID의 불연속성을 허용해도 되는 것인지 궁금합니다!

우려가 되는 상황이 있어서 주신 질문이실까요??
ID가 불연속적인 숫자일 때 어떤 '동시성 성능' 이슈가 있다고 판단하신 걸까요?

질문에 대한 답을 저 나름대로 해석해서 드리자면,
상관없을 것 같아요.
ID를 부여할 때 가장 중요한 것은 unique하다는 것입니다. 사실 이것만 지켜지면, 되죠.
중복되는 순간 그것은 아이디라고 할 수 없을테니까요.
ID를 autoincrement로 하는 것은 여러가지고 아이디 부여 전략중 하나입니다.
ID가 큰 row가 나중에 생긴 row라고 생각하면 안되겠죠.
그리고 추가적으로 ID는 어떤 도메인적인 의미가 없는 난수나 자동증가하는 숫자로 하는 것을 추천드립니다.


하늘 사실 많은 대화를 나누기 힘든 미션이지만 질문에 한번 답을 주시면서 한번더 핑퐁해보면 좋을것같습니다.

Comment on lines +5 to +6
date DATE NOT NULL,
time TIME NOT NULL,

Choose a reason for hiding this comment

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

👍

Comment on lines +34 to +49
public Reservation save(Reservation reservation) {
String sql = "INSERT INTO reservation (name, date, time) VALUES (?, ?, ?)";
KeyHolder keyHolder = new GeneratedKeyHolder();

jdbcTemplate.update(connection->{
PreparedStatement ps = connection.prepareStatement(sql, new String[]{"id"});
ps.setString(1, reservation.getName());
ps.setObject(2, reservation.getDate());
ps.setObject(3, reservation.getTime());
return ps;
}, keyHolder);

long id = Objects.requireNonNull(keyHolder.getKey()).longValue();

return new Reservation(id, reservation.getName(), reservation.getDate(), reservation.getTime());
}

Choose a reason for hiding this comment

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

👍 Approve

전반적으로 JDBC Template을 잘 사용하셨네요.
요구사항에도 맞게 생성된 ID를 수집하기 위해서 Keyholder 굿굿👍

Comment on lines 26 to 50
public List<ReservationResponse> getAllReservations() {
return reservationList.stream()
return reservationRepository.findAll().stream()
.map(ReservationResponse::from)
.collect(Collectors.toList());
}

public ReservationResponse createReservation(ReservationRequest request) {
try {
LocalDate date = LocalDate.parse(request.date());
LocalTime time = LocalTime.parse(request.time());

Long newId = index.incrementAndGet();

Reservation newReservation = new Reservation(
newId,
Reservation reservation = new Reservation(
null,
request.name(),
date,
time
);

reservationList.add(newReservation);
return ReservationResponse.from(newReservation);
Reservation savedReservation = reservationRepository.save(reservation);
return ReservationResponse.from(savedReservation);

} catch (DateTimeParseException e) {
throw new BadRequestReservationException(ErrorMessage.INVALID_DATE_TIME_FORMAT.getMessage());
}
catch (DateTimeParseException e) {
throw new BadRequestReservationException(ErrorMessage.INVALID_DATE_TIME_FORMAT.getMessage()); }
}

Choose a reason for hiding this comment

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

👀 Comment

여기서 2가지 질문이 있습니다.

  1. DateTimeParseException 을 custom exception으로 변환한 이유
  2. 예외 처리 범위를 메서드 전체로 한 이유

Copy link
Author

Choose a reason for hiding this comment

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

  1. DateTimeParseException 을 custom exception으로 변환한 이유

DateTimeParseException이 발생하면 기본적으로 500 에러가 반환됩니다. 하지만 날짜 형식이 잘못된 것은 서버 오류가 아닌 클라이언트의 잘못된 입력이라고 생각해 400 Bad Request로 명확히 응답하기 위해 커스텀 예외로 변환했습니다.

  1. 예외 처리 범위를 메서드 전체로 한 이유

try-catch 범위를 전체로 잡은 것은 실수입니다. 파싱 로직에서만 해당 예외가 발생하므로 범위가 너무 넓으면 다른 로직에서 발생하는 예상치 못한 예외까지 400 에러로 판단되어 오류를 놓칠 수 있을 것 같습니다. 파싱이 일어나는 부분만 감싸도록 범위를 좁히는 것이 옳다고 생각해 범위를 수정하였습니다.

Copy link

@dooboocookie dooboocookie Nov 25, 2025

Choose a reason for hiding this comment

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

DateTimeParseException이 발생하면 기본적으로 500 에러가 반환됩니다. 하지만 날짜 형식이 잘못된 것은 서버 오류가 아닌 클라이언트의 잘못된 입력이라고 생각해 400 Bad Request로 명확히 응답하기 위해 커스텀 예외로 변환했습니다.

어차피 CustomException 예외 핸들링해서 응답을 지정해준것 아닌가요?
DateTimeParseException도 그냥 응답을 주면 될 것 같은데요.
CustomException도 트레이드 오프가 있습니다.
관리포인트를 늘리죠. Java 혹은 사용하는 프레임웤이나 라이브러리들은 대부분 예외들을 잘 정리해놨는데요.
그를 활용하는 것도 좋은 방법일 것 같습니다.

(저는 커스텀예외 사용할 것 같아요)

Copy link
Author

Choose a reason for hiding this comment

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

GlobalExceptionHandler에서 표준 예외를 직접 잡으면 불필요한 클래스 추가를 막고 관리 포인트를 줄일 수 있을 것 같습니다. 하지만 커스텀 예외를 이용하면 서비스 계층의 의도를 명확히 드러낼 수 있고, 내부 로직이 바뀌더라도 컨트롤러 계층에는 영향이 없도록 결합도를 낮출 수 있는 것이 장점이 있어 커스텀 예외를 선택했습니다!

Comment on lines 52 to 57
public void deleteReservation(Long id) {
boolean removed = reservationList.removeIf(reservation -> reservation.getId().equals(id));
int deletedCount = reservationRepository.deleteById(id);

if (!removed) {
if (deletedCount == 0) {
throw new NotFoundReservationException(ErrorMessage.NOT_FOUND_RESERVATION.getMessage());
}

Choose a reason for hiding this comment

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

👀 Comment

삭제된 Reservation이 없을 때, 예외를 throw 하는 이유가 있을까요?
삭제된 내용이 애초에 없었다면 요청자는 원하는 결과 아니었을까요?
그리고 추가적으로 이 PR에는 포함되어있지 않지만, 해당 에러가 404가 아닌 400인 이유도 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

이전의 4단계 미션 요구사항에 '삭제 할 예약이 없는 경우 400으로 응답하세요'라는 조건이 있어서 이를 따르기 위해 400을 반환하도록 구현했습니다. 하지만 DELET메서드는 멱등성을 만족해야 합니다. 따라서 이미 삭제된 리소스에 대해 다시 삭제 요청을 보내더라도, 서버의 상태는 '삭제됨'으로 동일하므로 성공으로 처리하는 것이 더 자연스러운 설계라고 생각합니다. 삭제할 데이터가 없더라도 예외 대신 204 No Content를 반환하도록 수정했습니다.

Comment on lines 18 to 19
@Service
public class ReservationService {

Choose a reason for hiding this comment

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

👍 Approve

DB에 접근하는 Service 로직을 처리할 때에는,
그 일련의 과정을 하나의 트랜잭션에 두기 위해 @Transactional을 많이 사용하는데요.
혹시 하늘은 Transcational어노테이션을 사용하지 않은 이유가 따로 있으실까요?

Copy link
Author

Choose a reason for hiding this comment

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

현재 로직에서는 create나 delete가 각각 하나의 SQL만 실행하기 때문에 별도의 설정 없이도 동작한다고 생각했습니다. 하지만 @transactional을 통해 코드의 의도를 명확히 전달하는 것이 더 좋은 설계인 것 같습니다. 수정했습니다

Choose a reason for hiding this comment

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

저도 update를 위한 SQL문을 1개만 실행할 떄는 딱히 Transactional이 없어도 된다고 생각합니다.
확신이 있으면 그렇게 가셔도됩니다.
해당 코드들에서 Transactional 감싸지 않았을 때 문제점은 저는 발견하지 못했어요.
혹시나 해서 Keyholder의 동작도 살짝 체크해봤는데 괜찮을 것 같습니다.

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