refactor(order): 주문 내역 리스트 반환 값 수정 #246
Conversation
- 메뉴 가격 추가
WalkthroughOrderResponseDto의 메뉴 정보 필드가 기존의 메뉴명-수량(Map<String, Integer>) 구조에서 메뉴명-MenuDetail(Map<String, MenuDetail>) 구조로 변경되었습니다. MenuDetail 클래스가 새로 추가되어 수량과 가격 정보를 함께 담도록 하였으며, 관련 생성 로직도 이에 맞게 수정되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant UserOrder
participant OrderResponseDto
participant MenuDetail
UserOrder->>OrderResponseDto: fromEntity(userOrder) 호출
OrderResponseDto->>UserOrder: getOrderItems()
loop 각 OrderItem에 대해
OrderResponseDto->>MenuDetail: MenuDetail 생성 (수량, 가격)
OrderResponseDto->>OrderResponseDto: menuDetails Map에 merge (수량 누적, 가격 유지)
end
OrderResponseDto-->>UserOrder: OrderResponseDto 반환 (menuDetails 포함)
Estimated code review effort🎯 2 (Simple) | ⏱️ ~7분 Possibly related PRs
Suggested reviewers
Note ⚡️ Unit Test Generation is now available in beta!Learn more here, or try it out under "Finishing Touches" below. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/dto/MenuDetail.java (2)
9-10: 원시 타입 사용에 대한 일관성 검토가 필요합니다.필드에
int원시 타입을 사용했는데, OrderResponseDto의 다른 필드들(Integer totalPrice,Long id등)은 래퍼 타입을 사용하고 있습니다. 일관성을 위해Integer로 변경하는 것을 고려해보세요.- private int quantity; - private int price; + private Integer quantity; + private Integer price;
1-11: 필드 타입과 불변성 개선을 고려해보세요.현재 구현은 기본적으로 올바르지만 다음 사항들을 개선할 수 있습니다:
- 필드 타입:
int대신Integer를 사용하여 null 안전성을 확보할 수 있습니다.- 불변성: 필드를
final로 선언하여 불변 객체로 만드는 것을 권장합니다.- 유효성 검증: 수량과 가격이 음수가 될 수 있는지 비즈니스 로직을 확인해보세요.
개선된 버전:
@Getter @AllArgsConstructor public class MenuDetail { private final Integer quantity; private final Integer price; // 필요시 유효성 검증 생성자 추가 public MenuDetail(Integer quantity, Integer price) { if (quantity < 0 || price < 0) { throw new IllegalArgumentException("수량과 가격은 음수일 수 없습니다"); } this.quantity = quantity; this.price = price; } }nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/dto/OrderResponseDto.java (1)
39-39: 단가 동일성 가정에 대한 검증 고려코멘트에서 "단가는 동일하다는 가정"이라고 명시했는데, 만약 실제로 동일한 메뉴에 대해 다른 가격이 존재할 가능성이 있다면 검증 로직을 추가하는 것을 고려해보세요.
(oldVal, newVal) -> new MenuDetail( oldVal.getQuantity() + newVal.getQuantity(), - oldVal.getPrice() // 단가는 동일하다는 가정 + oldVal.getPrice() // 단가는 동일하다는 가정 )검증 로직을 추가하려면:
(oldVal, newVal) -> { if (oldVal.getPrice() != newVal.getPrice()) { // 로깅 또는 예외 처리 } return new MenuDetail( oldVal.getQuantity() + newVal.getQuantity(), oldVal.getPrice() ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/dto/MenuDetail.java(1 hunks)nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/dto/OrderResponseDto.java(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/dto/MenuDetail.java (1)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/dto/OrderResponseDto.java (1)
Getter(14-54)
🔇 Additional comments (7)
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/dto/MenuDetail.java (1)
8-11: DTO 클래스 설계가 적절합니다.MenuDetail 클래스는 수량과 가격 정보를 캡슐화하는 간단한 DTO로 잘 설계되었습니다. Lombok 어노테이션을 사용하여 보일러플레이트 코드를 줄인 점도 좋습니다.
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/dto/OrderResponseDto.java (6)
22-22: 필드명 변경이 의미를 더 명확하게 표현합니다.
menuNamesAndQuantities에서menuDetails로 변경하여 이제 수량뿐만 아니라 가격 정보도 포함한다는 의미를 더 명확하게 표현합니다.
4-4: LinkedHashMap 사용으로 순서 보장이 적절합니다.LinkedHashMap을 사용하여 메뉴 아이템의 삽입 순서를 보장하는 것은 사용자 경험 측면에서 좋은 선택입니다.
Also applies to: 26-26
28-42: Map.merge를 사용한 중복 메뉴 처리 로직이 효율적입니다.동일한 메뉴명에 대해 수량을 누적하는 merge 로직이 깔끔하고 효율적으로 구현되었습니다. 단가가 동일하다는 가정도 비즈니스 로직상 합리적입니다.
22-22: 필드명과 타입 변경이 적절합니다.
Map<String, Integer>에서Map<String, MenuDetail>로의 변경으로 메뉴 정보를 더 풍부하게 표현할 수 있게 되었습니다.
26-26: LinkedHashMap 사용으로 순서 보장.
LinkedHashMap을 사용하여 메뉴 아이템의 삽입 순서를 보장하는 것은 좋은 선택입니다.
50-50: 빌더 패턴의 필드명 변경이 일관성 있게 적용되었습니다.새로운
menuDetails필드명으로 일관성 있게 변경되었습니다.
| for (OrderItem item : userOrder.getOrderItems()) { | ||
| String name = item.getMenu().getName(); | ||
| int quantity = item.getQuantity(); | ||
| int price = item.getMenu().getPrice(); // 메뉴 단가 | ||
|
|
||
| // 메뉴명이 중복되면 수량만 누적 | ||
| // merge 람다함수 활용(해당 key가 있으면 수량 누적, 없으면 새로 생성) | ||
| menuDetails.merge(name, | ||
| new MenuDetail(quantity, price), | ||
| (oldVal, newVal) -> new MenuDetail( | ||
| oldVal.getQuantity() + newVal.getQuantity(), | ||
| oldVal.getPrice() // 단가는 동일하다는 가정 | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
동일 메뉴명의 서로 다른 가격 처리 로직을 검증해주세요.
현재 구현에서 잠재적인 문제가 있습니다:
Line 39의 주석에서 "단가는 동일하다는 가정"을 하고 있지만, 실제로 동일한 메뉴명이지만 다른 가격을 가진 경우(예: 사이즈별 가격 차이, 프로모션 등)에 대한 처리가 부족합니다.
다음 스크립트로 동일 메뉴명에 대한 서로 다른 가격 케이스가 존재하는지 확인해보세요:
만약 동일 메뉴명에 다른 가격이 있을 수 있다면, merge 로직을 다음과 같이 개선하는 것을 고려해보세요:
menuDetails.merge(name,
new MenuDetail(quantity, price),
(oldVal, newVal) -> {
if (!oldVal.getPrice().equals(newVal.getPrice())) {
// 로깅 또는 예외 처리
log.warn("동일 메뉴명 '{}' 에 대해 서로 다른 가격 발견: {} vs {}",
name, oldVal.getPrice(), newVal.getPrice());
}
return new MenuDetail(
oldVal.getQuantity() + newVal.getQuantity(),
oldVal.getPrice() // 또는 적절한 가격 결합 로직
);
}
);🏁 Script executed:
#!/bin/bash
# 동일 메뉴명이지만 다른 가격을 가진 케이스 확인
ast-grep --pattern $'class Menu {
$$$
private String name;
$$$
private int price;
$$$
}'Length of output: 99
동일 메뉴명에 대한 가격 차이 여부를 확인하기 위해 Menu 클래스 정의를 조회해 주세요.
🏁 Script executed:
#!/bin/bash
# Menu.java 파일 위치 탐색 및 상위 200줄 출력
fd -t f "Menu.java" | xargs -I{} sed -n '1,200p' {}Length of output: 1906
동일 메뉴명 그룹화 시 가격 차이 처리 검증 및 개선 제안
현재 OrderResponseDto.java(28–42행)의 merge 로직은 “단가는 동일하다는 가정” 하에 이름(name)만으로만 그룹화하기 때문에, 동일한 메뉴명으로 가격이 다른 경우(예: 프로모션, 사이즈별 옵션 미반영 등) 잘못된 합산 결과를 낼 수 있습니다.
코드베이스 상에서 Menu 엔티티에는 이름(name)에 대한 유니크 제약이 없고, 가격(price) 필드가 Integer 타입으로 선언되어 있어 실제로 같은 이름의 메뉴에 서로 다른 가격이 부여될 가능성이 존재합니다.
점을 확인해 보시고, 아래 중 하나 이상의 조치를 고려해 주세요:
- 그룹 키를 메뉴명(name) 대신 메뉴 ID(
menu.getId()) 또는 이름+가격 조합으로 변경 - 동일 메뉴명끼리 묶되, 가격 불일치 시 로그 출력 또는 예외 발생 로직 추가
- 가격 차이가 발생하지 않도록 DB나 서비스 레이어에서 메뉴명 유일성(unique) 보장
예시 개선안:
- menuDetails.merge(name,
- new MenuDetail(quantity, price),
- (oldVal, newVal) -> new MenuDetail(
- oldVal.getQuantity() + newVal.getQuantity(),
- oldVal.getPrice()
- )
- );
+ menuDetails.merge(name,
+ new MenuDetail(quantity, price),
+ (oldVal, newVal) -> {
+ if (!oldVal.getPrice().equals(newVal.getPrice())) {
+ log.warn("동일 메뉴명 '{}' 에 대해 서로 다른 가격 발견: {} vs {}",
+ name, oldVal.getPrice(), newVal.getPrice());
+ }
+ return new MenuDetail(
+ oldVal.getQuantity() + newVal.getQuantity(),
+ oldVal.getPrice()
+ );
+ }
+ );- 파일: nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/dto/OrderResponseDto.java (28–42행)
Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In
nowait-app-admin-api/src/main/java/com/nowait/applicationadmin/order/dto/OrderResponseDto.java
around lines 28 to 42, the current merge logic groups order items by menu name
assuming the price is always the same, which can cause incorrect aggregation if
prices differ for the same menu name. To fix this, change the grouping key to
use a unique identifier such as menu ID or a combination of menu name and price.
Alternatively, add logic to detect price mismatches during merging and either
log a warning or throw an exception. This ensures accurate aggregation of
quantities without mixing items of different prices.
작업 요약
Issue Link
#245
문제점 및 어려움
해결 방안
Reference
Summary by CodeRabbit
신규 기능
버그 수정