Skip to content
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

Step3 - [3단계 - 양방향을 단방향] #40

Merged
merged 8 commits into from
Jan 21, 2021
Merged

Conversation

jhhj424
Copy link

@jhhj424 jhhj424 commented Jan 18, 2021

안녕하세요 시홍님!!
결국 일단 고민한대로.. 리팩토링 진행 하고 PR 보내도록 하겠습니다.. :)

시홍님과 다른 분들께도 질문을 드렸었고, 받은 여러 의견을 바탕으로 다음 구조로 진행하였습니다.

일단 이번 미션은 양방향을 단방향 미션이었는데요, 시홍님의 의견처럼 무조껀 단방향으로 하기 보다는 양방향도 고려할 수 있으면 해도 좋을 것 같다는 생각을 했습니다.

그래서 일단 제가 결론 내린 구조는 이렇게 되었습니다.
image

일단 저는 단방향 일 대 다의 관계는 배제 하였고, 단방향 일 대 다가 필요할 경우 양방향 관계 를 고려하였습니다.

그리고 이전에 양방향 관계를 가졌던 Order - OrderLineItem 경우에는 일 대 다는 끊어버리고 다 대 일 관계만 유지하도록 변경하였습니다.


이번 미션을 진행하면서 먼저 인수테스트를 한번 작성해보았고😁,
이후에 리팩토링과 관계 변경을 진행하였습니다!

저도 고민을 했지만 아직 부족한게 많아, 생각지 못한 점이나 시홍님의 의견이 다른 부분이 있을 수 있다고 생각하기 때문에 리뷰 잘 부탁드리겠습니다!!

학습도 하면서 생각을 많이 하는동안 결과적으로는 진행 한 내용은 많이 없는데 며칠의 시간이 흘렀네요 (매일매일 꽤 많은 시간을 투자했음에도.. 😅)ㅠㅠ

어쨌든 이번 단계 진행하는 동안 이런저런 고민을 같이 나눠주셔서 감사합니다! 🙏

Copy link

@hongsii hongsii left a comment

Choose a reason for hiding this comment

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

3단계도 많은 고민을 통해 요구사항을 반영해주셨네요! 👍
피드백 남겼으니 확인하셔서 반영해주세요!

1. 단방향으로 끊어내고 OrderLineItem에 대한 별도의 영속화 로직을 가지는 것 보다, 두 객체의 관계에서는 양방향을
유지하는게 더 나을 수 있다.

2. 비즈니스적으로 보면, Order 객체가 생성될 때 OrderLineItem 객체도 같이
생성이 될 것이고 생성되었던 두 객체가 삭제되는 시점도 동일하게 될 것이기 때문에, 둘의 생명주기는 같다.

3. Order를
통해 OrderLineItem이 핸들링 되도록 하는편이 더 나을 수 있다.
Copy link

@hongsii hongsii left a comment

Choose a reason for hiding this comment

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

피드백 반영을 잘해주셨네요! 👍
간단한 피드백 남겼으니 확인하셔서 반영해주세요~!

1. Order 클래스에서 Menu 객체 참조 제거
2. 요구 사항 정리
3. 테스트 변경 & 추가
4. ExceptionAdvice 클래스 추가
5. Quantity 값 객체 추가
Copy link

@hongsii hongsii left a comment

Choose a reason for hiding this comment

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

제가 의도한 대로 잘 생각하셔서 피드백을 반영해주셨네요! 👍
이번 단계는 여기서 머지할게요! 다음 단계 진행해주세요~!

src/test/java/kitchenpos/menu/MenuAcceptanceTest.java Outdated Show resolved Hide resolved
@hongsii hongsii merged commit d6f25fe into next-step:jhhj424 Jan 21, 2021
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.

None yet

2 participants