-
Notifications
You must be signed in to change notification settings - Fork 164
[Spring MVC] 김시영 미션 제출합니다. #464
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: wfs0502
Are you sure you want to change the base?
Changes from all commits
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 |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| package roomescape; | ||
|
|
||
| import roomescape.exception.InvalidReservationException; | ||
|
|
||
| public class Reservation { | ||
| private long id; | ||
| private String name; | ||
| private String date; | ||
| private String time; | ||
|
|
||
| public Reservation() { | ||
|
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. 아무것도 안받고 사용하지 않는 Reservation이라는 생성자가 있군요. 만약 요렇게 생성된 객체는 실제로 유효한 Reservation일까요? |
||
| } | ||
|
|
||
| public Reservation(long id, String name, String date, String time) { | ||
| validate(name, date, time); | ||
|
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. 요런 validate는 테스트 코드로 검증해보면 좋았을 것 같아요 |
||
| this.id = id; | ||
| this.name = name; | ||
| this.date = date; | ||
| this.time = time; | ||
| } | ||
|
|
||
| private void validate(String name, String date, String time){ | ||
| if (name == null || name.isEmpty() || | ||
|
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. Reservation은 일종의 도메인적 성격을 띄고 있다고 생각해요. 요런 부분은 입력에 대한 검증부분인데 어떻게 해결해볼 수 있을까요? 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. 요렇게 특정 값이 null인지 비어있는지 확인하는 로직들은 굉장히 많습니다. 이 어노테이션과 DTO를 조합하면 정말 효과적으로 사용할 수 있습니다. 이에 대해서도 찾아보고 적용해보면 좋을 것 같아요 |
||
| date == null || date.isEmpty() || | ||
| time == null || time.isEmpty()) { | ||
| throw new InvalidReservationException(); | ||
| } | ||
| } | ||
|
|
||
| public long getId() { | ||
|
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. 요런 반복적인 코드를 나중에도 작성을 많이 해야합니다. 이런것을 해결하기 위해 lombok이라는 라이브러리가 있는데 이것도 한번 찾아서 학습해보면 좋을 것 같아요. 요 lombok은 진짜 많이 씁니다. |
||
| return id; | ||
| } | ||
|
|
||
| public String getName() { | ||
| return name; | ||
| } | ||
|
|
||
| public String getDate() { | ||
| return date; | ||
| } | ||
|
|
||
| public String getTime() { | ||
| return time; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,68 @@ | ||
| package roomescape.controller; | ||
|
|
||
| import java.net.URI; | ||
| import java.util.ArrayList; | ||
| import java.util.List; | ||
| import java.util.concurrent.atomic.AtomicLong; | ||
| import org.springframework.http.ResponseEntity; | ||
| import org.springframework.stereotype.Controller; | ||
| import org.springframework.web.bind.annotation.DeleteMapping; | ||
| import org.springframework.web.bind.annotation.ExceptionHandler; | ||
| import org.springframework.web.bind.annotation.GetMapping; | ||
| import org.springframework.web.bind.annotation.PathVariable; | ||
| import org.springframework.web.bind.annotation.PostMapping; | ||
| import org.springframework.web.bind.annotation.RequestBody; | ||
| import org.springframework.web.bind.annotation.ResponseBody; | ||
| import roomescape.Reservation; | ||
| import roomescape.exception.InvalidReservationException; | ||
| import roomescape.exception.NotFoundReservationException; | ||
|
|
||
| @Controller | ||
| public class ReservationController { | ||
|
|
||
| private final List<Reservation> reservations = new ArrayList<>(); | ||
| private final AtomicLong index = new AtomicLong(0); | ||
|
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. AtomicLong은 어떤 것이고 어떤 효과가 있나요? |
||
|
|
||
| @GetMapping("/reservation") | ||
| public String reservation() { | ||
| return "reservation"; | ||
| } | ||
|
|
||
| @GetMapping("/reservations") | ||
| @ResponseBody | ||
| public List<Reservation> getReservations() { | ||
| return reservations; | ||
| } | ||
|
|
||
| @PostMapping("/reservations") | ||
| public ResponseEntity<Reservation> addReservation(@RequestBody Reservation reservation) { | ||
|
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라는 것이 있습니다. 이렇게 RequestBody로 입력을 받는 것은 위험할 수 있습니다. 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에 대해 찾아보고 적용해보면 좋을것 같아요. 너무 이해가 안간다 싶으면 상현님 코드를 봐도 조금 도움이 될 것 같습니다. |
||
| Reservation newReservation = new Reservation( | ||
| index.incrementAndGet(), | ||
| reservation.getName(), | ||
| reservation.getDate(), | ||
| reservation.getTime() | ||
| ); | ||
|
|
||
| reservations.add(newReservation); | ||
|
|
||
| return ResponseEntity.created(URI.create("/reservations/" + newReservation.getId())) | ||
| .body(newReservation); | ||
| } | ||
|
|
||
| @DeleteMapping("/reservations/{id}") | ||
| public ResponseEntity<Void> deleteReservation(@PathVariable long id) { | ||
| Reservation reservation = reservations.stream() | ||
|
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. 잘 짜셨습니다. 로직이 깔끔하군요. 하지만, 하나 더 개선해볼만한 포인트가 있을 것 같아요. 존재하지 않는 경우엔 더 걸리죠. 요걸 O(1)안에 찾게하려면 어떻게 해야할까요? |
||
| .filter(it -> it.getId() == id) | ||
| .findFirst() | ||
| .orElseThrow(NotFoundReservationException::new); | ||
|
|
||
| reservations.remove(reservation); | ||
|
|
||
| return ResponseEntity.noContent().build(); | ||
| } | ||
|
|
||
| @ExceptionHandler({NotFoundReservationException.class, InvalidReservationException.class}) | ||
|
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. Controller안에서 ExceptionHandler를 쓸 수도 있지만 다른 방법도 있습니다. 한번 ControllerAdvice에 대해서 알아보고 적용해보면 좋을 것 같습니다! |
||
| public ResponseEntity<String> handleNotFoundException(RuntimeException e) { | ||
| return ResponseEntity.badRequest().body(e.getMessage()); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| package roomescape.exception; | ||
|
|
||
| public class InvalidReservationException extends RuntimeException { | ||
|
|
||
| public InvalidReservationException() { | ||
| super("필수 인자값이 비어있습니다."); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| package roomescape.exception; | ||
|
|
||
| public class NotFoundReservationException extends RuntimeException { | ||
|
|
||
| public NotFoundReservationException() { | ||
| super("예약 내역을 찾을 수 없습니다."); | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -20,7 +20,7 @@ | |
| <nav id="sidebar" class="text-white"> | ||
| <!-- 로고 영역 --> | ||
| <div class="sidebar-logo p-4"> | ||
| <a href="/"> | ||
| <a href="/static"> | ||
|
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. 이걸 home.html에서 index.html로 이름을 바꾸고 별도의 controller를 안 만든건 인상적이네요 |
||
| <img src="https://avatars.githubusercontent.com/u/141792611?s=96&v=4" alt="logo" class="img-fluid rounded-circle"> | ||
| </a> | ||
| </div> | ||
|
|
||
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.
요거는 한번 이야기했어야 했는데 조금 늦었군요.
혹시 자바의 final 키워드에 대해 아시나요? 변수에 대해서 reassign 을 막는 키워드인데, 객체의 불변을 유지하기에 좋습니다.
이 키워드를 이용해서 우리는 immutable하게 값을 사용할 수 있습니다. 최근 개발 패러다임에선 이런 immutable한 값을 사용하는게 선호되고 있는데 왜 그런걸까요?