[1 - 3 단계 방탈출 사용자 예약] 로빈(임수빈) 미션 제출합니다. #29

robinjoon · 2024-05-02T08:18:19Z

제이미 안녕하세요! 이번 미션 리뷰이 로빈입니다. 미션 수행 과정에서 여러 애로사항이 있었습니다. 리뷰할 때 이를 감안해주셨으면 해서 자세히 적었습니다.

개인적인 사정으로 인한 시간 제한

미션 진행 기간에 제가 이사를 가게 되어서 수요일 하루를 쓸 수 없었고, 목요일 오전에는 국민 취업 지원제도 상담 일정과 목요일 오후 테코톡 근로 활동을 하게 되어 미션을 수행할 수 있는 절대적인 시간이 절반으로 줄어든 상황이었습니다.

SQL 대신 Java Stream 활용

앞서 말씀드린 시간 제한으로 인해 SQL, 레퍼지토리, 서비스 를 모두 작성하고 테스트할 시간이 없다고 생각했습니다. 이에 모든 데이터를 가져오는 쿼리를 작성하고 대부분의 기능은 이 기능 + Stream 을 이용해 구현했습니다. 이것이 일반적으로 지양되는 것임을 잘 알고 있습니다. 하지만 요구사항이 동작하기 때문에 넘어갔습니다.

페어 프로그래밍 방식 변경

전통적인 페어프로그래밍 방식으로는 도저히 시간 내에 미션을 수행할 수 없을 것이고, 페어의 시간이 너무 낭비된다고 생각했습니다. 따라서, 화요일 하루동안 1, 2단계, 3단계 - 사용자 예약 기능을 거의 테스트 없이 작성한 뒤 수요일에 페어가 이의 테스트 코드를 작성하고 제가 수요일 늦은 밤에 이를 검토하고 코멘트를 남겨둔 뒤 목요일에 머지하는 방식으로 미션을 수행했습니다.

3단계 인기 테마 기능은 제가 국취제 상담 끝나고 캠퍼스 가는 길 지하철에서 코드를 작성했습니다. 이 코드를 페어와 함께 캠퍼스에서 확인하고 반영했습니다.

스프링 관련 경험

Servlet & JSP 기반 웹 페이지 개발 및 운영 경험이 있습니다.
Spring Boot 2.7 기반으로 프로젝트 진행 후 배포 및 운영 경험이 있습니다.

이번 단계 변경 사항

- 상태코드를 200 -> 201 변경 - Location 헤더 추가 Co-authored-by: zangsu <[email protected]>

- 상태코드를 200 -> 204 변경 Co-authored-by: zangsu <[email protected]>

Co-authored-by: zangsu <[email protected]>

…시 에러 발생 구현 Co-authored-by: zangsu <[email protected]>

Co-authored-by: zangsu <[email protected]>

- 어차피 자원이 존재하지 않는 것은 동일하므로 문제가 없다고 결정 Co-authored-by: zangsu <[email protected]>

Co-authored-by: zangsu <[email protected]>

구현 Co-authored-by: zangsu <[email protected]>

Co-authored-by: zangsu <[email protected]>

- 테마 이름, 설명, 썸네일 이미자가 비어 있을 경우 에러 - 중복된 이름의 테마 생성 요청시 에러 Co-authored-by: zangsu <[email protected]>

Co-authored-by: zangsu <[email protected]>

- 이미 구현된 기능의 체크 리스트 누락된 것 작성 Co-authored-by: zangsu <[email protected]>

Co-authored-by: zangsu <[email protected]>

- 이미 작성함.

- Controller 는 서비스의 코드를 그대로 호출하므로 테스트 작성이 필요하지 않다고 판단 - 단, 기존의 잘 동작하는 테스트 코드를 삭제하지는 않음

… 생각이 듭니다." 삭제 - 각자 추구하는 방향이 달라 페어 프로그래밍이 끝난 후 각자 개선하는 것으로 합의

- 도메인으로 이동할 수 있는 책임을 이동

- 각자 추구하는 방향이 달라 페어 프로그래밍이 끝난 후 각자 개선하는 것으로 합의

- 4~6 단계 인증 인가가 들어갈 때 고민이 해소 될 것이라 판단

robinjoon · 2024-05-02T08:26:30Z

src/main/java/roomescape/exception/ExceptionType.java

