[4기 - 한승원] SpringBoot Part3 Weekly Mission 제출합니다. #831
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
lleellee0
reviewed
Jul 16, 2023
Comment on lines
180
to
186
| List<Voucher> foundVouchers = new ArrayList<>(); | ||
| for (String[] voucher : vouchers) { | ||
| String discountTypeCompared = voucher[VoucherProperty.CUSTOMER_ID.index]; | ||
| if (discountType.equals(discountTypeCompared)) { | ||
| foundVouchers.add(mapToVoucher(voucher)); | ||
| } | ||
| } |
lleellee0
reviewed
Jul 16, 2023
There was a problem hiding this comment.
Restful하게 API가 설계되지 않은 것 같습니다!
다른분들에게 드렸던 피드백 동일하게 드립니다.
API 설계 관련해서는 표준적으로 권장되는 가이드와 여러 서비스들의 API 설계가 어떻게 되어있는지 둘러보시면 좋습니다~
Voucher 관리 서비스는 명확하게 API를 설계할 수 있는 반면 애매한 API를 가지게되는 서비스들도 있어서요.
만약 그런 API를 만들게될 경우 표준적인 설계 방법을 벗어나지 않는 선에서 예성님이 생각하는 기준에 입각하여 API를 설계해보시기 바랍니다.
lleellee0
reviewed
Jul 16, 2023
Comment on lines
+10
to
+39
| @ExceptionHandler(EmptyAssignerException.class) | ||
| protected ResponseEntity<ErrorResponse> handleCustomException(EmptyAssignerException emptyAssignerException) { | ||
| ErrorCode errorCode = emptyAssignerException.getErrorCode(); | ||
| return handleExceptionInternal(errorCode); | ||
| } | ||
|
|
||
| @ExceptionHandler(InvalidDataException.class) | ||
| protected ResponseEntity<ErrorResponse> handleCustomException(InvalidDataException invalidDataException) { | ||
| ErrorCode errorCode = invalidDataException.getErrorCode(); | ||
| return handleExceptionInternal(errorCode); | ||
| } | ||
|
|
||
| @ExceptionHandler(NoSuchDataException.class) | ||
| protected ResponseEntity<ErrorResponse> handleCustomException(NoSuchDataException noSuchDataException) { | ||
| ErrorCode errorCode = noSuchDataException.getErrorCode(); | ||
| return handleExceptionInternal(errorCode); | ||
| } | ||
|
|
||
| @ExceptionHandler(VoucherReassignmentException.class) | ||
| protected ResponseEntity<ErrorResponse> handleCustomException( | ||
| VoucherReassignmentException voucherReassignmentException) { | ||
| ErrorCode errorCode = voucherReassignmentException.getErrorCode(); | ||
| return handleExceptionInternal(errorCode); | ||
| } | ||
|
|
||
| private ResponseEntity<ErrorResponse> handleExceptionInternal(ErrorCode errorCode) { | ||
| return ResponseEntity | ||
| .status(errorCode.getHttpStatus().value()) | ||
| .body(new ErrorResponse(errorCode)); | ||
| } |
There was a problem hiding this comment.
우리가 예외를 계층적으로 나눠주는 이유는, 예외를 구분해주고 싶은 상황에선 구분해서 처리하고 구분해서 처리하고 싶지 않은 경우 상위 예외로 한번에 처리하려는 겁니다.
지금 4가지 예외는 모두 똑같은 방식으로 처리가 되고 있죠?
이렇게 되면 예외를 굳이 나눠서 처리해줄 필요가 없습니다.
|
승원님 수고하셨습니다~ |
lleellee0
approved these changes
Jul 16, 2023
maenguin
reviewed
Jul 19, 2023
Comment on lines
+30
to
+42
| @GetMapping | ||
| public ResponseEntity<List<VoucherResponseDTO>> find( | ||
| @RequestParam(required = false) String type, | ||
| @RequestParam(required = false, defaultValue = "-1") Long id | ||
| ) { | ||
| if (id > 0) { | ||
| return findById(id); | ||
| } | ||
| if (type != null) { | ||
| return findByType(type); | ||
| } | ||
| return findAll(); | ||
| } |
There was a problem hiding this comment.
동적 쿼리를 구현하시려고 노력한 흔적이 보이네요
이미 아실 수도 있지만 QueryDSL을 사용하면 동적쿼리를 쉽게 처리할 수 있습니다.
좀더 검색에 대한 니즈가 강해서 고도화할 필요가 있으면 엘라스틱 서치를 사용하게 될꺼구요
maenguin
reviewed
Jul 19, 2023
Comment on lines
+49
to
+52
| private ResponseEntity<List<VoucherResponseDTO>> findById(@PathVariable long id) { | ||
| VoucherResponseDTO response = voucherService.findById(id); | ||
| return ResponseEntity.ok(List.of(response)); | ||
| } |
There was a problem hiding this comment.
ResponseEntity를 사용해서 응답을 제어하는 부분이 딱히 없다면 List 그대로 반환해도 동일한 코드가 될것 같네요
maenguin
reviewed
Jul 19, 2023
| return ResponseEntity.ok(response); | ||
| } | ||
|
|
||
| @PostMapping |
maenguin
approved these changes
Jul 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
📌 과제 설명
바우처 관리 애플리케이션
👩💻 요구 사항과 구현 내용
(기본) 바우처 서비스 관리페이지 개발하기
(기본) 바우처 서비스의 API 개발하기
✅ 피드백 반영사항
이전PR에 남겨주신 내용들까지 반영했습니다!