[#6] 좌석 API #8
[#6] 좌석 API #8
Conversation
hyeonsunny
force-pushed
the
feature/6
branch
2 times, most recently
from
f6a3a6b to
a6cd289
Compare
hyeonsunny
force-pushed
the
feature/6
branch
from
a6cd289 to
adbdf97
Compare
| * @param playTimeId n회차의 공연시간 id | ||
| * @return | ||
| */ | ||
| @GetMapping("/play/order/{playTimeId}") |
There was a problem hiding this comment.
컨트롤러 위에 RequestMapping을 붙히면 중복부분을 제거할 수 있습니다.
@RestController
@Slf4j
@RequestMapping("/play")
public class PlayController {
@GetMapping("/order/{playTimeId}")
}
There was a problem hiding this comment.
@RequestMapping 로 중복 제거했습니다.
There was a problem hiding this comment.
api를 제공하는데
클라이언트 1곳 이면 뭐.. 그냥저냥
인터파크 에서 만들었는데
네이버, 카카오나 이런데도 제공을 해야한다.
/play/first /play , /play/0 ....
/play/seccond ...
범용 api
/play
| public PlayAtDTO playAtList(@RequestParam String goodsId, @RequestParam String startDate, @RequestParam String endDate) { | ||
| List<String> list = playService.findPlayAtList(goodsId, startDate, endDate); | ||
|
|
||
| PlayAtDTO dto = new PlayAtDTO(); |
There was a problem hiding this comment.
Dto에 setter 다 빼주세요.
immutable하지 않고 언제든 값이 바뀔 수 있는 여지를 줍니다.
생성자로 받는 방식을 사용하고, setter 말고 왠만하면 custom하게 메서드를 작성해주세요
There was a problem hiding this comment.
setter 제거 후
아래 코멘트 주신 내용 중 ‘PlayAtDTO 안에 playAt 리스트’ 에 고민하다보니
List 자료형의 playAt 리스트가 필요했던거라
불필요한 DTO라 판단되어 삭제하였습니다.
| * @return Object | ||
| */ | ||
| @GetMapping("/play/first") | ||
| public SeatDTO firstPlayOrderInfo(@RequestParam String goodsId, @RequestParam String playAt) { |
There was a problem hiding this comment.
playAt 이 String이면 이걸 날짜로 또 변환해줘야 하지 않을까요?
대안이 어떤게 있는지 찾아봐주세요
There was a problem hiding this comment.
여러 포맷이 있고 포맷 때문에 파라미터 요청이 틀릴 가능성과
다른 개발자들 및 시간이 흘른 뒤 보아도 playAt만 보고도 명확히 알기 위해
데이터베이스에서 play_at의 데이터타입을 ‘Date’로
코드단에서는 @DateTimeFormat를 활용하여 ‘yyyyMMdd’ 형식 데이터를 자동으로
LocalDate 타입으로 변환하도록 수정하였습니다.
| @Getter | ||
| @Setter | ||
| @ToString | ||
| public class PlayAtDTO { |
There was a problem hiding this comment.
PlayAtDTO 안에 playAt 리스트가 있는건 좀 의아한데요
다른 DTO로 네이밍하는게 좋지 않을까요? 해석이 안됩니다
There was a problem hiding this comment.
playAt리스트만 필요하고 전달하는 DTO라
PlayAtDTO의 역할과 의미가 모호한 것 같아
조회한 데이터를 List 담아
ApiResponse를 통해 반환하는 형태로 변경하였습니다.
| @Setter | ||
| @ToString | ||
| public class PlayTime { | ||
| private String playAt; |
There was a problem hiding this comment.
String 바꿔주세요.
나중에 전혀 이해할 수 없습니다
There was a problem hiding this comment.
날짜 형태 String 데이터 LocalDate로 모두 변경하였습니다.
|
|
||
| seatDateDto.setTimeTableList(timeTableList); | ||
| seatDateDto.setSeatCountList(seatCountDTOList); | ||
| } catch(Exception e) { |
There was a problem hiding this comment.
e.printStackTrace 호출 시에 어떤 일이 발생하나요?
그리고 실제 서비스에 이런 코드가 나가게 되면 성능적인 측면에서 어떤 단점이 생길지 찾아서 적어주세요
There was a problem hiding this comment.
e.printStackTrace() 은 표준 출력으로 I/O가 발생하기 때문에
디버깅용으로 사용하면 콘솔 출력으로 인한 애플리케이션 성능 저하가 발생합니다.
또한 로그 관리를 할 수 없어 예외를 분석하기에 어려움이 있고
스택 트레이스에 있는 파일 경로과 같은 정보가 노출될 수 있는 보안 문제가 있습니다.
때문에 스택 트레이스가 아닌
로깅 도구를 사용하여 백업 및 모니터링을 통합적으로 관리하고
로깅 레벨을 정의하여 개발과 운영 환경에서 전략적으로 로그를 관리할 수 있습니다.
- RequestMapping 적용 - 데이터 타입 변경 id(String -> long), playAt(String -> LocalDate) - service 단에서 예외처리 책임 부여
| @GetMapping("/list") | ||
| public ResponseEntity<ApiResponse<List<String>>> playAtList( | ||
| @RequestParam long goodsId | ||
| , @RequestParam @DateTimeFormat(pattern = "yyyyMMdd") LocalDate startDate |
|
|
||
| @Override | ||
| public List<String> selectPlayAtList(long goodsId, LocalDate startDate, LocalDate endDate) { | ||
| String sql = """ |
|
|
||
| @Override | ||
| public List<String> findPlayAtList(long goodsId, LocalDate startDate, LocalDate endDate) { | ||
| List<String> list = new ArrayList<>(); |
There was a problem hiding this comment.
음.. 이 아래도 try-catch가 도배가 되어 있습니다
controller advice의 롤로 넘겨주세요
There was a problem hiding this comment.
좌석, 콘텐츠 service단의 try-catch 삭제,
예외 체크 후 exception 던지는 방식으로 변경 및 테스트하였습니다!
|
데이터베이스 설계 공유가 늦었습니다. API 명세 보실 때 필요할 것 같아 함께 링크합니다.