Skip to content

Conversation

@tyakamyz
Copy link

안녕하세요 리뷰어님
4단계 - 멀티 모듈 적용 PR 드립니다 :)

모듈은 유지보수 측면이나 의존성을 나눈 부분이 연계된다고 생각하여 패키지 기준으로 나누었습니다!

한가지 고민되었던 부분이 있어서 리뷰어님께 조언을 구하고자 공유 드립니다
이전 단계에서 Table이 Order를 의존하고 있던 관계를 DIP 원칙에 따라 인터페이스를 사용하여 의존성을 변경하였습니다!
근데 이번 모듈에서 해당 부분을 적용하려고하니 Order에서 Menu와 Table 두곳을 의존해야했고
Order와 Table은 양방향으로 의존하는 형태가 되었습니다ㅜ

Order와 Table을 하나의 모듈로 합치는 방법과 의존성 제거를 위해 Table쪽으로 책임을 위임하는 방법 중
고민 끝에 Table쪽으로 책임을 위임하였습니다
이런 경우는 어떤 방식으로 처리하는게 더 좋을까요?

그럼 마지막 리뷰도 잘 부탁드리겠습니다! 👍

Before (모듈 적용 전)

image

After (모듈 적용 후)

image

Copy link

@enemfk777 enemfk777 left a comment

Choose a reason for hiding this comment

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

안녕하세요 시현님 :)

4단계도 잘 진행해 주셨습니다!

바로 머지해도 될 것 같지만,
질문 주신 사항에 코멘트 남겨드렸으니 한번 확인 후 다시 요청 부탁드려요!

Comment on lines 50 to 59
project(':module-order') {
dependencies {
compile project(':module-menu')
}
}

project(':module-table') {
dependencies {
compile project(':module-order')
}

Choose a reason for hiding this comment

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

module-order 에서 module-table을 의존 해하게 하면 OrderStatusValidator가 module-order로 이동할 수 있을 것 같아요 :)
StatusValidator 인터페이스는 그대로 module-table 에 두고요 :)

order가 여러개의 의존성을 임포트 받는건 문제가 없다고 생각됩니다 :)

@tyakamyz
Copy link
Author

tyakamyz commented Jul 1, 2022

리뷰어님 안녕하세요
피드백 적용하고 테스트 완료했습니다!

코드량이 굉장했던 미션인데 꼼꼼하게 리뷰해주셔서 감사합니다 :)
덕분에 가장 힘들었던 미션을 잘 끝낸 것 같네요
마지막까지 감사드립니다 👍

Copy link

@enemfk777 enemfk777 left a comment

Choose a reason for hiding this comment

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

안녕하세요 시현님 :)

마지막 코멘트까지 잘 반영해주셨네요..!!
고생하셧습니다!!

이번 미션은 여기까지입니다.
이 다음 미션 하나만 남았는데, 꼭 완주하시길 응원하겠습니다..!!

@enemfk777 enemfk777 merged commit 97c9930 into next-step:tyakamyz Jul 1, 2022
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