Step2 아토 #2

hyxrxn · 2024-04-23T09:34:23Z

리팩토링은 하나도... 안되어 있습니다
기능 구현만 되어있어요

왜 이렇게 했느냐?
라고 물어본다면
학습 테스트에 그렇게 되어있어서...
라고 대답할 수밖에 없는 상태입니다... 대부분요.
리팩토링 추천 부분! 위주로 코멘트 주시면 감사하겠습니다

다들 홧팅

* feat(RoomEscapeController): 홈 화면 생성 Co-authored-by: hyxrxn <[email protected]> * feat(RoomEscapeController): welcome page 리다이렉트 Co-authored-by: hyxrxn <[email protected]> * feat(RoomEscapeController): 예약 내역 조회 Co-authored-by: hyxrxn <[email protected]> * refactor(ReservationDto): 예약 정보 전달 Co-authored-by: hyxrxn <[email protected]> * style(RoomEscapeApplication): 불필요한 개행 삭제 Co-authored-by: hyxrxn <[email protected]> * feat(RoomEscapeController): 예약 추가 Co-authored-by: hyxrxn <[email protected]> * feat(RoomEscapeController): 예약 삭제 Co-authored-by: hyxrxn <[email protected]> * style(RoomEscapeController): 개행 추가 Co-authored-by: hyxrxn <[email protected]> * refactor(ReservationResponse): 클래스명 변경 Co-authored-by: hyxrxn <[email protected]> * fix(index.html): 페이지 이동 불가 버그 개선 Co-authored-by: hyxrxn <[email protected]> * refactor(controller): 컨트롤러 분리 Co-authored-by: hyxrxn <[email protected]> * style(controller): 디폴트값 명시 Co-authored-by: hyxrxn <[email protected]> * refactor(ReservationRequest): LocalDateTime 변환 로직 수정 Co-authored-by: hyxrxn <[email protected]> * style(import): import문 순서 컨벤션 적용 Co-authored-by: hyxrxn <[email protected]> * refactor(Reservation): 불필요한 생성자 제거 * refactor(ReservationRequest): 래퍼 클래스 이용하게 통일 --------- Co-authored-by: Arachneee <[email protected]>

- 예약 추가 / 취소 기능 구현 필요

- 예약 추가 시 해당 예약 바디로 리턴 - reservation-legacy.js에서 상태코드 수정

robinjoon

아토 화이팅!!

robinjoon · 2024-04-24T05:26:28Z

src/test/java/roomescape/MissionStepTest.java


 @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)
 @DirtiesContext(classMode = DirtiesContext.ClassMode.BEFORE_EACH_TEST_METHOD)
