Skip to content

[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

Open
wants to merge 9 commits into
base: somefood
Choose a base branch
from
Open
13 changes: 7 additions & 6 deletions src/main/java/roomescape/reservation/Reservation.java
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
package roomescape.reservation;

import roomescape.time.Time;

import java.time.LocalDate;
import java.time.LocalTime;
import java.util.Objects;

public class Reservation {
Expand All @@ -12,20 +13,20 @@ public class Reservation {

private final LocalDate date;

private final LocalTime time;
private final Time time;

private Reservation(Long id, String name, LocalDate date, LocalTime time) {
private Reservation(Long id, String name, LocalDate date, Time time) {
this.id = id;
this.name = Objects.requireNonNull(name, "이름은 필수입니다.");
this.date = Objects.requireNonNull(date, "예약 날짜는 필수 입니다.");
this.time = Objects.requireNonNull(time, "예약 시간은 필수입니다.");
}

public static Reservation ofNew(String name, LocalDate date, LocalTime time) {
public static Reservation ofNew(String name, LocalDate date, Time time) {
return new Reservation(null, name, date, time);
}

public static Reservation ofExist(Long id, String name, LocalDate date, LocalTime time) {
public static Reservation ofExist(Long id, String name, LocalDate date, Time time) {
return new Reservation(id, name, date, time);
}

Expand All @@ -41,7 +42,7 @@ public LocalDate getDate() {
return date;
}

public LocalTime getTime() {
public Time getTime() {
return time;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import roomescape.reservation.ReservationDao;
import roomescape.reservation.dto.ReservationCreateRequest;
import roomescape.reservation.dto.ReservationCreateResponse;
import roomescape.time.Time;
import roomescape.time.TimeDao;

import java.net.URI;
import java.net.URISyntaxException;
Expand All @@ -15,9 +17,11 @@
public class ReservationCommandController {

private final ReservationDao reservationDao;
private final TimeDao timeDao;

public ReservationCommandController(ReservationDao reservationDao) {
public ReservationCommandController(ReservationDao reservationDao, TimeDao timeDao) {
this.reservationDao = reservationDao;
this.timeDao = timeDao;
}

@GetMapping("/reservations")
Expand All @@ -28,10 +32,13 @@ public ResponseEntity<List<Reservation>> getReservationList() {

@PostMapping("/reservations")
public ResponseEntity<ReservationCreateResponse> createReservation(@RequestBody ReservationCreateRequest request) throws URISyntaxException {
Time time = timeDao.findById(request.getTime())
.orElseThrow();

Long id = reservationDao.save(Reservation.ofNew(
request.getName(),
request.getDate(),
request.getTime()
time
));

URI uri = new URI("/reservations/" + id);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,6 @@ public class ReservationController {

@GetMapping("/reservation")
public String getReservationPage() {
return "reservation";
return "new-reservation";
}
}
Original file line number Diff line number Diff line change
@@ -1,15 +1,14 @@
package roomescape.reservation.dto;

import java.time.LocalDate;
import java.time.LocalTime;

public class ReservationCreateRequest {

private String name;

private LocalDate date;

private LocalTime time;
private Long time;

public String getName() {
return name;
Expand All @@ -19,8 +18,7 @@ public LocalDate getDate() {
return date;
}

public LocalTime getTime() {
public Long getTime() {
return time;
}

}
41 changes: 27 additions & 14 deletions src/main/java/roomescape/reservation/infra/ReservationDaoImpl.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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 = ?";

Choose a reason for hiding this comment

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

조인을 이용해서 reservation을 조회할 때 time 정보까지 한번에 불러오는 것 좋네요!

Choose a reason for hiding this comment

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

Suggested change
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 = ?";
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 = ?
""";

아래와 같이 관리하면 불필요한 + 단어를 제거할 수 있어 조금 더 가독성이 좋을 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

The 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();
Expand All @@ -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);

Expand Down
31 changes: 31 additions & 0 deletions src/main/java/roomescape/time/Time.java
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;
}
}
15 changes: 15 additions & 0 deletions src/main/java/roomescape/time/TimeDao.java
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

Choose a reason for hiding this comment

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

@RequestMapping을 활용해서 url의 공통적인 부분을 추출해보는 것도 좋을 것 같아요
저는 @RequestMapping을 활용하면 개인적으로 한 눈에 요청 주체를 파악하기 쉬어서 @RequestMapping을 이용하는 것을 선호합니다.

Choose a reason for hiding this comment

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

더불어 현재 시간 관리 페이지로 응답해주는 처리 담당이 없는 것 같습니다. 이 부분 추가해 주세요!

Copy link
Author

Choose a reason for hiding this comment

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

@RequestMapping을 활용해서 url의 공통적인 부분을 추출해보는 것도 좋을 것 같아요 저는 @RequestMapping을 활용하면 개인적으로 한 눈에 요청 주체를 파악하기 쉬어서 @RequestMapping을 이용하는 것을 선호합니다.

저는 회사의 코드 영향 때문일 수도 있는데, 회사에서 석환님 방식대로 컨트롤러 상단 @RequestMapping에 공통적인 부분을 두어져 있는게 더러 있습니다.
근데 이것이 개인적으로 IDE에서 검색하기 힘들어져서 저는 각 Mapping에 full url 두는 방법을 좀 더 선호하게 되었습니다 ㅎㅎ
물론 /api 등의 공통된 접두사가 붙게되거나 하면 공통적인 부분을 추출하는것도 좋다 생각하지만, 현재 제 생각은 그렇습니다~!

Copy link
Author

Choose a reason for hiding this comment

The 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();

Choose a reason for hiding this comment

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

혹시 존재하지 않는 시간대를 조회하는 경우에 아무런 예외를 발생시키지 않는 이유는 무엇인가요?

Copy link
Author

Choose a reason for hiding this comment

The 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()
));

Choose a reason for hiding this comment

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

Time 객체를 반환하는 요청과 TimeCreateResponse 객체로 반환하는 요청의 차이가 있을까요? 현재 Time 객체와 TimeCreateResponse 객체 간의 차이가 없다고 느껴지는데 분리하신 이유는 무엇인가요?

Choose a reason for hiding this comment

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

더불어서 dto를 이용하는 이유는 무엇인가요?

Copy link
Author

Choose a reason for hiding this comment

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

네 저번 1주차에도 해주셨던 질문 같은데요, 저번처럼 validation 목적으로 DTO를 만든 것도 있습니다. (아직은 안 쓰지만? ㅎㅎ)
지금은 단순한 컨트롤러이지만 추가 요구 사항이 발생해서 여러 객체와 협업해야하는 상황에서 Time 객체로 입력을 받으면 추후 유연성이 떨어질 것을 고려하기에 여러 값을 받을 수 있는 DTO로 구성하고, 해당 객체로 비즈니스 로직에서 다양하게 활용할 수 있게 하기 위해서 사용 했습니다~!

}

Choose a reason for hiding this comment

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

해당 요청은 같은 시간을 중복해서 저장할 수 있을 것 같습니다.

Copy link
Author

Choose a reason for hiding this comment

The 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();
}
}
13 changes: 13 additions & 0 deletions src/main/java/roomescape/time/dto/TimeCreateRequest.java
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;
}
}

23 changes: 23 additions & 0 deletions src/main/java/roomescape/time/dto/TimeCreateResponse.java
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;
}
}
Loading