Skip to content

Comments

[BE] OrderService 삭제 및 chekoutInstant API 통일#302

Merged
soyun-i merged 1 commit intodevelopfrom
be/feat/299
Apr 14, 2025
Merged

[BE] OrderService 삭제 및 chekoutInstant API 통일#302
soyun-i merged 1 commit intodevelopfrom
be/feat/299

Conversation

@soyun-i
Copy link
Contributor

@soyun-i soyun-i commented Apr 11, 2025

🚀 어떤 기능을 구현했나요 ?

  • 최상단 OrderService가 존재하여 너무 복잡한 거 같아 삭제했습니다
  • checkoutInstant도 여러 개를 한번에 구매할 수 있어서 기존에 존재하던 checkout API 으로 통일했습니다

🔥 어떤 문제를 마주했나요 ?

✨ 어떻게 해결했나요 ?

📝 어떤 부분에 집중해서 리뷰해야 할까요?

📚 참고 자료 및 회고

@soyun-i soyun-i self-assigned this Apr 11, 2025
@GetMapping("/{orderId}")
public ResponseEntity<OrderDetailPageResponse> getOrder(@CurrentUser User user, @PathVariable Long orderId){
OrderDetailPageResponse orderDetailResponse = orderService.getOrder(user, orderId, null, null);
OrderDetailPageResponse orderDetailResponse = orderGetService.getOrder(user, orderId, null, null);
Copy link
Contributor

@jihaneol jihaneol Apr 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

orderGetService 이 무엇인가요.
주문을 얻어 오는 건가요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

맞습니다! 하나의Service에 주문 관련 모든 걸 포함하기 싫어서 분리했습니다

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트 코드를 작성하면 좋을 것 같아요!
주문과 같은 복잡한 서비스는 테스트 코드를 작성하면서 자연스럽게 구조를 개선하고 서비스 레이어를 분리하는 방식이 될 수 있습니다. 테스트 코드를 작성하는 과정에서 의존성 분리, 파라미터 정리, 공통 로직 추출 등이 자연스럽게 이루어져 유지보수성과 확장성에 강한 코드를 완성할 수 있다고 생각합니다.

@GetMapping("/getCancel/{orderId}")
public ResponseEntity<OrderCancelResponse> getOrderCancel(@CurrentUser User user, @PathVariable Long orderId) {
OrderCancelResponse orderCancelResponse = orderService.getCancelPage(user, orderId, null, null);
OrderCancelResponse orderCancelResponse = orderGetService.getCancelPage(user, orderId, null, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

orderGetService에서 getCancelPage가 있으니 주문을 가져오는데 null을 보내는 이유가 있을까요?
있다면 dto로 만들어서 처리하는 방법이 더 좋을 것 같아요

  1. 의미가 불명확하다.
  2. 파라미터 늘어날때 유지 보수 힘들다.
  3. 서비스 로직에 null 처리 코드와 같은 방어로직이 쌓인다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

현재 회원은 orderId만 있으면 주문 취소 상세페이지를 가져올 수 있는데 비회원은 orderNumber와 phone이 있어야만 가져올 수 있게 설계되어있습니다! 같른 메서드를 사용하려다보니 null를 보내주게 되었습니다ㅜ

@PostMapping
public ResponseEntity<OrderResponse> createGuestOrder(@RequestBody @Valid OrderRequest orderRequest) {
OrderResponse orderResponse = orderService.createGuestOrder(orderRequest);
OrderResponse orderResponse = orderCreationService.createOrder(null, orderRequest);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이 부분도 위 리뷰와 동일합니다.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

이것도 똑같이 메서드를 하나로 처리하려다보니 생긴 이슈입니다. 기능은 똑같은데 회원 비회원 메서드를 따로 만들면 중복코드이지 않나 싶습니다ㅜ 더 좋은 방법이 있다면 알려주시면 감사하갰습니다!🥹

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 오버로딩을 사용했습니다. 이렇게 하면 의도도 명확하고 실수도 줄일 수 있습니다

Comment on lines -57 to -64
//주문 취소를 위한 쿼리
@Query("SELECT o FROM Order o " +
"LEFT JOIN FETCH o.orderDetails od " +
"LEFT JOIN FETCH od.saleProduct sp " +
"LEFT JOIN FETCH sp.stock " +
"WHERE o.id = :orderId AND o.userId = :userId")
Optional<Order> findForCancellation(@Param("userId") Long userId, @Param("orderId") Long orderId);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주문 취소 쿼리를 삭제하신 이유가 있을 까요?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

사용하고있지 않아서 삭제했습니다!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주문 취소를 못하는 건가요?

@soyun-i soyun-i merged commit 4534059 into develop Apr 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants