-
Notifications
You must be signed in to change notification settings - Fork 0
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
[1~6단계 - 방탈출 예약 관리] 커찬(이충안) 미션 제출합니다. #5
base: leegwichan
Are you sure you want to change the base?
[1~6단계 - 방탈출 예약 관리] 커찬(이충안) 미션 제출합니다. #5
Conversation
* feat(MainController): /admin 및 기본 welcome page 응답 기능 구현 * feat(AdminController): 예약 페이지 요청 응답 기능 구현 * feat(Reservation): 예약 리스트 조회 기능 구현 * fix(AdminController): 페이지 조회 기본 URL 변경 - /admin/* -> /* * feat(Reservation): 예약 추가 기능 구현 * feat(Reservation): 예약 취소 기능 구현 * fix: 요구 사항 수정 반영 - URL: /* -> /admin/* * refactor(ReservationRepository): Controller와 예약 저장소 분리 * refactor(ReservationRepository): 의존성 주입 도입 * fix(reservation-legacy.html): 기능 요구 수정에 따른 페이지 하이퍼링크 변경 적용 * style(Reservation): class 하단 공백 추가 * refactor(ReservationRepository): 필드가 재할당되지 않기 위해 final 키워드 추가 --------- Co-authored-by: DobbyMac <[email protected]>
- 각종 조건 추가 등을 대비하여, 메서드 이름 구체화 - 메서드 이름에 Reservation 키워드 제거
- @DirtiesContext 대신 DB를 매번 초기화 해줌으로써 각 테스트 별로 데이터를 구분함
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.
수고 많았오!!
@@ -0,0 +1,21 @@ | |||
package roomescape; |
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.
나도 1~3 단계에선 하지 않았지만 API 명세라던가 기능구현 목록을 추가해주는게 좋을 거 같아!
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.
응! 리뷰어한테 같은 피드백을 받았어!
그래서 9단계까지 끝내고 README에 명세서를 추가했어!
URI location = URI.create("/reservations/" + newReservation.id()); | ||
return ResponseEntity.created(location).body(newReservation); |
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.
이 부분 나는 뭔가 문자열을 + 연산하는게 껄끄럽다고 해야하나 암튼 그래서 아래처럼 처리했는데 맘에 드는 방식대로 해도 괜찮을 듯!
@PostMapping
public ResponseEntity<ReservationResponse> save(@RequestBody final ReservationRequest reservationRequest) {
final ReservationResponse reservationResponse = reservationService.save(reservationRequest);
final URI uri = UriComponentsBuilder.fromPath("/reservations/{id}")
.buildAndExpand(reservationResponse.id())
.toUri();
return ResponseEntity.created(uri)
.body(reservationResponse);
}
import roomescape.Reservation; | ||
|
||
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) | ||
@Sql(scripts = "/truncate.sql", executionPhase = Sql.ExecutionPhase.BEFORE_TEST_METHOD) |
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.
오 @Sql 애노테이션을 좀 더 쉽게 테스트 할 수 있겠군 👍 배워갑니다
@@ -0,0 +1,39 @@ | |||
package roomescape.acceptance; |
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.
현재 테스트가 LMS에서 제공하는 테스트 밖에 없는 거 같은데 다른 테스트도 추가해주!!!
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.
이번 단위에서는 크게 복잡하지 않아서, 다른 테스트는 추가하지 않았음! 아마 다음 미션부터는 의식적으로 테스트를 짤 것 같아. 대신에 LMS에서 제공하는 테스트를 가공해서 작성했어!
} | ||
|
||
@GetMapping | ||
public ResponseEntity<List<Reservation>> listReservations() { |
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.
listReservations 보다 좀 더 좋은 이름이 있지 않을까?
모든 예약을 반환해준다는 의미를 전달해주면 좋을 거 같아
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.
https://www.slipp.net/questions/79 -> 포비(자바지기)에게 이야기 해보세요.
는 농담이고, 네이밍 컨벤션을 찾아봤는데 위 자료가 보였고, 납득 가능한 것 같아서 사용했어
(여기서 list는 동사형으로 list의 의미로 봤을 떄)
|
||
@DisplayName("어드민 메인 페이지 조회") | ||
@Test | ||
void get_welcomePage() { |
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.
get 다음은 스네이크케이스고 welcomePage는 카멜케이스인데 통일해주!
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.
일부러 통일 안했음!
get 요청으로 날린다는 것을 강조하기 위해, get_welcomPage() 라고 했어!
|
||
@DisplayName("어드민 예약 관리 페이지 조회") | ||
@Test | ||
void get_reservationPage() { |
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.
ditto
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.
ditto
@@ -0,0 +1,13 @@ | |||
package roomescape; | |||
|
|||
public record 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.
reservation을 record로 만든 이유가 궁금해
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의 역할은 DTO와 크게 다르지 않다고 생각해.
클라이언트에서 받은 정보를 그대로 DB에게 주고, DB에게 받은 정보를 그대로 클라이언트에게 넘기니까
이후에 추가적인 요구사항이나, 추가적인 작업이 필요하다면 class로 바꿀 것 같아.
} | ||
|
||
private Integer countReservation() { | ||
return jdbcTemplate.queryForObject("SELECT count(1) from reservation", Integer.class); |
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.
from
-> FROM
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.
이걸 놓치네... (나는 이런 부분이 약한듯...)
assertThat(createdReservation).isEqualTo(expectedReservation); | ||
assertThat(countReservation()).isEqualTo(1); |
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.
위에 createdReservation 처럼 아래 countReservation 메서드도 변수로 만든다음 테스트하는건 어떻게 생각해??
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.
나는 거꾸로 assertThat(countReservation()).isEqualTo(1);
형식으로 통일했어!
이것 만으로도 삭제 후에 카운트했다는 것을 알 수 있지 않을까? (아닌가?)
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.
레디! 남겨준 리뷰 잘봤어! 땡큐! 레디가 남겨줬던 코멘트에 간단한 생각 남겨놨어!
추가적인 의견 있으면 코멘트 남겨도 되고, 직접 불러서 물어봐도 되!
@@ -0,0 +1,21 @@ | |||
package roomescape; |
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.
응! 리뷰어한테 같은 피드백을 받았어!
그래서 9단계까지 끝내고 README에 명세서를 추가했어!
@@ -0,0 +1,13 @@ | |||
package roomescape; | |||
|
|||
public record 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.
지금 Reservation의 역할은 DTO와 크게 다르지 않다고 생각해.
클라이언트에서 받은 정보를 그대로 DB에게 주고, DB에게 받은 정보를 그대로 클라이언트에게 넘기니까
이후에 추가적인 요구사항이나, 추가적인 작업이 필요하다면 class로 바꿀 것 같아.
} | ||
|
||
@GetMapping | ||
public ResponseEntity<List<Reservation>> listReservations() { |
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.
https://www.slipp.net/questions/79 -> 포비(자바지기)에게 이야기 해보세요.
는 농담이고, 네이밍 컨벤션을 찾아봤는데 위 자료가 보였고, 납득 가능한 것 같아서 사용했어
(여기서 list는 동사형으로 list의 의미로 봤을 떄)
|
||
@DisplayName("어드민 메인 페이지 조회") | ||
@Test | ||
void get_welcomePage() { |
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.
일부러 통일 안했음!
get 요청으로 날린다는 것을 강조하기 위해, get_welcomPage() 라고 했어!
@@ -0,0 +1,39 @@ | |||
package roomescape.acceptance; |
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.
이번 단위에서는 크게 복잡하지 않아서, 다른 테스트는 추가하지 않았음! 아마 다음 미션부터는 의식적으로 테스트를 짤 것 같아. 대신에 LMS에서 제공하는 테스트를 가공해서 작성했어!
|
||
@DisplayName("어드민 예약 관리 페이지 조회") | ||
@Test | ||
void get_reservationPage() { |
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.
ditto
assertThat(createdReservation).isEqualTo(expectedReservation); | ||
assertThat(countReservation()).isEqualTo(1); |
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.
나는 거꾸로 assertThat(countReservation()).isEqualTo(1);
형식으로 통일했어!
이것 만으로도 삭제 후에 카운트했다는 것을 알 수 있지 않을까? (아닌가?)
} | ||
|
||
private Integer countReservation() { | ||
return jdbcTemplate.queryForObject("SELECT count(1) from reservation", Integer.class); |
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.
이걸 놓치네... (나는 이런 부분이 약한듯...)
1 ~ 3단계 PR 메시지
이전 Spring 경험
신경 쓴 점
관습적으로 사용하던 구조 탈피
처음 Spring을 배웠을 때에는 주어진 코드를 따라치기 급급했었습니다. 그래서 '정말 이런 구조가 필요로 할까?'라는 생각이 하지 못했습니다. 이후, 객체 지향의 여러 원칙들을 배우는 과정에서 Spring에서 관습적으로 사용하던 구조가 정말 유의미한 지에 대해 의문이 들었습니다. 그래서 이번 과제에서는 '현재 요구사항에 필요한 최소한의 구조'를 사용하는 데 집중했습니다.
현재 요구사항에서 Controller - Service - Repository 가 필요할까?
현재의
ReservationController
는 예약 조회, 생성, 삭제를 담당하고 있습니다.ReservationController
에서 저장되어 있는 예약까지 관리하는 것은 역할이 과하다고 생각하여ReservationRepository
를 추가했습니다. 하지만 지금 상황에서 Service까지 추가할 필요하는 없다고 생각하여 Service를 따로 만들지 않았습니다.만약에 예약이 특정 조건에서 생성이 되야 한다거나 다른 객체들과 복잡하게 연결이 된다면, Service 계층을 분리할 수 있을 것 같습니다.
현재 요구사항에서 Domain과 DTO를 나눠야 할까?
현재 요구사항은 단순히 예약 정보를 저장하고 조회하는 방식입니다. 객체의 저장 및 조회를 어플리케이션 내
ArrayList
로 관리하고, 응답을 구성할 때는Reservation
객체에 있는 정보를 그대로 사용합니다. 그래서 도메인 객체와 DTO 객체를 구분하지 않고, 하나의Reservation
객체를 사용했습니다.만약에 데이터베이스가 도입이 되고 수정 포인트가 달라지게 된다면(API 수정으로 인한 변경과 DB Table 수정으로 인한 변경), DTO와 Domain 객체를 분리할 수 있을 것 같습니다.
Domain에 시간 관리 객체(
LocalDateTime
)을 도입해야 할까?현재 요구사항은 단순히 예약 정보를 저장하고 조회하는 방식입니다. 그 이외의 절차가 필요하지 않았습니다. 그래서 일단 String을 이용해서 관리하도록 했습니다.
이후 예약 날짜, 예약 시간과 관련된 요구 사항이 추가된다면,
LocalDate
와 같은 시간 관리 객체를 도입할 수 있을 것 같습니다.궁금한 점
확장성 고려하기
이번에 작성한 프로젝트는 "어떻게 해야 간단한 코드로 요구 사항을 만족시킬 수 있을까?"에 초점을 맞췄습니다. 지금 제 코드를 다시 돌아보면, 잘 작성한 코드인지 의문이 드는 것 같습니다. 저에게는 "미래를 예측하여 코드를 복잡하게 만드는 것이 옳지 않다."는 말과 "확장성을 고려해야 한다."는 말이 반댓말 같이 받아 들여지기도 합니다.
'확장성을 고려하면서도, 복잡하지 않는 코드'는 어떤 의미일까요?