팀장님의 코드리뷰
팀장님이 팀원들의 코드리뷰를 해주셨다.
코드를 짜면서 기존에 맞추지 못했던 코딩 컨벤션도 정의하고 각자 개선시킬 만한 부분들을 정리해주셨다.
간단하게 적어보자면...
- DTO에 안정성을 위해
setter를 제거하고 생성자 어노테이션만 사용하기 - 서비스 단에
Transactional어노테이션 추가 - 변경될 일 없는 인자에
final키워드 붙이기
추가로 내가 구현한 부분에서 기능적으로 개선시켰으면 하는 부분을 말씀해주셨다.
예약페이지에서 날짜와 인원을 선택하고 예약 버튼을 누르면 예약 데이터가 생성되고 결제페이지로 이동한다.
결제페이지에서 결제까지 완료되면 isPaid가 true로 바뀌어서 예약 확정이 된다.
그런데 결제페이지에서 뒤로가기를 한 후에 다시 예약 버튼을 누르면 예약 데이터가 또 생성된다.
기존에 생성되었던 예약 데이터를 다시 사용하면 쓸데없이 데이터가 쌓이는 것을 방지할 수 있다.
이를 해결하기 위해 프론트에서 상태관리를 해주면 좋겠다고 제안해주셨다.
상태관리를 위한 Recoil 라이브러리를 간단한 코드와 함께 설명해주셨고 알려주신 내용을 참고해서 구현해봐야겠다.
고민
코드 리팩토링
코드를 다 올릴 수가 없어서 짧게 적어보자면..
컨트롤러와 서비스의 역할을 생각했을 때 어떤 코드를 어디에 작성하는 게 더 좋을지 고민이 되었다.
체크인/체크아웃 날짜 타입 변경
체크인 날짜와 체크아웃 날짜를 저장하거나 가져올 때 시간은 중요하지 않다.
오히려 시간 정보까지 같이 있으니까 처리하기에 더 까다로운 느낌이다.
LocalDateTime이었던 두 필드를 LocalDate로 바꿔도 될지 고민이다.
오늘 구현한 내용
예약하기 페이지 달력 선택 시 선택 불가능한 날짜 불러오기
예약하기 페이지에서 체크인/체크아웃 날짜 선택 시 이미 예약이 되어있는 날짜들은 선택이 불가능해야 한다.
프론트에서 해당 숙소의 아이디(hotelId)를 url 파라미터로 주는 GET 요청을 보낸다.
여기서 달력이 렌더링 될 때 해당 훅을 사용해서 응답을 받아온다.
백엔드에서는 그 hotelId로 Reservation 테이블의 해당 숙소를 가진 예약 내역을 List로 다 찾는다.
DTO에 예약이 되어있는 날짜들(각 예약 내역의 체크인 날짜부터 체크아웃 날짜까지 전부)을 리스트로 담아서 생성한다.
이 기능을 수행하는 메서드 getDateRange를 어디에 두면 좋을지에 대해 챗지피티와 나눈 대화이다.
Q.
getDateRange를Reservation엔티티 클래스에 정의한 이유를 물어봤다.A.
getDateRange메소드를Reservation클래스 안에 정의한 이유는 객체 지향 프로그래밍의 원칙 중 하나인 "캡슐화(Encapsulation)"와 "책임의 단일성(Single Responsibility Principle)"을 따르기 위함입니다. 이 원칙들은 객체 지향 프로그래밍에서 중요한 개념들로, 코드의 유지보수성과 확장성을 향상시키는 데 기여합니다.
- 캡슐화(Encapsulation):
Reservation클래스는 체크인 날짜(checkInDate)와 체크아웃 날짜(checkOutDate)를 필드로 가지고 있으며, 이 두 날짜 사이의 모든 날짜를 계산하는 것은Reservation객체의 책임입니다.getDateRange메소드를 클래스 내부에 정의함으로써, 이 계산 로직을Reservation객체가 스스로 관리하도록 하여, 데이터와 기능을 하나의 단위로 묶는 캡슐화의 원칙을 따르게 됩니다.- 책임의 단일성(Single Responsibility Principle, SRP): SRP는 한 클래스가 하나의 책임만 가져야 한다는 원칙입니다. 여기서 '책임'은 '변경의 이유'를 의미합니다.
Reservation클래스는 예약에 관련된 데이터(체크인, 체크아웃 날짜 등)를 관리하고, 이와 관련된 동작(날짜 범위 계산 등)을 수행하는 것이 그 책임입니다.getDateRange메소드를Reservation클래스에 포함시킴으로써, 예약과 관련된 모든 책임을 한 곳에 집중시킬 수 있습니다.이러한 방식으로 클래스를 설계하면, 코드가 더 읽기 쉽고, 유지보수하기 쉬우며, 재사용성이 높아집니다. 또한, 나중에
Reservation클래스의 로직이 변경되더라도 해당 로직과 관련된 모든 기능을 한 곳에서 관리할 수 있어 유연하게 대응할 수 있습니다.
Q. DTO에 넣어도 괜찮을지 물어봤다.
A.
getDateRange메소드를 DTO(Data Transfer Object)에 포함시킬지 아니면 기존의Reservation클래스에 두어야 할지 결정하는 것은, 해당 메소드가 수행하는 역할과 DTO의 목적을 고려하여 결정해야 합니다. 각 접근 방식의 장단점을 고려해 보겠습니다.
Reservation클래스에getDateRange를 유지하는 경우:
- 책임의 적절한 할당:
Reservation클래스는 예약과 관련된 로직을 캡슐화합니다. 체크인 날짜부터 체크아웃 날짜 사이의 날짜 범위를 계산하는 것이 예약의 일부로 간주될 수 있으므로,Reservation클래스 내에 이 메소드를 유지하는 것이 적절할 수 있습니다.- 도메인 로직과 데이터 전송의 분리: 일반적으로 DTO는 순수한 데이터 전송을 목적으로 사용됩니다. 비즈니스 로직이나 계산 로직을 DTO에 포함시키는 것은 일반적으로 권장되지 않습니다. DTO는 다른 시스템 또는 계층 간의 데이터 전달에 초점을 맞추어야 합니다.
- DTO에
getDateRange를 포함시키는 경우:
- 특정 요구 사항에 맞춘 데이터 제공: 만약 여러 날짜가 담긴 리스트를 여러 곳에서 반복적으로 필요로 하고, 이 데이터가 특정 요구 사항에 맞춰진 형태라면, DTO 내에
getDateRange메소드를 포함시켜 이 데이터를 쉽게 재사용할 수 있게 하는 것이 효율적일 수 있습니다.- 레이어 간의 명확한 데이터 흐름: DTO를 사용하여 레이어 간 데이터를 전달하고, 각 레이어에서 필요한 데이터 변환 로직을 DTO 내부에 캡슐화함으로써, 시스템의 다른 부분에서는 이 로직을 신경 쓰지 않고 데이터만 사용할 수 있습니다.
- 결론:
Reservation클래스에getDateRange를 유지하는 것이 바람직합니다. 이 방식은 책임의 적절한 할당과 도메인 로직의 캡슐화 측면에서 더 나은 접근입니다.- DTO에 로직을 추가하는 것은 일반적으로 권장되지 않지만, 특정 상황에서 요구 사항에 더 잘 맞을 수 있습니다. 예를 들어, 여러 계층 간에 동일한 데이터 형식을 반복적으로 전달해야 하는 경우 유용할 수 있습니다.
최종 결정은 애플리케이션의 구체적인 요구 사항과 아키텍처에 따라 달라질 수 있으므로, 이러한 측면들을 종합적으로 고려하여 결정하시는 것이 좋습니다.
...챗지피티의 응답에 설득당해서 getDateRange를 Reservation 클래스 내부에 작성하는 것으로 결정했다.
