-
Notifications
You must be signed in to change notification settings - Fork 45
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[2-1단계 - 테마 관리 기능 추가] #103
base: heoseungjun
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요 승준님! 2단계도 잘 진행해주셨습니다.
리뷰 요청 방식이 next-step 에서 제공되는 기본 가이드와 상이한거 같아요!
step2, step3 문서를 참고해서 리뷰요청이 되면 더욱 좋을 것 같습니다!
구현은 군더더기 없이 잘 해주셔서 간단한 코멘트만 몇 개 남겼습니다!
@RestController | ||
@RequestMapping("/themes") | ||
public class ThemeController { | ||
|
||
private final ThemeService themeService; | ||
|
||
@Autowired | ||
public ThemeController(ThemeService themeService) { | ||
this.themeService = themeService; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
생성자 쪽에 @Autowired
를 생략할 수 있겠어요!
@Autowired | ||
public ReservationRepository(JdbcTemplate jdbcTemplate) { | ||
this.jdbcTemplate = jdbcTemplate; | ||
this.jdbcInsert = new SimpleJdbcInsert(jdbcTemplate) | ||
.withTableName("reservation") | ||
.usingGeneratedKeyColumns("id"); | ||
} | ||
|
||
public Long save(Reservation reservation) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
역시나 @Autowired
어노테이션을 생략할 수 있을거 같아요 😄
Map<String, Object> params = new HashMap<>(); | ||
params.put("name", reservation.getName()); | ||
params.put("date", reservation.getDate()); | ||
params.put("time_id", reservation.getTimeId()); | ||
params.put("theme_id", reservation.getThemeId()); | ||
|
||
return jdbcInsert.executeAndReturnKey(params).longValue(); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
simpleJdbcInsert 를 사용한 덕분에 코드가 엄청 간결해졌네요!
} | ||
|
||
public Long addReservation(Reservation reservation) { | ||
return reservationRepository.save(reservation); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
한 시간대에 중복 예약이 되지 않도록 검증해주는 로직이 추가되면 더 좋겠네요!
public List<Reservation> readAll() { | ||
String sql = "SELECT r.id, r.name as reservation_name, t.name as theme_name, r.date, rt.start_at as start_at FROM reservation as r INNER JOIN reservation_time as rt ON r.time_id = rt.id INNER JOIN theme as t ON r.theme_id = t.id"; | ||
return jdbcTemplate.query(sql, rowMapper); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reservation 을 우선 조회한 후, ReservationTimeRepository 를 통해 ReservationTime 을 조회하는 건 어떨까요?
쿼리 횟수가 1회 증가하지만 그것이 부담스럽지 않은 상태이고,
쿼리가 짧아져서 이해하기에도, 유지보수하기에도 더 좋은 상태가 될거 같아요!
또한 질문주신 문제점도 쉽게 해결해볼 수 있을거라 생각되네요!
리뷰어님 안녕하세요! 지난 번에 말씀드린 오류는 잘 해결되었습니다.
다만 현재 어려움을 겪고 있는 부분이 하나 있습니다.
테스트 시 예약 시간 및 테마는 잘 추가되는데요, 예약 추가 시 예약 ID와 예약자명을 제외한 나머지 정보는 화면에 출력되지 않고 있습니다. ReservationRepository에 SQL 구문이나 RowMapper 설정 쪽에 문제가 있을 것으로 예측되는데 잘 모르겠어 문의드립니다. 감사합니다!