-
Notifications
You must be signed in to change notification settings - Fork 153
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
3단계 - 리팩터링(매장 식사 주문) #240
base: saerang
Are you sure you want to change the base?
3단계 - 리팩터링(매장 식사 주문) #240
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
안녕하세요.
몇 가지 코멘트 남겼습니다. 확인 부탁드려요.
|
||
private UUID menuId; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
menuId 로 간접참조 하고 있는데요. 이 경우 Menu 의 가격이 변경된다면 예전 주문을 확인할 때 변경된 가격을 노출시켜주는 걸까요?
public List<UUID> getMenuIds() { | ||
return orderLineItems.stream().map(TobeOrderLineItem::getMenuId).collect(Collectors.toList()); | ||
} | ||
|
||
public void validation(final int size) { | ||
if (orderLineItems.size() != size) { | ||
throw new IllegalArgumentException(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
사용하지 않는 코드는 지우는 것이 어떨까요?
|
||
public class OrderMenu { | ||
private final UUID id; | ||
private final boolean displayed; | ||
private final BigDecimal menuPrice; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Order BC 내에 Menu 정의 👍
각 컨텍스트마다 Menu는 다른 의미를 가지는데요. 이 부분은 문서도 수정해 보면 좋을 것 같습니다.
OrderMenu는 따로 DB에 저장될 필요는 없을까요?
|
||
@Table(name = "order_table") | ||
@Entity | ||
public class TobeOrderTable { | ||
@Column(name = "id", columnDefinition = "binary(16)") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
orderTable은 따로 리팩터링 하지 않은 이유가 있으신가요?
패키지도 신경써보면 좋을 것 같아요. application layer로 보이네요.
|
||
public void setId(final UUID id) { | ||
this.id = id; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
setter는 제거해 보면 좋을 것 같습니다.
@Transient | ||
private BigDecimal price; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
price는 VO로 만들어 보는 것이 어떨까요?
|
||
// public static TobeOrder order(final OrderStatus status) { | ||
// final TobeOrder order = new TobeOrder(UUID.randomUUID(), status, LocalDateTime.of(2020, 1, 1, 12, 0), | ||
// | ||
// ); | ||
// order.setId(UUID.randomUUID()); | ||
// order.setType(OrderType.DELIVERY); | ||
// order.setStatus(status); | ||
// order.setOrderDateTime(); | ||
// order.setOrderLineItems(Arrays.asList(orderLineItem())); | ||
// return order; | ||
// } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
주석은 지우는 것이 어떨까요?
|
||
public class InMemoryTobeOrderRepository implements TobeOrderRepository { | ||
private final Map<UUID, TobeOrder> orders = new HashMap<>(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fake 객체 👍
현재 어플리케이션 빌드가 실패하는데요. 이 부분도 확인되면 좋을 것 같아요. 🙂 |
마지막 미션 잘 부탁드립니다.