+    EMPTY_NAME(BAD_REQUEST, "이름은 필수 값입니다."),
+    EMPTY_TIME(BAD_REQUEST, "시작 시간은 필수 값입니다."),
+    EMPTY_DATE(BAD_REQUEST, "날짜는 필수값 입니다."),
+    EMPTY_THEME(BAD_REQUEST, "테마는 필수값 입니다."),
+    EMPTY_DESCRIPTION(BAD_REQUEST, "테마 설명은 필수값 입니다."),
+    EMPTY_THUMBNAIL(BAD_REQUEST, "테마 썸네일은 필수값 입니다."),
+    PAST_TIME_RESERVATION(BAD_REQUEST, "이미 지난 시간에 예약할 수 없습니다."),
+    DUPLICATE_RESERVATION(BAD_REQUEST, "같은 시간에 이미 예약이 존재합니다."),
+    DUPLICATE_RESERVATION_TIME(BAD_REQUEST, "이미 예약시간이 존재합니다."),
+    DUPLICATE_THEME(BAD_REQUEST, "이미 동일한 테마가 존재합니다."),
+    INVALID_DATE_TIME_FORMAT(BAD_REQUEST, "해석할 수 없는 날짜, 시간 포맷입니다."),
+    DELETE_USED_TIME(BAD_REQUEST, "예약이 존재하는 시간은 삭제할 수 없습니다."),
+    DELETE_USED_THEME(BAD_REQUEST, "예약이 존재하는 테마는 삭제할 수 없습니다."),
+    NOT_FOUND_RESERVATION_TIME(BAD_REQUEST, "존재하지 않는 시간입니다."),
+    NOT_FOUND_THEME(BAD_REQUEST, "없는 테마입니다."),


모든 HttpStatus가 BAD_REQUEST 로 동일하지만, 4 ~ 6단계에 인증 관련 기능을 구현하면서 다른 상태 코드가 사용될 것이라 생각해 HttpStatus 를 지정할 수 있도록 유지했습니다.

4~6단계를 현재 단계에서 미리 짐작하고 프로그래밍해도 괜찮을까요?

4~6단계를 현재 단계에서 미리 짐작하고 프로그래밍해도 괜찮을까요?

상황 설명을 조금 더 구체적으로 말씀드리겠습니다!

미션을 구현하는 과정에서, 각 예외 발생 시 HTTP 상태코드가 정상적인 상황과 다르다는 것을 인지

따라서 예외가 HTTP 상태코드를 결정한다면 좋을 것 같다고 판단

구현 과정에서 HTTP 상태코드가 모두 BAD_REQUEST 임을 인지

고정적으로 HTTP 상태코드가 모두 BAD_REQUEST를 반환하도록 할지 고민

이후 단계 미션 과정에서 고정적으로 HTTP 상태코드가 모두 BAD_REQUEST를 반환하는 것이 유리하지 않을 것이라 판단.

유지 보수를 좋게 하기 위해 리팩토링을 한다면, 미래의 요구사항 변경 방향성을 예상하고 이를 근거로 결정하는 것이 당연하다고 생각합니다.

그렇다면 4~6단계에 구현하면서 다른 상태 코드를 사용하려는 것보단,
추후 예외에 따른 다른 응답을 제공하기 위해라고 이해하면 되겠군요!

jamie9504

안녕하세요, 로빈 😄 리뷰어 제이미입니다.

남겨주신 코멘트 잘 읽어보았습니다. 💪

간략한 코멘트들만 남겨두었으니, 리팩터링 후 리뷰요청 주세요!
오늘도 좋은 하루 보내세요~!

jamie9504 · 2024-05-03T03:52:40Z

README.md

@@ -0,0 +1,248 @@
+# 요구사항 문서
+
+- [x] API 명세를 현재 프론트엔드 코드가 잘 동작할 수 있도록 수정


SQL 대신 Java Stream 활용
앞서 말씀드린 시간 제한으로 인해 SQL, 레퍼지토리, 서비스 를 모두 작성하고 테스트할 시간이 없다고 생각했습니다. 이에 모든 데이터를 가져오는 쿼리를 작성하고 대부분의 기능은 이 기능 + Stream 을 이용해 구현했습니다. 이것이 일반적으로 지양되는 것임을 잘 알고 있습니다. 하지만 요구사항이 동작하기 때문에 넘어갔습니다.

이 부분은 그러면 리팩터링해보도록 하는 것으로 하죠 💪

jamie9504 · 2024-05-03T03:54:10Z

src/main/java/roomescape/exception/ExceptionType.java

+    EMPTY_NAME(BAD_REQUEST, "이름은 필수 값입니다."),
+    EMPTY_TIME(BAD_REQUEST, "시작 시간은 필수 값입니다."),
+    EMPTY_DATE(BAD_REQUEST, "날짜는 필수값 입니다."),
+    EMPTY_THEME(BAD_REQUEST, "테마는 필수값 입니다."),
+    EMPTY_DESCRIPTION(BAD_REQUEST, "테마 설명은 필수값 입니다."),
+    EMPTY_THUMBNAIL(BAD_REQUEST, "테마 썸네일은 필수값 입니다."),
+    PAST_TIME_RESERVATION(BAD_REQUEST, "이미 지난 시간에 예약할 수 없습니다."),
+    DUPLICATE_RESERVATION(BAD_REQUEST, "같은 시간에 이미 예약이 존재합니다."),
+    DUPLICATE_RESERVATION_TIME(BAD_REQUEST, "이미 예약시간이 존재합니다."),
+    DUPLICATE_THEME(BAD_REQUEST, "이미 동일한 테마가 존재합니다."),
+    INVALID_DATE_TIME_FORMAT(BAD_REQUEST, "해석할 수 없는 날짜, 시간 포맷입니다."),
+    DELETE_USED_TIME(BAD_REQUEST, "예약이 존재하는 시간은 삭제할 수 없습니다."),
+    DELETE_USED_THEME(BAD_REQUEST, "예약이 존재하는 테마는 삭제할 수 없습니다."),
+    NOT_FOUND_RESERVATION_TIME(BAD_REQUEST, "존재하지 않는 시간입니다."),
+    NOT_FOUND_THEME(BAD_REQUEST, "없는 테마입니다."),


