SAPHY-55 feat: 상품 구매 현황 조회 기능 구현 #90
Conversation
| @GetMapping("/{purchaseId}") | ||
| @Operation(summary = "구매 현황 단건 조회 API", description = "사용자의 구매 현황을 단건으로 조회하는 API 입니다.") | ||
| public ApiResponse<PurchaseResponse> findById( | ||
| @AuthenticationPrincipal CustomUserDetails customUserDetails, | ||
| @PathVariable Long purchaseId | ||
| ) { | ||
| Member loggedInmember = customUserDetails.getMember(); | ||
| return new ApiResponse<>(purchaseService.findById(loggedInmember, purchaseId)); | ||
| } |
There was a problem hiding this comment.
단건 조회했을 때 더 자세하게 확인해야 할 내용이 딱히 없는 것 같아서
단건 조회는 나중에 필요없으면 삭제?해도 될 듯
| public static PurchaseStatus findByName(String name) { | ||
| try { | ||
| return PurchaseStatus.valueOf(name.toUpperCase()); | ||
| } catch (IllegalArgumentException e) { | ||
| throw SaphyException.from(ErrorCode.PURCHASE_STATUS_NOT_FOUND); | ||
| } | ||
| } |
There was a problem hiding this comment.
좀 더 명시적으로 적으려면 아래처럼 적을 수도 있긴 한데
public static PurchaseStatus findByName(String name) {
return Arrays.stream(PurchaseStatus.values())
.filter(status -> status.name().equalsIgnoreCase(name))
.findFirst()
.orElseThrow(() -> SaphyException.from(ErrorCode.PURCHASE_STATUS_NOT_FOUND));
}근데 굳이 이미 구현된 valueOf() 메서드를 안 쓰고 이런 식으로 코드를 늘리는 게 별로 좋아보이진 않는 듯 ㅋㅋㅋㅋㅋ 걍 지금 코드 그대로 가도 괜찮을 것 같긴 함
There was a problem hiding this comment.
equalsIgnoreCase로 대소문자 구분 없이 enum 타입 체크하는 방식 좋은 거 같습니다~! 그리고 findFirst()로 null 대신 Optional.empty() 반환하는 방식도 좋은 거 같습니다!(혹시 이거 클린 코드 7장에서 보신건지?!)
There was a problem hiding this comment.
지피티가 알려주긴 했습니다;; ㅋㅋㅋㅋㅋㅋㅋ
| @@ -37,7 +37,15 @@ public List<PurchaseResponse> findAll(Member member) { | |||
| .toList(); | |||
| } | |||
|
|
|||
| public List<PurchaseResponse> findByStatus(Member member, String status) { | |||
There was a problem hiding this comment.
저희 서비스가 status관련 enum이 많아서 purchaseStatus라고 명시적으로 적어주면 더 좋을 거 같아요:)
+메서드도요!
| @@ -37,7 +37,15 @@ public List<PurchaseResponse> findAll(Member member) { | |||
| .toList(); | |||
| } | |||
|
|
|||
| public List<PurchaseResponse> findByStatus(Member member, String status) { | |||
| PurchaseStatus purchaseStatus = PurchaseStatus.findByName(status); // 여기에 오류 발생시키긴 했는데, 뭔가 더 좋은 방법 있을거 같습니다.. | |||
There was a problem hiding this comment.
음.. 일단 여러가지 고민이 들었을 거 같네요. 제 생각도 한번 아래에 정리해볼게요!
- 우선
PurchaseStatus에 대해 Enum타입으로 받을지 String으로 받을지에 대한 고민
String(현재 방식)으로 받아서 예외처리하는 것이 더 좋아보입니다!
지금 저희 코드들도 대부분 이 방식을 따라왔고 유연성이나 예외처리 확장성 측면에서도 이 방법이 좋은 거 같습니다!
- 예외처리를 어느 계층에서 해야할 것인지에 대한 고민
예외처리를 service와 enum 중에서 어디서 할 것인지에 대한 고민 같은데, enum으로 변환하는 과정에서의 오류니까 해당 부분에 두는 현재 방식도 좋아보입니다~!
| public static PurchaseStatus findByName(String name) { | ||
| try { | ||
| return PurchaseStatus.valueOf(name.toUpperCase()); | ||
| } catch (IllegalArgumentException e) { | ||
| throw SaphyException.from(ErrorCode.PURCHASE_STATUS_NOT_FOUND); | ||
| } | ||
| } |
There was a problem hiding this comment.
equalsIgnoreCase로 대소문자 구분 없이 enum 타입 체크하는 방식 좋은 거 같습니다~! 그리고 findFirst()로 null 대신 Optional.empty() 반환하는 방식도 좋은 거 같습니다!(혹시 이거 클린 코드 7장에서 보신건지?!)
📄 Summary
🕰️ Actual Time of Completion
🙋🏻 More