-public class MissionStepTest {
+class MissionStepTest {


이 테스트 코드는 End to End 테스트라고 볼 수 있어요.
단위 테스트나 통합 테스트가 작성될 필요가 있겠네요.

robinjoon · 2024-04-24T05:29:58Z

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

+    public Reservation toReservation(Long id) {
+        return new Reservation(id, name, LocalDateTime.of(date, time));
+    }


Request DTO가 도메인을 아는 구조네요.

사용하지 않는 메서드더라구요.... 삭제해버렸습니다

robinjoon · 2024-04-24T05:32:40Z

src/main/java/roomescape/dto/ReservationResponse.java

+        String date,
+        String time


타입을 String으로 정의한건 그냥 LMS 따라한건가요? 각각 날짜와 시간에 특화된 타입을 사용하는 것은 어떨까요?

따라한거 맞습니다...!
특화된 타입을 사용하면 뭐가 좋나요??
저는 어차피 body에 전달되는 모양이 똑같을거라고 생각해서 이대로 뒀습니다

robinjoon · 2024-04-24T05:33:54Z

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

+        this.jdbcTemplate = jdbcTemplate;
+    }
+
+    @GetMapping("")


이 경우에는 ("") 를 아예 생략하고 @GetMapping 이라 할 수 있어요!

오오 대박 이유를 설명할 게 있긴 있었네요

이건 일부러 붙였습니다!
다 붙이는 것을 컨벤션으로 정하면 실수를 줄일 수 있을 것 같아서 페어와 합의를 했습니다
실수로 빼먹고 안 붙인건지 아니면 없어도 돼서 생략한건지 구분이 되니까요... ㅎㅎ

robinjoon · 2024-04-24T05:36:01Z

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

+                (resultSet, rowNum) ->
+                        new Reservation(
+                                resultSet.getLong("id"),
+                                resultSet.getString("name"),
+                                LocalDateTime.of(
+                                        resultSet.getDate("date").toLocalDate(),
+                                        resultSet.getTime("time").toLocalTime()
+                                )
+                        )


RowMapper를 따로 클래스로 빼는건 어떨까요?

robinjoon · 2024-04-24T05:38:09Z

src/test/java/roomescape/MissionStepTest.java

+class MissionStepTest {
+
+    @Autowired
+    private JdbcTemplate jdbcTemplate;

    @Test
    void 일단계() {


메서드 이름을 바꿔보는건 어떨까요?

kelly6bf

안녕하세요 아토! 아무래도 미션 크기가 작다 보니 리뷰 드릴 내용이 별로 없네요..! 👀 Spring 학습이 이번이 처음이라고 들었는데 요구사항들을 잘 구현하고 있어서 대단하다는 생각이 들었습니다! (코딩은 역시 뇌지컬이구나..)

PR에 작성하신 대로 최대한 개선해 보면 좋을 부분들에 대해서 리뷰를 작성해 봤어요! 혹시 리뷰 내용 중 이해되지 않거나 잘못된 부분이 있으면 언제든 질문 주세요! 앞으로의 미션도 파이팅!!

PS. 시간에 쫓겨서 리뷰를 작성하다 보니 띄어쓰기가 좀 개판입니당.. 다음번에 리뷰를 또 하게 된다면 그때는 맞춤법 검사기도 한 번 돌라셔 올릴게요! 가독성이 많이 떨어지셨다면 죄송.. 😢 (그래도 이 코멘트는 돌렸습니다..ㅎ)

kelly6bf · 2024-04-24T12:52:22Z

src/main/java/roomescape/controller/AdminController.java

+@Controller
+public class AdminController {
+
+    @GetMapping("")


@RequestMapping에 설정된 Base URL을 그대로 요청한다면 다음과 같이 ""를 제거하고 작성할 수도 있습니다!

@GetMapping public String admin() { return "admin/index"; }

로빈에게 답변한 것을 복붙하겠습니닷

#2 (comment)

오오 대박 이유를 설명할 게 있긴 있었네요

이건 일부러 붙였습니다!
다 붙이는 것을 컨벤션으로 정하면 실수를 줄일 수 있을 것 같아서 페어와 합의를 했습니다
실수로 빼먹고 안 붙인건지 아니면 없어도 돼서 생략한건지 구분이 되니까요... ㅎㅎ

kelly6bf · 2024-04-24T12:54:18Z

src/main/java/roomescape/controller/HomeController.java

+@Controller
+public class HomeController {
+
+    @GetMapping("")


역시 마찬가지로 ""를 제거하고 사용할 수 있어요! 😋

@GetMapping public String index() { return "redirect:/admin"; }

kelly6bf · 2024-04-24T13:00:04Z

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

+
+    private JdbcTemplate jdbcTemplate;
+
+    public ReservationController(JdbcTemplate jdbcTemplate) {


취향차이라고 생각하지만 생성자 파라미터에 final을 붙이는것에 대해서 어떻게 생각하시나요!

IntelliJ는 생성자 자동 생성과 함께 final키워드도 자동으로 붙여주는 설정도 제공하는걸요? 🫣
https://jaehhh.tistory.com/45

어떤 차이가 있는지 공부해본적이 없어서 생각을 못했네요

차이 먼저 공부해보겠습니다
ㅋㅋㅋㅋㅋㅋ

kelly6bf · 2024-04-24T13:10:59Z

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

+        return ResponseEntity.noContent().build();
+    }
+
+    private List<Reservation> getReservations() {


Controller에서 직접 DB에 접근해서 데이터를 조회하고있네요! 아직은 작성할 쿼리의 복잡도가 크지않아서 컨트롤러가 직접 DB에서 데이터를 가져오는 방식에 크게 문제가 없어 보이지만, 만약 쿼리가 더 복잡해진다면? 혹은 수정사항이 생긴다면? 아토의 코드를 처음 접한 동료들은 자연스럽게 어떤 클래스를 먼저 확인해볼까요? 🤔

사실 아토는 이미 Layered architecture가 무엇인지, DB에 직접 쿼리를 전송하고 결과 데이터를 받아오는 작업을 컨트롤러보다 더 잘할 수 있는 친구가 누군인지 알고 있을거라 생각해요! (Re..로 시작하는 친구였나?)

컨트롤러가 많은 것들을 책임지는 코드를 작성해봤으니 이번에는 분리하는 코드를 작성해보는건 어떨까요? 🤭

추가로 컨트롤러가 DB에 직접 접근하는등 많은 역할을 가지고 있는 구조를 Smaprt UI Pattern이라고 불러요! 무조건 Layered architecture로 나누는건 좋고, Smart UI Pattern을 따르는건 좋지않고 보다는 직접 두 구조의 장단점을 찾아보면서 기준을 세워본다면 앞으로의 미션에서도 더 수월하지 않을까 생각해요! ☺️

https://dev.to/ielgohary/layered-architecture-and-smart-ui-domain-driven-design-by-eric-evans-part-ii-chapter-4-1il7

kelly6bf · 2024-04-24T13:19:43Z

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

+        KeyHolder keyHolder = new GeneratedKeyHolder();
+        jdbcTemplate.update(connection -> {
+            PreparedStatement ps = connection.prepareStatement(
+                    "insert into reservation (name, date, time) values (?, ?, ?)",


JdbcTemplate을 사용해 쿼리를 작성할때 동적인 파라미터를 설정하기 위해서는 ?을 기준으로 순서에 맞춰 파라미터를 설정해야하죠! 물론 지금처럼 파라미터 개수가 적고 간단한 쿼리라면 지금 방식도 크게 나쁘지 않지만.. 파라미터가 점점 늘어난다면..? 순서 유지를 하는것도 꽤 골치아플 수 있어요!

이런 문제를 먼저 겪은 선배 개발자분들은 NamedParameterJdbcTemplate라는 기술을 내놓으셨습니다! 이번 기회에 찾아보고 적용해보면 팍팍한 우테코 미션이 아주 조금은 개선될지도...? 👀

공식 문서 : https://docs.spring.io/spring-framework/reference/data-access/jdbc/core.html#jdbc-NamedParameterJdbcTemplate
친절한 한국 블로그 : https://code-lab1.tistory.com/278

안그래도 이번에 코드리뷰를 하며 알게 된 기능이네요...!
링크 첨부까지,,, 너무 감사합니닷🥹

kelly6bf · 2024-04-24T13:35:51Z

src/test/java/roomescape/MissionStepTest.java

 import org.springframework.test.annotation.DirtiesContext;
+import roomescape.dto.ReservationResponse;

 @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.DEFINED_PORT)


@SpringBootTest의 SpringBootTest.WebEnvironment.DEFINED_PORT는 어떤 설정일까요? 만약 애플리케이션을 실행한 상태에서 테스트를 수행한다면 어떤일이 발생할까요? 🤔

물론 애플리케이션 끄고 테스트하면 그만 아님?ㅎ라고 한다면 할말이 없지만! 그래도 테스트를 수행할 때 마다 애플리케이션을 끄고키는건 너무 번거로울거 같아요 😢 애플리케이션 실행과 관련없이 @SpringBootTest가 설정된 테스트를 수행할 방법은 없을까요?!

RANDOM_PORT : https://docs.spring.io/spring-boot/reference/testing/spring-boot-applications.html#testing.spring-boot-applications.with-running-server

PS. RANDOM_PORT로 설정했는데.. 에러가 난다고요? 뭐 연결이 안되서 뭐시기 한다고요? 왜 그런 문제가 발생할까요... 어떻게 해결하지 👀
https://stackoverflow.com/questions/40665315/testing-spring-boot-rest-application-with-restassured

- 기존 구조 활용

- 변수 추출 - 상수 분리 - SimpleJdbcInsert 사용

- 존재한다면 예외 발생 - 존재하지 않는다면 정상 삭제

- 조회 개수가 삭제 이후 1개에서 0개가 되었는지 확인

hyxrxn and others added 5 commits April 18, 2024 00:24

feat: 데이터베이스 연동

22e85c6

refactor(ReservationController): 예약 내역 조회 시 데이터베이스 활용

8d5e7fa

- 예약 추가 / 취소 기능 구현 필요

refactor(ReservationController): 예약 내역 추가 및 삭제 시 데이터베이스 활용

114446c

fix(ReservationController): 예약 추가 및 삭제 시 페이지 미갱신 문제 수정

b3f2d74

- 예약 추가 시 해당 예약 바디로 리턴 - reservation-legacy.js에서 상태코드 수정

robinjoon reviewed Apr 24, 2024

View reviewed changes

kelly6bf requested changes Apr 24, 2024

View reviewed changes

hyxrxn added 22 commits April 25, 2024 16:08

feat(AdminController): 시간 관리 페이지 응답

b97eba1

fix: html 네비게이션 바 버튼 링크 수정

d027c6c

feat(schema.sql): 예약 시간 스키마 추가

c0d56ce

feat(TimeController): 예약 시간 조회, 추가 및 삭제

6fb9f5b

- 기존 구조 활용

refactor(ReservationController): api 명세에 맞춰 상태 코드 수정

98ae4d0

feat(AdminController): 예약 페이지 응답

a30029b

feat(ReservationController): 시간을 테이블에서 선택해서 예약 추가, 삭제 및 조회

ae0c874

refactor: 패키지 분리

eb2777a

refactor(Controller): 가독성 개선

b2be619

- 변수 추출 - 상수 분리 - SimpleJdbcInsert 사용

feat(ReservationTimeDao): reservation_time 테이블 접근 책임 분리

08e273a

feat(ReservationDao): reservation 테이블 접근 책임 분리

78952eb

refactor(ReservationTimeDao): dto 대신 도메인을 파라미터로 받도록 변경

d02fd4b

refactor(ReservationDao): dto 대신 도메인을 파라미터로 받도록 변경

1ff3c24

test: DynamicTest 활용해 연속적인 테스트 진행

537ae1c

refactor: 필드가 불변하도록 변경

c914a99

refactor(MissionStepTest): 테스트 이름 설정

a8d9f06

feat(TimeController): body 유효성 검증

cc2ac8d

feat(ReservationController): body 유효성 검증

31c3bb6

refactor(TimeController): 해당 시간의 예약이 존재하는지 확인 후 삭제

a5136cb

- 존재한다면 예외 발생 - 존재하지 않는다면 정상 삭제

feat(TimeService): 예약 시간 관련 비즈니스 플로우 책임 분리

bbb56ad

feat(ReservationService): 예약 관련 비즈니스 플로우 책임 분리

724c60c

test(EndToEndTest): 삭제가 잘 되었는지 확인

5a1fa50

- 조회 개수가 삭제 이후 1개에서 0개가 되었는지 확인

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Step2 아토 #2

Step2 아토 #2

hyxrxn commented Apr 23, 2024 •

edited

Loading

robinjoon left a comment

robinjoon Apr 24, 2024

robinjoon Apr 24, 2024

hyxrxn Apr 26, 2024

robinjoon Apr 24, 2024

hyxrxn Apr 26, 2024

robinjoon Apr 24, 2024

hyxrxn Apr 25, 2024

robinjoon Apr 24, 2024

robinjoon Apr 24, 2024

kelly6bf left a comment

kelly6bf Apr 24, 2024

hyxrxn Apr 25, 2024

kelly6bf Apr 24, 2024

kelly6bf Apr 24, 2024

hyxrxn Apr 25, 2024

kelly6bf Apr 24, 2024

kelly6bf Apr 24, 2024

hyxrxn Apr 25, 2024

kelly6bf Apr 24, 2024


		private JdbcTemplate jdbcTemplate;

		public ReservationController(JdbcTemplate jdbcTemplate) {

Step2 아토 #2

Are you sure you want to change the base?

Step2 아토 #2

Conversation

hyxrxn commented Apr 23, 2024 • edited Loading

robinjoon 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

Choose a reason for hiding this comment

kelly6bf 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

Choose a reason for hiding this comment

hyxrxn commented Apr 23, 2024 •

edited

Loading