4~6단계를 현재 단계에서 미리 짐작하고 프로그래밍해도 괜찮을까요?

jamie9504 · 2024-05-03T03:54:47Z

README.md

+    - [x] 중복된 이름의 테마 생성 요청시 에러
+    - [x] 예약이 있는 테마를 삭제 요청시 에러
+
+- [ ] 사용자 예약 기능 추가


jamie9504 · 2024-05-03T04:01:14Z

src/main/java/roomescape/controller/AvailableTimeController.java

+import roomescape.service.AvailableTimeService;
+
+@RestController
+@RequestMapping("/availableTimes")


Time이 아닌 AvailableTimes를 사용한 이유는 무엇인가요!

RESTFUL API에 대해 알아볼까요?

Time이 아닌 AvailableTimes를 사용한 이유는 무엇인가요!

기존에 존재하는 GET /times 의 응답으로 기대되는 자원과 다른 자원이라 생각해서 GET /availableTimes 를 사용했습니다. GET /times은 "방탈출이 진행될 수 있는 시간을 표현한 자원" 을 응답하지만, GET /availableTimes 은 "방탈출이 진행될 수 있는 시간과 예약 가능 여부를 표현한 자원"이기 때문에 별개의 uri 를 사용하도록 했습니다.

이를 URL Path로 명시하는 것과, Url Param으로 명시하는 것은 어떠한 차이가 있을까요?

jamie9504 · 2024-05-03T05:05:14Z

src/main/java/roomescape/exception/RoomescapeException.java

