-
Notifications
You must be signed in to change notification settings - Fork 164
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단계 - 리팩터링(매장 식사 주문) #242
base: seungjoopet
Are you sure you want to change the base?
3단계 - 리팩터링(매장 식사 주문) #242
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.
안녕하세요. 🙇
메뉴 식사 주문 리팩토링 고생 많으셨습니다.
몇 가지 코멘트 남겨두었어요.
확인 부탁드릴게요.
코멘트 리뷰에 대해 적용은 꼭 안하셔도 되니
논의하고 싶은 부분 및 코멘트에 대한 의견은 부탁드릴게요. 😄
import kitchenpos.eatinorders.domain.eatinorder.EatinOrder; | ||
import org.springframework.data.jpa.repository.JpaRepository; | ||
|
||
public interface JpaEatinOrderRepository extends EatinOrderRepository, |
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.
이번엔 시간 관계상 전체적으로 TC를 제외하셨군요 😄
} | ||
|
||
@Override | ||
public EatinOrderDTO init(final EatinOrderInitCommand command) { |
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.
등록에 대해서 용어 사전에 정리하신 것은 없지만 init이라는 이름은 모호한 것 같은데 적당한 것으로 바꿔볼 수 있을까요?
다른 곳에서는 register를 사용하셨었죠 😄
|
||
@Table(name = "order_table_new") | ||
@Entity | ||
public class OrderTableNew { |
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.
용어 불일치가 있네요 이미 인지하고 계신 사항이겠죠? 꼭 수정은 안해주셔도 됩니다 😄
용어 사전에서 정리한 이름은 RestaurantTable
이네요.
package kitchenpos.eatinorders.domain.ordertable; | ||
|
||
public enum Status { | ||
EMPTY, NOT_EMPTY |
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.
이쪽도 이전 용어 사전이랑은 다르긴 하네요.
필요하면 지속적으로 용어 사전, 모델링을 지속적으로 수정할 수 있습니다.
용어 불일치에 대해 경계하면 좋을 것 같습니다.
} | ||
|
||
@Override | ||
public UUID create(final Name name) { |
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.
상품, 메뉴 쪽은 register
매장 식사 주문 테이블은 create 이군요 😄
어떤 관점으로 바라보고 차이를 두셨는지 알 수 있을까요?
정말 달라야 한다면 다른 독자가 create, register에 대해 저와 같이 오해할 수 있을 것 같고 그렇다면
이 또한 용어 사전에 정리될 수 있다는 포인트가 된다는 점 참고 해주시면 좋을 것 같아요 😄
리뷰어님 안녕하세요!
매장식사 주문을 구현하였으며, 대부분의 코드는 이전 단계와 비슷한 방식으로 프로그래밍했습니다.
업무와 함께 병행하느라 급하게 과제를 진행하여 테스트코드는 누락되어 있습니다.
(시간이 너무 없어서 매장 식사 주문 비즈니스 로직도 틀렸을 수 있을 것 같아요^_ㅠ)
고민
이번에는
Usecase는 어떤 단위로 구성해야 하는가? 에 대한 고민을 많이 했습니다.
보통 저희가 SRP, ISP를 달성하려고 노력을 많이 하는데요
어플리케이션서비스(UseCase)에서 이 SRP는 어떤 단위로 구성될 수 있을까? 이런 고민이 들었습니다.
그래서 제 개인적인 생각으로 일단 구성을 해보았습니다.
OrderTable application service를 보시면,
OrderTableCreationUseCase, OrderTableSitUseCase 로 나뉘어져 있는데요.
리뷰어님께서 제가 고민한 부분에 대한 의견이 있으시면 말씀부탁드립니다~!
참고할만한 자료가 있으면 공유해셔도 좋습니다!
마지막 리뷰요청드립니다 :)