-
Notifications
You must be signed in to change notification settings - Fork 147
[Spring MVC] 홍석주 미션 제출합니다. #415
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
base: somefood
Are you sure you want to change the base?
Changes from 5 commits
3e10081
58bbf3e
4583056
6088940
3d4516f
dc9ccee
0d5a4b6
d5630c0
ed1aec8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -9,22 +9,25 @@ | |||||||||||||||||||||||||||||||||||||||||
import roomescape.exception.NotFoundReservationException; | ||||||||||||||||||||||||||||||||||||||||||
import roomescape.reservation.Reservation; | ||||||||||||||||||||||||||||||||||||||||||
import roomescape.reservation.ReservationDao; | ||||||||||||||||||||||||||||||||||||||||||
import roomescape.time.Time; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
import java.sql.Date; | ||||||||||||||||||||||||||||||||||||||||||
import java.sql.PreparedStatement; | ||||||||||||||||||||||||||||||||||||||||||
import java.sql.Time; | ||||||||||||||||||||||||||||||||||||||||||
import java.util.List; | ||||||||||||||||||||||||||||||||||||||||||
import java.util.Objects; | ||||||||||||||||||||||||||||||||||||||||||
import java.util.Optional; | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
@Repository | ||||||||||||||||||||||||||||||||||||||||||
public class ReservationDaoImpl implements ReservationDao { | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
private static final RowMapper<Reservation> ALL_RESERVATION_ROW_MAPPER = (rs, rowNum) -> Reservation.ofExist( | ||||||||||||||||||||||||||||||||||||||||||
private static final RowMapper<Reservation> RESERVATION_ROW_MAPPER = (rs, rowNum) -> Reservation.ofExist( | ||||||||||||||||||||||||||||||||||||||||||
rs.getLong("id"), | ||||||||||||||||||||||||||||||||||||||||||
rs.getString("name"), | ||||||||||||||||||||||||||||||||||||||||||
rs.getDate("date").toLocalDate(), | ||||||||||||||||||||||||||||||||||||||||||
rs.getTime("time").toLocalTime() | ||||||||||||||||||||||||||||||||||||||||||
Time.ofExist( | ||||||||||||||||||||||||||||||||||||||||||
rs.getLong("time_id"), | ||||||||||||||||||||||||||||||||||||||||||
rs.getTime("time_value").toLocalTime() | ||||||||||||||||||||||||||||||||||||||||||
) | ||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
private final JdbcTemplate jdbcTemplate; | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -35,14 +38,17 @@ public ReservationDaoImpl(JdbcTemplate jdbcTemplate) { | |||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
@Override | ||||||||||||||||||||||||||||||||||||||||||
public Optional<Reservation> findById(Long id) { | ||||||||||||||||||||||||||||||||||||||||||
String sql = "SELECT * FROM reservation WHERE id = ?"; | ||||||||||||||||||||||||||||||||||||||||||
String sql = "SELECT " + | ||||||||||||||||||||||||||||||||||||||||||
"r.id as reservation_id," + | ||||||||||||||||||||||||||||||||||||||||||
"r.name," + | ||||||||||||||||||||||||||||||||||||||||||
"r.date," + | ||||||||||||||||||||||||||||||||||||||||||
"t.id as time_id," + | ||||||||||||||||||||||||||||||||||||||||||
"t.time as time_value " + | ||||||||||||||||||||||||||||||||||||||||||
"FROM reservation r " + | ||||||||||||||||||||||||||||||||||||||||||
"INNER JOIN reservation_time t ON r.time_id = t.id " + | ||||||||||||||||||||||||||||||||||||||||||
"WHERE r.id = ?"; | ||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
아래와 같이 관리하면 불필요한 + 단어를 제거할 수 있어 조금 더 가독성이 좋을 것 같습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 하도 자바 8만 쓰다보니 멀티라인의 존재를 자꾸 잊게 되네요 ㅎㅎ |
||||||||||||||||||||||||||||||||||||||||||
try { | ||||||||||||||||||||||||||||||||||||||||||
Reservation reservation = jdbcTemplate.queryForObject(sql, (rs, rowNum) -> Reservation.ofExist( | ||||||||||||||||||||||||||||||||||||||||||
rs.getLong("id"), | ||||||||||||||||||||||||||||||||||||||||||
rs.getString("name"), | ||||||||||||||||||||||||||||||||||||||||||
rs.getDate("date").toLocalDate(), | ||||||||||||||||||||||||||||||||||||||||||
rs.getTime("time").toLocalTime() | ||||||||||||||||||||||||||||||||||||||||||
), id); | ||||||||||||||||||||||||||||||||||||||||||
Reservation reservation = jdbcTemplate.queryForObject(sql, RESERVATION_ROW_MAPPER, id); | ||||||||||||||||||||||||||||||||||||||||||
return Optional.ofNullable(reservation); | ||||||||||||||||||||||||||||||||||||||||||
} catch (EmptyResultDataAccessException e) { | ||||||||||||||||||||||||||||||||||||||||||
return Optional.empty(); | ||||||||||||||||||||||||||||||||||||||||||
|
@@ -52,20 +58,27 @@ public Optional<Reservation> findById(Long id) { | |||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
@Override | ||||||||||||||||||||||||||||||||||||||||||
public List<Reservation> findAll() { | ||||||||||||||||||||||||||||||||||||||||||
String sql = "SELECT * FROM reservation"; | ||||||||||||||||||||||||||||||||||||||||||
return jdbcTemplate.query(sql, ALL_RESERVATION_ROW_MAPPER); | ||||||||||||||||||||||||||||||||||||||||||
String sql = "SELECT " + | ||||||||||||||||||||||||||||||||||||||||||
"r.id AS reservation_id," + | ||||||||||||||||||||||||||||||||||||||||||
"r.name," + | ||||||||||||||||||||||||||||||||||||||||||
"r.date," + | ||||||||||||||||||||||||||||||||||||||||||
"t.id AS time_id," + | ||||||||||||||||||||||||||||||||||||||||||
"t.time AS time_value " + | ||||||||||||||||||||||||||||||||||||||||||
"FROM reservation r " + | ||||||||||||||||||||||||||||||||||||||||||
"INNER JOIN reservation_time t ON r.time_id = r.id"; | ||||||||||||||||||||||||||||||||||||||||||
return jdbcTemplate.query(sql, RESERVATION_ROW_MAPPER); | ||||||||||||||||||||||||||||||||||||||||||
} | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
@Override | ||||||||||||||||||||||||||||||||||||||||||
public Long save(Reservation reservation) { | ||||||||||||||||||||||||||||||||||||||||||
String sql = "INSERT INTO reservation (name, date, time) VALUES (?, ?, ?)"; | ||||||||||||||||||||||||||||||||||||||||||
String sql = "INSERT INTO reservation (name, date, time_id) VALUES (?, ?, ?)"; | ||||||||||||||||||||||||||||||||||||||||||
KeyHolder keyHolder = new GeneratedKeyHolder(); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
jdbcTemplate.update(connection -> { | ||||||||||||||||||||||||||||||||||||||||||
PreparedStatement ps = connection.prepareStatement(sql, new String[]{"id"}); | ||||||||||||||||||||||||||||||||||||||||||
ps.setString(1, reservation.getName()); | ||||||||||||||||||||||||||||||||||||||||||
ps.setDate(2, Date.valueOf(reservation.getDate())); | ||||||||||||||||||||||||||||||||||||||||||
ps.setTime(3, Time.valueOf(reservation.getTime())); | ||||||||||||||||||||||||||||||||||||||||||
ps.setLong(3, reservation.getTime().getId()); | ||||||||||||||||||||||||||||||||||||||||||
return ps; | ||||||||||||||||||||||||||||||||||||||||||
}, keyHolder); | ||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
package roomescape.time; | ||
|
||
import java.time.LocalTime; | ||
|
||
public class Time { | ||
|
||
private Long id; | ||
|
||
private LocalTime time; | ||
|
||
private Time(Long id, LocalTime time) { | ||
this.id = id; | ||
this.time = time; | ||
} | ||
|
||
public static Time ofNew(LocalTime time) { | ||
return new Time(null, time); | ||
} | ||
|
||
public static Time ofExist(Long id, LocalTime time) { | ||
return new Time(id, time); | ||
} | ||
|
||
public Long getId() { | ||
return id; | ||
} | ||
|
||
public LocalTime getTime() { | ||
return time; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
package roomescape.time; | ||
|
||
import java.util.List; | ||
import java.util.Optional; | ||
|
||
public interface TimeDao { | ||
|
||
Optional<Time> findById(Long id); | ||
|
||
List<Time> findAll(); | ||
|
||
Time save(Time time); | ||
|
||
void delete(Long id); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
package roomescape.time.controller; | ||
|
||
import org.springframework.http.ResponseEntity; | ||
import org.springframework.web.bind.annotation.*; | ||
import roomescape.time.Time; | ||
import roomescape.time.TimeDao; | ||
import roomescape.time.dto.TimeCreateRequest; | ||
import roomescape.time.dto.TimeCreateResponse; | ||
|
||
import java.net.URI; | ||
import java.net.URISyntaxException; | ||
import java.util.List; | ||
|
||
@RestController | ||
public class TimeCommandController { | ||
Comment on lines
+17
to
+18
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 더불어 현재 시간 관리 페이지로 응답해주는 처리 담당이 없는 것 같습니다. 이 부분 추가해 주세요! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
저는 회사의 코드 영향 때문일 수도 있는데, 회사에서 석환님 방식대로 컨트롤러 상단 @RequestMapping에 공통적인 부분을 두어져 있는게 더러 있습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
아 요건 페이지 제공 안된줄 알고 넘겼는데, 있었군요..! 바로 추가하겠습니다 ㅎㅎ |
||
|
||
private final TimeDao timeDao; | ||
|
||
public TimeCommandController(TimeDao timeDao) { | ||
this.timeDao = timeDao; | ||
} | ||
|
||
@GetMapping("/times/{id}") | ||
public Time getTime(@PathVariable("id") long id) { | ||
return timeDao.findById(id) | ||
.orElseThrow(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 혹시 존재하지 않는 시간대를 조회하는 경우에 아무런 예외를 발생시키지 않는 이유는 무엇인가요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 다른 거 구현하다 간단하게 만들고 넘겼었습니다 |
||
} | ||
|
||
@GetMapping("/times") | ||
public List<Time> getTimes() { | ||
return timeDao.findAll(); | ||
} | ||
|
||
@PostMapping("/times") | ||
public ResponseEntity<TimeCreateResponse> createTime(@RequestBody TimeCreateRequest request) throws URISyntaxException { | ||
Time time = timeDao.save(Time.ofNew( | ||
request.getTime() | ||
)); | ||
|
||
URI uri = new URI("/times/" + time.getId()); | ||
return ResponseEntity | ||
.created(uri) | ||
.body(new TimeCreateResponse( | ||
time.getId(), | ||
time.getTime() | ||
)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Time 객체를 반환하는 요청과 TimeCreateResponse 객체로 반환하는 요청의 차이가 있을까요? 현재 Time 객체와 TimeCreateResponse 객체 간의 차이가 없다고 느껴지는데 분리하신 이유는 무엇인가요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 더불어서 dto를 이용하는 이유는 무엇인가요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 네 저번 1주차에도 해주셨던 질문 같은데요, 저번처럼 validation 목적으로 DTO를 만든 것도 있습니다. (아직은 안 쓰지만? ㅎㅎ) |
||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 해당 요청은 같은 시간을 중복해서 저장할 수 있을 것 같습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 오 맞네요 |
||
|
||
@DeleteMapping("/times/{id}") | ||
public ResponseEntity<Void> deleteTime(@PathVariable("id") long id) { | ||
timeDao.delete(id); | ||
|
||
return ResponseEntity.noContent().build(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
package roomescape.time.dto; | ||
|
||
import java.time.LocalTime; | ||
|
||
public class TimeCreateRequest { | ||
|
||
private LocalTime time; | ||
|
||
public LocalTime getTime() { | ||
return time; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package roomescape.time.dto; | ||
|
||
import java.time.LocalTime; | ||
|
||
public class TimeCreateResponse { | ||
|
||
private Long id; | ||
|
||
private LocalTime time; | ||
|
||
public TimeCreateResponse(Long id, LocalTime time) { | ||
this.id = id; | ||
this.time = time; | ||
} | ||
|
||
public Long getId() { | ||
return id; | ||
} | ||
|
||
public LocalTime getTime() { | ||
return time; | ||
} | ||
} |
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을 조회할 때 time 정보까지 한번에 불러오는 것 좋네요!