-
Notifications
You must be signed in to change notification settings - Fork 1
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
[feat] 여행후기 기본 구현 완료 #39
Conversation
- TripRecord 엔티티 추가 - TripRecordSchedule 추가
- 해당 컨트롤러 내용 security에 제외되도록 추가
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.
기본 틀 구현을 잘하신 것 같습니다.
수고 많으셨습니다!
import org.springframework.web.bind.annotation.RestController; | ||
|
||
@RestController | ||
@RequestMapping("/v1/trip-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.
URL에 도메인을 어떻게 표시할지는 추후에 논의해서 통일하면 좋을듯 합니다 :)
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.
원래는 restful 기준에 따르면 소문자와 - _ 이 두개로 표현하는게 정석으로 알고 있습니다.
private final TripRecordService tripRecordService; | ||
|
||
@PostMapping | ||
public ResponseEntity<ResponseDTO<TripRecordResponseDto>> tripRecordAdd( |
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.
컨트롤러의 메소드에선 서비스 계층의 메소드와 다르게 동사를 뒤에 배치한 이유가 있나요?
일반적으로 메소드명은 동사가 앞에 나오는 것으로 이해하고 있어서요.
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.
구분을 위해서 그렇게 했습니다.
사람에 따라서는 컨트롤러와 서비스 단 구분을 위해서 함수명 단어를 아예 다르게 하시는 분도 있는 모양입니다.
제가 알기로는 이 부분은 팀내 약속과 개인의 취향의 영역이라고 알고있는데, 대표적인 컨벤션 같은게 있나요?
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.
아하 그렇군요! 제가 알기로도 정해진 건 없다고 들었습니다! 메소드명까지 굳이 통일할 필요가 없다면 재량껏 사용해도 될듯 해요. 다른 분들은 어떻게 생각하시는지 궁금하네요 ㅎㅎ
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.
처음 ExpenseType을 봤을 때 지출 유형이라고 이해했어요. 지출 유형은 카드, 현금, 계좌이체 등을 먼저 떠올릴 수 있으니,
지출 범위라는 의미로 ExpenseRange가 어떤지 제안 드립니다 :)
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.
고려해보겠습니다.
private String title; | ||
private String content; | ||
|
||
private ExpenseType expenseType; |
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.
@Enumerated 애노테이션이 빠진 것 같아요
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.
추가하겠습니다.
private LocalDate tripEndDay; | ||
|
||
private Integer totalDays; | ||
private Integer average_rating; |
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.
변수명에 카멜 케이스가 아닌 스네이크 케이스를 사용하신 이유가 있나요?
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.
이건 실수 같네요 수정하겠습니다.
|
||
|
||
@OneToMany(mappedBy = "tripRecord", fetch = FetchType.LAZY, cascade = CascadeType.REMOVE) | ||
private List<TripRecordSchedule> tripRecordSchedules; |
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.
김영한님 말씀으론 관례적으로 바로 ArrayList를 만든다고 하네요.
private List<TripRecordSchedule> tripRecordSchedules = new ArrayList<>();
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.
네 적용하겠습니다
|
||
private final TripRecordRepository tripRecordRepository; | ||
|
||
@Transactional |
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.
지난번 이야기 했던 것 같은데, @transactional(readOnly=true)을 클래스단에 붙이고 수정하는 메소드에 @transactional을 하는 것보다 메소드마다 따로 표기하는 이유에 대해서 다시 말씀해주실 수 있을까요? 저도 어떻게 적용해야 하는지 고민이 되네요!
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.
우선 저는 트랜잭션 범위를 명확하게 하고, 정확하게 옵션들을 적용하기 위해서 이렇게 하는게 좋다고 제거 어디서 배워서 이렇게 적용하고 있습니다.
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.
그렇군요. 성능적인 부분보다 가독성 차원에서 하는거라면 취향의 영역인것 같네요!
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.
return 문을 보면 한 줄 인라인으로 리팩토링 할 수 있어보이는데, 가독성을 위해서 일부로 변수를 선언하신거죠? 👍
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.
네 맞습니다.
저번 미니 프로젝트 멘토님의 조언입니다. 이건 완전 취향의 영역인거 같아요.
TripRecordResponseDto responseDto = tripRecordService.addTripRecord(requestDto); | ||
ResponseDTO<TripRecordResponseDto> responseBody = ResponseDTO.okWithData(responseDto); | ||
|
||
return ResponseEntity |
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.
등록한 여행 후기 내용 전체를 다시 프론트로 돌려주는 이유가 있나요?
등록된 여행 후기 Id만 반환시키고, 프론트에서 해당 id를 통해 GET 요청을 하도록 하면 되지 않을까 궁금해서 여쭤봅니다.
프론트에서 POST url을 그대로 가지고 있으면 중복 등록이 될 수 있어서 GET으로 리다이렉트 하는 것처럼요!
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.
이건 그냥 습관, 성향 차이로 알고 있습니다.
🎯 목적
새 기능 (New Feature)
리팩토링 (Refactoring)
버그 수정 (Bug Fix)
테스트 (Test)
CI/CD
설정 (Setup)
간략한 설명:
: 여행후기(trip_record)에 대한 기본 CRUD 구현, 여행후기스케쥴(trip_record_schedule) 엔티티 구현
🛠 작성/변경 사항
🔗 관련 이슈
💡 특이 사항
📖 문서화
🧪 테스트
📚 참조
참조 링크
참고 자료