+
+import org.springframework.http.HttpStatus;
+
+public class RoomescapeException extends RuntimeException {


RoomescapeException이 RuntimeException를 구현하는 이유는 있을까요?

RoomescapeException이 RuntimeException를 구현하는 이유는 있을까요?

질문의 의도가 정확히 무엇인지 파악하기 힘들어서 생각나는 여러 관점으로 모두 서술해 보겠습니다.

커스텀 예외가 유용한가?

라이브러리나 프레임워크에서 투척하는 예외와 구분하기 용이하다.

Java의 표준 예외를 사용한다면, 다른 라이브러리의 문제로 발생한 예외인지, 우리가 작성한 코드에서 발생한 예외인지 코드레벨에서 구분하기가 힘듭니다. 메세지로만 구분할 수 있기 때문입니다. 따라서, 내부에서 사용하는 라이브러리의 예외가 클라이언트로 전달될 가능성이 생깁니다. 그리고, 이는 심각한 보안 문제로 이어질 수 있습니다.

반면, 직접 커스텀한 예외를 사용한다면 다른 프레임워크나 라이브러리에서 이를 사용하지 않으므로 커스텀한 예외가 우리가 작성한 코드에서 발생한 것임을 알 수 있습니다.

Checked 예외 vs Unchecked 예외

RuntimeException을 구현한 예외는 Unchecked 예외로 예외를 처리하는 코드를 명시적으로 작성하지 않아도 됩니다. 반면 Checked 예외는 예외를 처리하는 try-catch 문을 작성하거나, throws 키워드를 통해 메서드가 특정 예외를 발생시킨다는 사실을 명시해야 합니다.

Checked 예외의 단점

캡슐화를 해친다.

throws 키워드를 통해 메서드가 특정 예외를 발생시킨다는 사실을 명시하는 것은 더 높은 추상화 수준을 가진 클래스가 낮은 추상화 수준의 예외를 아는 것입니다. 따라서 이는 캡슐화를 깨는 것이고 이로 인해 객체지향 프로그래밍과 레이어드 아키텍처의 장점을 모두 퇴색시킵니다.

그렇다고 try - catch 문을 사용할 경우, 예외를 처리할 수 있어야 하는데, 대부분의 경우 복구할 수 없는 예외입니다. 예를 들어 사용자의 잘못된 입력으로 인해 발생한 예외나, 외부 시스템의 문제로 발생한 예외는 어플리케이션에서 복구할 수 없습니다.

Java는 물론 최신 개발 트랜드와 맞지 않는다.

Java 8 에 추가된 Function 인터페이스에는 throws 절이 없기 때문에 checked 예외와의 궁합이 좋지 않습니다. 다른 표준 함수형 인터페이스들도 마찬가지고 이는 Java가 예외를 바라보는 시각이 달라졌음을 의미합니다.

코틀린 등 여러 다른 언어에서는 Checked 예외를 지원하지 않습니다. 이는 Checked 예외가 언어에서 그 기능을 제외할 정도로 단점이 크다는 것을 의미합니다.

RuntimeException하위의 구체적인 예외를 상속하지 않은 이유

RoomescapeException 은 비즈니스 로직에 의해 발생하는 모든 예외를 표현할 때 사용하는 것을 의도하고 설계했습니다. 따라서, 많은 예외적인 상황에서 사용됩니다. 따라서 가장 추상적인 Unchecked 예외인 RuntimeException을 상속했습니다.

예를 들어 사용자로부터 예약자 이름에 허용되지 않은 문자가 포함되었다고 한다면, 이는 IllegalArgumentException을 사용할 수 있을 것입니다. 반면, 사용자가 이미 예약이 존재하는 시간과 테마에 예약을 요청한다면 이는 IllegalStateException이 보다 적합할 것입니다. 따라서, Java 에서 이미 만들어둔 구체적인 예외를 상속한다면 적절하지 않은 예외가 발생한다고 볼 수 있게 됩니다.

각종 측면에서 잘 알아봐주셨네요! 👍

마지막에 알아본, RuntimeException하위의 구체적인 예외를 상속하지 않은 이유를 생각해보면
해당 RoomescapeException 예외는 사실상 RuntimeException처럼 큰 예외로군요 😃

jamie9504 · 2024-05-03T05:06:55Z

src/main/java/roomescape/domain/Theme.java

+    }
+
+    private void validateThumbnail(String thumbnail) {
+        if (thumbnail == null || thumbnail.isBlank()) {


Blank가 아닌지만 검증해도 될까요?

Blank가 아닌지만 검증해도 될까요?

이미지 url이기 때문에 사실 url 형태인지, 나아가 해당 요청을 통해 이미지를 가져올 수 있는지 까지 검증할 필요성이 있다는 생각이 듭니다. 다음 PR에 반영하겠습니다.

jamie9504 · 2024-05-03T05:08:10Z

src/main/java/roomescape/dto/ReservationRequest.java

+
+import java.time.LocalDate;
+
+public record ReservationRequest(LocalDate date, String name, long timeId, long themeId) {


Request DTO들은 검증을 하지 않아도 괜찮을까요?

Request DTO들은 검증을 하지 않아도 괜찮을까요?

결론 : 네. 괜찮다고 생각합니다.

DTO에 검증 로직을 추가했을 때 장점

빠르게 실패할 수 있다.

Request DTO에 검증 로직을 추가하면 다른 레이어가 동작하기 전에 잘못된 요청을 검증할 수 있으므로 더 빠른 실패가 가능합니다. 이는 더 적은 리소스로 사용자에게 더 빠르게 응답할 수 있음을 의미합니다.

DTO에 검증 로직을 추가했을 때 단점

검증 로직이 여기 저기 흩어진다.

DTO에 검증 로직을 추가하더라도, 모든 것을 검증할 수 없기 때문에 도메인이나 서비스 등 다른 곳에도 어떤 검증 로직이 추가되어야만 합니다. 어떤 문제가 발생했을 때 확인해야 하는 부분이 늘어나게 됩니다.

중복된 검증이 발생할 수 있다.

DTO와 도메인에 모두 검증을 추가한다면 이는 중복된 코드가 됩니다.

중복된 검증을 피하는 과정에서 숨겨진 결합이 발생할 수 있다.

DTO에도 검증을 추가했다고 해서 도메인이 그 검증을 수행하지 않는다면 DTO와 도메인 사이에 숨겨진 결합이 생기게 됩니다. DTO의 검증에 문제가 있다면 이를 신뢰하는 도메인 객체 역시 문제가 생기기 때문입니다. 이런 종류의 결합은 코드 상에 의존 관계로 인해 명확히 들어난 결합보다 훨씬 찾기 힘들고 그렇기 때문에 훨씬 위험합니다.

이런 위험성을 감내하고 각 검증 로직을 나름의 기준에 따라 특정 레이어에서 하도록 배분한다고 합시다. 이 경우 그 기준을 정하는 비용, 기준에 의해 판단하는 비용, 기준을 유지보수하는 비용이 발생합니다.

현재 상황에서 장단점 간 비교

현재 시스템에서 필요한 여러 검증들은 도메인 객체와 서비스에 나뉘어 작성되어 있습니다. 즉, 이미 검증 로직이 여기 저기 흩어져 있는 상태입니다. 따라서 DTO 에 검증 로직을 추가한다면 검증 로직이 더욱 많은 곳으로 흩어지게 되므로 설상가상이 됩니다.

서비스와 도메인은 이미 서비스가 도메인을 의존하는 상황이기 때문에 명시적 결합이 존재합니다. 따라서 여기에 숨겨진 결합이 추가되더라도 이미 결합이 존재하기 때문에 어떤 문제가 발생했을 때 서비스와 도메인을 모두 확인해야 하는 것은 달라지지 않습니다. 하지만 DTO 는 도메인과 완전히 분리되어있기 때문에 둘 사이에 숨겨진 결합이 추가된다면 이는 단점을 그대로 가져가게 됩니다.

반면, 현재 서비스에서 실패를 응답하는 속도가 전혀 느리지 않고, DTO에서 검증이 가능한 작업 중 시간이 오래 걸리는 검증 역시 존재하지 않습니다. 따라서 DTO에 검증 로직을 추가하더라고 얻는 이점이 거의 없습니다.

jamie9504 · 2024-05-03T05:08:59Z

src/main/java/roomescape/repository/ThemeRepository.java

+import java.util.Optional;
+import roomescape.domain.Theme;
+
+public interface ThemeRepository {


Interface를 분리해서 얻은 장/단점은 무엇이 있을까요?

Interface를 분리해서 얻은 장/단점은 무엇이 있을까요?

장점

데이터베이스와 독립된 테스트를 작성할 수 있게 되었다.

만약 Repository 를 인터페이스로 만들지 않았다면 Service나 Controller의 단위 테스트에서 데이터베이스와 연결되는 레퍼지토리를 그대로 이용해 테스트를 작성해야 합니다. 이 경우 문제가 발생했을 때 문제의 원인이 잘못된 쿼리에 있는지 Service에 있는지 알기 위해서 분석이 필요합니다.

반면 인터페이스를 사용하면 테스트에서는 별도로 잘 구현된 혹은 특정 고정된 값만을 반환하는 테스트용 레포지토리 구현체를 사용할 수 있으므로 문제의 원인을 파악하기 쉬워집니다.

단점

인터페이스에 메서드가 추가될 때 해야 하는 작업이 많아진다.

기능의 추가로 인터페이스에 새로운 메서드가 필요해질 경우 인터페이스와 구현체 모두 수정이 필요하므로 작업량이 늘어나게 됩니다.

장점이 더 크다고 판단한 이유

한번 작성되고 유지보수되지 않을 시스템이라면, 테스트 용이성의 가치는 낮습니다. 테스트는 변경이 발생했을 때 시스템이 안정적으로 유지될 수 있도록 하기 위한 것인데, 유지보수되지 않을 것이라면 변경이 발생할 이유가 없기 때문입니다.

반면, 지금 작성하고 있는 시스템은 당장 리팩토링이 예정되어있고, 새로운 기능의 추가가 결정되어있는 상황입니다. 따라서, 테스트 용이성의 가치가 지금까지 만들었던 시스템들보다 큽니다. 따라서 당장의 작업량의 증가보다 앞으로의 테스트 용이성이 더 큰 가치라 판단했습니다.

궁금한 부분이, 인터페이스를 사용해야만 테스트용 구현체를 생성할 수 있나요?
상속이라던가를 이용하는 방법도 있?지 않나요

robinjoon · 2024-05-07T05:00:21Z

안녕하세요 제이미! 리팩토링 및 남겨주신 질문에 답변 달았습니다!

리팩토링 목록

Java Stream 만을 이용한 기능을 SQL 로 적절히 이용하도록 수정
Java 코드 내 SQL 컨벤션 통일

변경된 기능 목록

테마 추가 시 썸네일 이미지 제약 조건 추가 : http, https URL 로 제한
예약 가능한 시간 조회 API 명세 수정 : GET /availableTimes => GET /times/book-able

jamie9504

안녕하세요, 로빈 😄 리뷰어 제이미입니다.

이번 단계에서 해야하는 부분은 충족된 것 같아서 이만 Approve & Merge 합니다.

간략히 코멘트를 남겨두었으니, 확인하시면서 다음 단계 진행해주세요!
해당 코멘트에 대해 이야기 나눌 부분이 있다면, 다음 단계 PR 코멘트에 남겨주세요.

오늘도 좋은 하루 보내세요. 😃

jamie9504 · 2024-05-08T06:16:29Z

src/main/java/roomescape/exception/ExceptionType.java

+    EMPTY_NAME(BAD_REQUEST, "이름은 필수 값입니다."),
+    EMPTY_TIME(BAD_REQUEST, "시작 시간은 필수 값입니다."),
+    EMPTY_DATE(BAD_REQUEST, "날짜는 필수값 입니다."),
+    EMPTY_THEME(BAD_REQUEST, "테마는 필수값 입니다."),
+    EMPTY_DESCRIPTION(BAD_REQUEST, "테마 설명은 필수값 입니다."),
+    EMPTY_THUMBNAIL(BAD_REQUEST, "테마 썸네일은 필수값 입니다."),
+    PAST_TIME_RESERVATION(BAD_REQUEST, "이미 지난 시간에 예약할 수 없습니다."),
+    DUPLICATE_RESERVATION(BAD_REQUEST, "같은 시간에 이미 예약이 존재합니다."),
+    DUPLICATE_RESERVATION_TIME(BAD_REQUEST, "이미 예약시간이 존재합니다."),
+    DUPLICATE_THEME(BAD_REQUEST, "이미 동일한 테마가 존재합니다."),
+    INVALID_DATE_TIME_FORMAT(BAD_REQUEST, "해석할 수 없는 날짜, 시간 포맷입니다."),
+    DELETE_USED_TIME(BAD_REQUEST, "예약이 존재하는 시간은 삭제할 수 없습니다."),
+    DELETE_USED_THEME(BAD_REQUEST, "예약이 존재하는 테마는 삭제할 수 없습니다."),
+    NOT_FOUND_RESERVATION_TIME(BAD_REQUEST, "존재하지 않는 시간입니다."),
+    NOT_FOUND_THEME(BAD_REQUEST, "없는 테마입니다."),


그렇다면 4~6단계에 구현하면서 다른 상태 코드를 사용하려는 것보단,
추후 예외에 따른 다른 응답을 제공하기 위해라고 이해하면 되겠군요!

jamie9504 · 2024-05-08T06:17:09Z

src/main/java/roomescape/controller/AvailableTimeController.java

+import roomescape.service.AvailableTimeService;
+
+@RestController
+@RequestMapping("/availableTimes")


이를 URL Path로 명시하는 것과, Url Param으로 명시하는 것은 어떠한 차이가 있을까요?

jamie9504 · 2024-05-08T06:18:34Z

src/main/java/roomescape/exception/RoomescapeException.java

+
+import org.springframework.http.HttpStatus;
+
+public class RoomescapeException extends RuntimeException {


각종 측면에서 잘 알아봐주셨네요! 👍

마지막에 알아본, RuntimeException하위의 구체적인 예외를 상속하지 않은 이유를 생각해보면
해당 RoomescapeException 예외는 사실상 RuntimeException처럼 큰 예외로군요 😃

jamie9504 · 2024-05-08T06:19:37Z

src/main/java/roomescape/repository/ThemeRepository.java

+import java.util.Optional;
+import roomescape.domain.Theme;
+
+public interface ThemeRepository {


궁금한 부분이, 인터페이스를 사용해야만 테스트용 구현체를 생성할 수 있나요?
상속이라던가를 이용하는 방법도 있?지 않나요

jamie9504 · 2024-05-08T06:21:19Z

src/main/java/roomescape/controller/ReservationController.java

+    }
+
+    @GetMapping
+    public List<ReservationResponse> findAllReservations() {


여기서는 ResponseEntity를 사용하지 않은 이유는 무엇인가요?

jamie9504 · 2024-05-08T06:22:14Z

src/main/java/roomescape/domain/Reservation.java

+        return this.id == id;
+    }
+
+    public boolean isReservationTimeOf(long id) {


사용되는 메서드들인가요?

jamie9504 · 2024-05-08T06:23:35Z

src/main/java/roomescape/exception/ExceptionType.java

+import org.springframework.http.HttpStatus;
+
+public enum ExceptionType {
+    EMPTY_NAME(BAD_REQUEST, "이름은 필수 값입니다."),


ExceptionType으로 분기해서 사용하는 것과, 같은 성격을 CustomException으로 묶는 것은 어떠한 차이가 있을까요?

robinjoon and others added 30 commits April 30, 2024 14:30

feat: 이전 미션 코드 가져오기

64335cd

docs: 요구사항 변경에 따른 README.md 수정

9d575fe

fix: 시간 생성 API 응답 변경

258c9b6

- 상태코드를 200 -> 201 변경 - Location 헤더 추가 Co-authored-by: zangsu <[email protected]>

fix: 예약 생성 API 응답 변경

dbb1b8b

- 상태코드를 200 -> 201 변경 - Location 헤더 추가 Co-authored-by: zangsu <[email protected]>

fix: 예약, 예약 시간 삭제 API 응답 변경

202216d

- 상태코드를 200 -> 204 변경 Co-authored-by: zangsu <[email protected]>

fix: 누락된 설정 파일 추가

bae62c9

Co-authored-by: zangsu <[email protected]>

fix: hh:MM 형식이 아닌 시간 생성 요청시 예외 처리

fc59282

Co-authored-by: zangsu <[email protected]>

refactor : Repository 가 DTO를 사용하지 않도록 변경

65733d0

Co-authored-by: zangsu <[email protected]>

feat : ReservationTime 의 startAt 에 대한 null 검증 추가 및 존재하지 않는 시간에 예약 생성 …

1bbadab

…시 에러 발생 구현 Co-authored-by: zangsu <[email protected]>

feat : 중복된 예약 시간 생성 요청 시 에러 구현

a14d390

Co-authored-by: zangsu <[email protected]>

feat : 예약이 있는 예약 시간을 삭제 요청 시 에러 구현

2da2b7a

Co-authored-by: zangsu <[email protected]>

docs : 존재하지 않는 자원을 삭제하는 경우 예외처리 삭제

4f222f4

- 어차피 자원이 존재하지 않는 것은 동일하므로 문제가 없다고 결정 Co-authored-by: zangsu <[email protected]>

feat: 동일한 날짜와 시간에 예약 생성 요청 시 에러 구현

6a4ebb7

Co-authored-by: zangsu <[email protected]>

feat: ISO 8601 표준에 따른 YYYY-MM-dd 포맷에 해당하지 않는 날짜가 포함된 예약 생성 요청 시 에러

60e2f88

구현 Co-authored-by: zangsu <[email protected]>

feat: 지나간 날짜와 시간의 예약 요청 시 에러 구현

80b2371

Co-authored-by: zangsu <[email protected]>

feat: 이름이 비어있는 예약 요청 시 에러 구현

a5ca6d4

Co-authored-by: zangsu <[email protected]>

refactor: 커스텀 예외 도입

36fe37e

Co-authored-by: zangsu <[email protected]>

docs: api 명세 수정

3e37f25

Co-authored-by: zangsu <[email protected]>

docs: api 명세 추가

9d83ffe

Co-authored-by: zangsu <[email protected]>

docs: 기능 목록 추가

6f1be1a

Co-authored-by: zangsu <[email protected]>

feat: 변경된 페이지로 반환하도록 수정

c75c485

Co-authored-by: zangsu <[email protected]>

feat: 테마 CRD 구현

4d4c985

- 테마 이름, 설명, 썸네일 이미자가 비어 있을 경우 에러 - 중복된 이름의 테마 생성 요청시 에러 Co-authored-by: zangsu <[email protected]>

feat: 기존 에약 API 에 테마 추가

44c1974

Co-authored-by: zangsu <[email protected]>

docs: 누락된 문서 작성

af7f7e6

- 이미 구현된 기능의 체크 리스트 누락된 것 작성 Co-authored-by: zangsu <[email protected]>

docs: 이미 해결한 Todo 삭제

31df8bd

Co-authored-by: zangsu <[email protected]>

docs: 이미 해결한 Todo 삭제

564ecaf

Co-authored-by: zangsu <[email protected]>

fix: equals 메서드가 id만으로 비교하도록 수정

550f4cd

Co-authored-by: zangsu <[email protected]>

fix: 중복 예약 검증 로직이 테마도 확인하도록 변경

d1c9bee

Co-authored-by: zangsu <[email protected]>

docs: 사용자 예약 기능 및 API 명세 추가

1cfec9a

Co-authored-by: zangsu <[email protected]>

feat: 사용자 예약 기능 구현

37c3bc1

Co-authored-by: zangsu <[email protected]>

robinjoon added 10 commits May 2, 2024 16:10

docs: "//TODO : 테스트 생성" 삭제

1378a92

- 이미 작성함.

docs: "//Todo 테스트코드 작성" 삭제

093f68b

- Controller 는 서비스의 코드를 그대로 호출하므로 테스트 작성이 필요하지 않다고 판단 - 단, 기존의 잘 동작하는 테스트 코드를 삭제하지는 않음

docs: "todo [로빈] 이 테스트를 작성하다 보니 Response dto 에도 NotNull 검증은 필요할 것 같다는…

1024c7e

… 생각이 듭니다." 삭제 - 각자 추구하는 방향이 달라 페어 프로그래밍이 끝난 후 각자 개선하는 것으로 합의

docs: "TODO : 도메인에게 넘길 수 있을 것 같은데" 삭제

258466c

- 도메인으로 이동할 수 있는 책임을 이동

docs: "Todo : [로빈] Response 가 변환 로직을 가지고 있으면 아래 코드도 간단해 질 것 같음" 삭제

8c4efbc

- 각자 추구하는 방향이 달라 페어 프로그래밍이 끝난 후 각자 개선하는 것으로 합의

refactor : 도메인으로 이동할 수 있는 책임을 이동

c7c5033

docs: "Todo 상태코드 고민해보기"

0391803

- 4~6 단계 인증 인가가 들어갈 때 고민이 해소 될 것이라 판단

refactor: getter 대신 도메인 로직을 사용하도록 변경

c7f2dad

docs: 해결된 todo 삭제

2416896

style: 컨벤션 적용

e8e7a6f

robinjoon commented May 2, 2024

View reviewed changes

robinjoon mentioned this pull request May 2, 2024

로빈 1단계 제출 woowacourse-6th-code-review-study/spring-roomescape-member#8

Open

jamie9504 requested changes May 3, 2024

View reviewed changes

robinjoon added 11 commits May 4, 2024 16:00

docs: 누락된 체크 표시 추가

62cef24

docs: 리팩토링 대상 목록 정리

1561f7e

refactor: RowMapper 구현 클래스 추가 및 sql 스타일 통일

c9b2f37

refactor: Stream 대신 SQL을 사용하도록 수정

92e8519

style: 길이가 긴 SQL 문을 텍스트 블록으로 표현

a1ed68a

test: 인기 테마 조회 기능 테스트 추가

5d02747

test: 누락된 Repository 테스트 추가

eb38c7d

docs: 리팩토링 목록 수정

9881fc7

feat: 테마의 thumbnail 이 url 형태인지 검증하는 기능 추가

239c241

refactor: 정적 페이지를 반환하는 컨트롤러 이름 수정

20fddde

refactor: 예약 가능한 시간 조회 API 명세 수정

e12b17e

jamie9504 assigned robinjoon and jamie9504 May 8, 2024

jamie9504 approved these changes May 8, 2024

View reviewed changes

jamie9504 merged commit b43e229 into woowacourse:robinjoon May 8, 2024

robinjoon mentioned this pull request May 10, 2024

[4 - 6단계 방탈출 사용자 예약] 로빈(임수빈) 미션 제출합니다. #96

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1 - 3 단계 방탈출 사용자 예약] 로빈(임수빈) 미션 제출합니다. #29

[1 - 3 단계 방탈출 사용자 예약] 로빈(임수빈) 미션 제출합니다. #29

robinjoon commented May 2, 2024 •

edited

Loading

robinjoon May 2, 2024

jamie9504 May 3, 2024

robinjoon May 3, 2024

jamie9504 May 8, 2024

jamie9504 left a comment

jamie9504 May 3, 2024

jamie9504 May 3, 2024

jamie9504 May 3, 2024

jamie9504 May 3, 2024

robinjoon May 3, 2024

jamie9504 May 8, 2024

jamie9504 May 3, 2024

robinjoon May 5, 2024

jamie9504 May 8, 2024

jamie9504 May 3, 2024

robinjoon May 4, 2024

jamie9504 May 3, 2024

robinjoon May 4, 2024

jamie9504 May 3, 2024

robinjoon May 5, 2024

jamie9504 May 8, 2024

robinjoon commented May 7, 2024

jamie9504 left a comment

jamie9504 May 8, 2024

jamie9504 May 8, 2024

jamie9504 May 8, 2024

jamie9504 May 8, 2024

jamie9504 May 8, 2024

jamie9504 May 8, 2024

jamie9504 May 8, 2024

		@@ -0,0 +1,248 @@
		# 요구사항 문서

		- [x] API 명세를 현재 프론트엔드 코드가 잘 동작할 수 있도록 수정


		import org.springframework.http.HttpStatus;

		public class RoomescapeException extends RuntimeException {


		import java.time.LocalDate;

		public record ReservationRequest(LocalDate date, String name, long timeId, long themeId) {

[1 - 3 단계 방탈출 사용자 예약] 로빈(임수빈) 미션 제출합니다. #29

[1 - 3 단계 방탈출 사용자 예약] 로빈(임수빈) 미션 제출합니다. #29

Conversation

robinjoon commented May 2, 2024 • edited Loading

개인적인 사정으로 인한 시간 제한

SQL 대신 Java Stream 활용

페어 프로그래밍 방식 변경

스프링 관련 경험

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jamie9504 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

커스텀 예외가 유용한가?

라이브러리나 프레임워크에서 투척하는 예외와 구분하기 용이하다.

Checked 예외 vs Unchecked 예외

Checked 예외의 단점

캡슐화를 해친다.

Java는 물론 최신 개발 트랜드와 맞지 않는다.

RuntimeException하위의 구체적인 예외를 상속하지 않은 이유

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DTO에 검증 로직을 추가했을 때 장점

빠르게 실패할 수 있다.

DTO에 검증 로직을 추가했을 때 단점

검증 로직이 여기 저기 흩어진다.

중복된 검증이 발생할 수 있다.

중복된 검증을 피하는 과정에서 숨겨진 결합이 발생할 수 있다.

현재 상황에서 장단점 간 비교

Choose a reason for hiding this comment

Choose a reason for hiding this comment

장점

데이터베이스와 독립된 테스트를 작성할 수 있게 되었다.

단점

인터페이스에 메서드가 추가될 때 해야 하는 작업이 많아진다.

장점이 더 크다고 판단한 이유

Choose a reason for hiding this comment

robinjoon commented May 7, 2024

리팩토링 목록

변경된 기능 목록

jamie9504 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

robinjoon commented May 2, 2024 •

edited

Loading

`RuntimeException`하위의 구체적인 예외를 상속하지 않은 이유