-
Notifications
You must be signed in to change notification settings - Fork 253
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
[1단계 - 사다리 생성] 테오(최우성) 미션 제출합니다. #96
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 java.util.List; | ||
|
||
public class Line { |
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.
Maker에서 모든 로직을 다 처리하다보니, 가장 중요한 Line, Ladder, Block이 아무 역할을 하지 않고 도메인모델이 데이터홀더 역할만 하고 있는것 같아요!.. (빈약한 도메인 모델)
만약이렇게 된다면, Maker에서 로직들이 쌓이게 되면서, 중복 로직이 생길 수 있고, maker로 로직들이 쌓일 것 같은데ㅎㅎ, 테오는 어떻게 생각하시나요?
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.
말씀해주신대로 Line, Ladder가 핵심 도메인 객체이나, 빈약한 도메인 모델이라는 생각이 들었습니다!
기능이 추가되거나, 요구사항이 변경되는 경우에도 Line, Ladder 객체는 영향을 받지 않고 Maker 객체만 영향을 받을 가능성이
큰 것 같다고 저도 생각해요.
그리고 언제든지 사다리 게임의 규약을 어기는 사다리
가 만들어질 수도 있으며, Ladder, Line 객체는 상태를 스스로 관리하지 못하는 꼴이 됩니다. 즉, 핵심 도메인 객체들임에도 불구하고 상태 관리의 책임을 Maker 객체들이 가지고 있게 되는거죠.
해결책을 몇 가지 생각해 보았는데요,
- Ladder, Line을 그대로 유지하되, Maker 객체에서만 인스턴스를 반환될 수 있도록 한다.
- Ladder, Line에게 Maker 객체들의 책임을 위임한다.
저는 2번 방식이 조금 더 낫다고 생각해서 Maker들의 책임을 Ladder, Line에게 이동시켰습니다.
다만 아직 2번이 좋은 방법이라고 확신하지 못하고 있는데요, 핵심 도메인 객체에서 BlockGenerator를 필드로 갖기 때문
입니다.
물론 Line 객체만 BlockGenerator를 가져도 되지만, 테스트의 용이성 때문에
Line을 포함하는 Ladder 객체까지 BlockGenerator를 필드로 가지게 되었습니다.
마찬가지로 나중에 Ladder를 감싸는 LadderGame이라는 객체가 생기면, 그 객체도 BlockGenerator를 가지게 될 것인데요,
이런 구조가 도메인에 혼란을 야기하지는 않을까요? 서브웨이는 어떻게 생각하는지 궁금합니다!
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.
저도 테오가 변경해주신 것처럼 2번 형태가 쫌 더 도메인 집약적인 것 같습니다.
저도 추가적으로 질문드리고 싶은것이. 꼭 Ladder에서 blockGenerator를 갖고 있는 형태여야 하나요?
private List<Line> generateEachLines(final int playerNumber, final int height) {
List<Line> lines = new ArrayList<>();
for (int i = 0; i < height; i++) {
lines.add(new Line(playerNumber, new BlockGenerator()));
}
return lines;
}
이렇게 메서드에서 직접 BlockGenerator 를 넣어주는 형태로 하면 해결되는 부분이 아닐까요?
만약 Block 생성 전략이 다른것으로 변경된다면, DI로 Ladder에서 주입해주기 떄문에, 주입해주는 곳만 변경해주면 OCP도 만족하는 코드가 아닐까 해서요!
저렇게 변경하고 테스트를 돌려봤는데요. 모두 통과하는 테스트로 작성되어있어서 질문드려요!
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.
답변 감사합니다 서브웨이!
저도 그런 형태를 상상해보았는데요, 사실 Ladder
에서 주입해줘도 되지만, Ladder
객체의 테스트가 어려워져서 포기했습니다.
어쨌거나 포함관계에 있어 '포함되는 객체가 랜덤에 의존적이면, 테스트가 불가능'하게 되니까요.
하지만 말씀해주신대로, 현재 서브웨이가 제시해주신 코드대로 수정해도 아무런 문제가 없습니다!
Ladder 객체에 대해 제한적인 부분만 테스트했기 때문인데요, 블록 값을 일일이 지정하면서(TestBlockGenerator로)
테스트하는 것은 단위 테스트가 아니라 통합 테스트라고 생각했기 때문입니다.
만약 요구사항이 변경되어 통합 테스트도 구현되어야 한다면, 테스트를 위해 Ladder도 BlockGenerator를 인스턴스 변수로 가져야 하지 않을까요?!
그리고 BlockGeneator를 주입해주는 부분이 Ladder
객체라는 점이 예측 불가능하고, 찾기 어렵다는 단점이 있을 것 같습니다.
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.
상세한 설명 감사합니다 서브웨이.. 🥺 (루터회관 앞 서브웨이가 좋아질거같아요..)
제가 아직 스프링 같은 웹 프레임워크를 제대로 사용해보지 않았는데, 말씀해주신
객체의 생성을 프레임워크가 가져가게 되는데요!
부분은 제가 아직 웹 프레임워크를 제대로 공부해본적이 없어 아직 이해가 어렵지만, 나중에 웹 공부하면서 차차 이해가 될 것이라 예상합니다!
어쨌거나 하나의 정답만 있는 것이 아니므로, 스타일을 따라가는게 중요하겠네요!
저는 서브웨이처럼 도메인 집약적인 구조가 좋다고 생각하므로, 이를 유지해야겠습니다 👍
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.
테오 요거 제가 답변을 다시 정리하려고 지웠는데요!ㅎㅎ 다시 답변 달겠습니다! 다시 정리해서 답변 달게요ㅎㅎ
근데, 루터회관 앞 서브웨이는 정말 추억이네요ㅋㅋ
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.
답변 다시 작성해서 올려요! 프레임워크 내용은 약간 잘못된 것 같아서 제거했습니다!..🥲
지금 코드는 말씀주신것 처럼 굳이 갖지 말아야 할 속성들을 갖게 되는 단점이 있을 것 같아요. 하지만, 현재 방식으로 하는게 좀 더 도메인 집약적인 형태인것 같아서 2번에 더 마음이 끌리는 것 같습니다.
나중에 현업에 들어가시게 되면 정말 많은 코드들을 다뤄야하는데, 이때 관련 있는 도메인객체끼리 묶어 응집도를 높이고(캡슐화), 서로 관련이 떨어지는 것끼리 결합을 낮춰주는 것이 중요해질거에요!
그렇게 하면, 관련없는 객체끼리 결합을 낮추기때문에 비지니스 요구사항을 빠르게 파악할 수 있게 됩니다! 버그도 덜 발생할 것이에요! 저는 요런 것을 생각했을 때 2번이 더 타당하다고 생각했어요!
근데 BlockGenerator를 상위 계층에서 계속 넣어주어서 필요하지 않는 속성을 갖는 것이 고민이라면, 제가 의견드린 것처럼 그냥 Line에서 직접 넣어주는 것도 괜찮다고 생각합니다ㅎㅎ. Line의 테스트가 어렵다고 말씀주셨는데, 테스트의 범위를 스스로 잘 적절하게 정하면 된다고 해결된다고 생각해요
사실 저는 아직까지도, Line에서만 BlockGenerator을 갖도록 의견드리고 싶어요ㅎㅎ 제 개인적인 입장인데, 테스트코드 때문에 비지니스 영역이 영향이 가는건 선호하지 않습니다!
저는 정답은 없다고 생각해요ㅎㅎ 뭐가 더 합리적인가?를 생각해보시고 2단계에서 적용해주세요!
제가 봤을 땐, 테오가 이미 학습하는 수준에서 1,2번에 대해서 고민을 잘 해주셔서 스스로 합리적인 것을 선택만 하시면 될 것 같습니다!
첫번째 질문
이 말이 약간 어색한것 같아요!.. 저는 테오의 질문을 다음과 같이 이해했습니다. domain 영역내의 함수
fun checkLadder() {
if (a > 1) {
throw RuntimeException("여기가 변경")
}
} 에러메시지가 변한다고 도메인이 외부영역로부터 변화를 받는것일까요? 여기서 외부 영역이 무엇을 이야기하는걸까요? (에러메시지를 외부로 전달해야한다면, 도메인내 에러메시지랑 외부로 노출되는 메시지랑 나뉠 필요가 있을 수 있을 것 같아요. 이 말뜻이 이해가 안된다면 추가 질문주세요!) 그리고, 도메인에 특화된 예외도 도메인 영역이라고 생각합니다! (LadderLengthException 도 도메인영역의 객체라고 생각함)
보통 예외를 커스텀하게 전환하는 이유는 예외에서 도메인적인 표현을 하기 위함이라고 생각합니다. 혹시 제가 질문을 이해를 잘못하고 답변했다면, 코멘트 달아주세요! 두번째 질문저도 테오의 말에 동의합니다. 따라서, dto 사용이나 getter로 빼주는게 맞다고 생각해요 저는 도메인 모델은 외부로부터 보호받아야하는 순수한 객체라고 생각하는데요. View에서 도메인 모델 의존을 하면, 외부의 변경사항이 모델의 변경사항으로 이어질 수 있습니다! 의존을 안하는 방향으로 수정해보면 좋을 것 같아요. 음, 저는 dto를 사용한다고 오버엔지니어링이라고 하는것에 동의하지 않습니다ㅎㅎ dto가 엄청난 기술적인 영역도 아닐뿐더러, dto는 말그대로 data transfer object 입니다. 데이터를 전달하는 책임을 갖는 객체이기 때문에 큰 문제 없다고 생각해요!(책임 분리) 보통 외부(웹,앱 등등..)와의 통신에서 dto개념을 사용하지만, View를 클라이언트영역이라고 생각하면 사용해도 무관하다 생각합니다. 저희의 학습목표를 생각해보면, 오버엔지니어링의 범위를 어느정도 정할 수 있을거라고 생각해요! |
우선 서브웨이 리뷰하느라 고생하셨어요..! 👍 서브웨이가 외부영역은 무엇인지, 도메인을 보호하는 이유가 무엇인지.. 등을 여쭤보았는데요, 여기서 한번 답해볼게요! (첫번째 질문) 도메인의 외부 영역은 대표적으로 View가 있을 것이라 생각합니다. 사다리 게임에서 변하지 않는 것은 그렇기 때문에 도메인 모델을 정의하는 것이 중요하고(사다리 게임은 사다리로 구성되어야 해! 등), 그렇기 때문에 도메인을 영어 View가 되었든, 한국어 View가 되었든, Console이든, Web이든, View는 변화할 수 있습니다. 그렇기 때문에 저는 예외 메세지를 도메인 영역에서 뽑아내려고 시도했었습니다. 그런데 생각해보니 Custom Exception 안에 예외 메세지를 넣어도 서브웨이 말씀처럼 예외 자체가 도메인의 영역이니 바뀐게 없다는 생각이 들더라구요. 이 문제를 해결하려면 '메세지를 View 단에서 정의'해야 한다고 생각하게 되었습니다.
|
안녕하세요 테오! 답변이 늦었습니다!. 쉽게 변하는 것 들로부터 가장 중요한 비지니스의 변경을 막기 위함, 즉, 비지니스 변경에 대해서 유연하게 대처하기 위함이란 말과 결국 동일한 말이 될 것 같은데, 테오가 말씀주신 것처럼 클라이언트, 기술로부터 의존을 끊어내어 비지니스를 보호하는 것이 맞습니다.
네 맞습니다. 사실, 에러메시지의 경우 View로 노출되는 에러도 있지만, 내부에서 디버깅 확인용으로 사용하기 때문에 '저걸 저렇게 분리하는게 맞나?' 라는생각이 들었는데 테오의 의도가 VIew단으로 노출되는 거라 나눠주신것 같아서 말씀드렸습니다! 그런데, 지금까지 도메인 내용이 외부로 노출되지 않도록 가이드를 드렸기때문에 그걸 지키면서 개발하는 것이라면, 외부로 노출되는 메시지를 따로 ErrorOutputView로 만들어도 좋을 것 같아요ㅎㅎ 제가 놓친부분이고 생각을 못한부분이라 저도 배워가네요ㅎㅎ |
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.
안녕하세요 테오!
1단계 사다리 미션 리뷰 잘 적용해주셨네요!
테오질문에 대한 답변을 다시 정리해서 올렸는데 확인부탁드려요! (#96 (comment))
1단계는 이만 merge 하겠습니다! 추가 코멘트 달아두었는데, 2단계때 고민후에 적용해주세요~
|
||
public class LadderLengthException extends CustomException { | ||
|
||
private static final String MESSAGE = "사다리의 길이는 <플레이어 수 - 1> 이상이어야 합니다."; |
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.
첫 리뷰 드린것처럼 도메인적인 의미를 넣어 어떤 에러인지 파악하기 위한 LadderLengthException, PlayerNumberException, PlayerNameLengthException은 도메인패키지로 이동해야하지 않을까요?
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.
저도 동의합니다! 도메인적 성향이 강하기때문에, 예외를 도메인으로 이동하고 '메세지만' 뽑아내는 방법을 한번 연구해봐야겠어요 ㅎㅎ
@@ -0,0 +1,10 @@ | |||
package ladder.exception; |
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.
테오 customException에 대해서 쫌 더 생각해봤습니다.
도메인을 외부로부터 보호하는 이유는 기술적 영역, 클라이언트 영역 같은 쉽게 변화할 수 있는 것으로부터 중요한 비지니스 영역을 분리해서 비지니스의 이해화 유연한 변경을 할 수 있도록 하자가 목표 입니다.
그럼 에러메시지가 그런 것들을 방해하는 요소가 될까요? 라고 질문한다면, 저는 아니라고 생각합니다.
따라서 테오가 의도한 CustomException는 과연 필요할까? 라는 의문이 들었는데요.
View의 변경이 도메인에 영향을 끼친다는 것과는 쫌 다른 차원인거 같아요.
에러메시지도 View에 표현되는 것이라 구분해야 한다고 한다고 생각이 들면
LadderLengthException, PlayerNumberException, PlayerNameLengthException는 도메인 영역이 되어야 하니까 아래처럼 controller에서 직접 에러메시지를 표현하는게 되어야 하지 않나 생각듭니다.
public class LadderGameController {
public void run() {
try {
Players players = initPlayers();
Ladder ladder = initLadder(players.size());
showResult(players, ladder);
} catch(LadderLengthException e) {
print("외부로 노출될 메시지")
} catch(PlayerNumberException e) {
print("외부로 노출될 메시지")
} ...
}
...
customException에 대한 리뷰에 대해서 계속 혼란주는거 같아서 죄송합니다🥲
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.
서브웨이 🥙 말씀은,
도메인을 보호하는 이유는 쉽게 변경되는 부분으로부터 중요한 비즈니스 로직을 분리하기 위함 이고,
과연 예외메세지의 변경으로부터 도메인을 보호하는 것이 올바른가? View와는 다른 차원이지 않을까? 라고 이해했습니다.
사실 말씀해주신 것처럼, 일반적으로 이야기하는 View의 변경이 도메인에 영향을 미치는 것
과는 조금 다른 개념인 것 같긴 합니다.
예외메세지 하나때문에 객체가 생기고, 코드로 주신 것처럼 예외마다 어떤 메세지를 쏴줄지를 결정해주는 메커니즘을 만드는 것이
득보다 실이 큰 느낌이 들기도 하는 것 같습니다.
이런 부분도 상황에 따라 달라질 수 있는 부분일까요?
요즘 학습을 하면서 느끼는 것인데, 모든 기술 및 로직에는 trade-off가 존재하는 것 같아요.
나중에 여러 언어로 View가 생긴다면, 이 경우에는 예외 메세지를 도메인에서 뽑아내는게 중요하겠죠. View마다 언어가 다르니까요!
이런 경우에는 저희가 지금 다루고 있는 방식이 사용될 수도 있을까요?! 완전히 엇나간 접근방식인지, 아니면 현재 단계에서 다루기에는
너무 비용이 큰 방식인지 궁금합니다.
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.
적고보니 뭔가 또 질문드리면서 끝난 거 같은데.. ㅋㅋㅋ
2단계에서 일단 제 의견을 증명할 수 있는 방법(?)을 찾아 오겠습니다
그때 코드를 통해 다시 한 번 봐주시면 감사하겠습니다!!
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.
네넵! 그때 테오가 고민하는 언어별 View에 대해 대응을 어떤식 할 수 있는지(?) 이야기 나눠봐요!
안녕하세요 서브웨이! 🥙
사다리 생성 게임 구현 완료했습니다.
구현 과정에서 이런 저런 고민이 많았는데요,
스스로 고민해볼 수 있는 것들은 제외하고, 도움이 필요한 부분에 대해 몇가지 의견을 여쭙고 싶습니다.
고민사항
CustomException
을 정의해서 도메인 영역에서 예외 메세지들을 제거했습니다.메세지 자체를 Exception이 가지고 있게 함으로써, 메세지가 변하더라도 도메인 영역은 수정되지 않도록 설계했습니다.
에러 메세지가 영어로 바뀌거나 하는 등에 도메인 영역이 영향받을 수 있다고 생각했기 때문입니다.
이러한 설계 방식은 어떠한지 궁금합니다.
그리고 사용자 입력이 아닌 개발자 실수로 인해 발생하는 예외도 커스텀 예외로 정의할 필요가 있을까요?
(다음과 같은 경우입니다. RuntimeException을 던지고 있으나, 여기서도 커스텀 예외를 던져야 할까요?)
만약 도메인 객체를 그대로 넘겨주게 된다면, 불필요한 정보 및 내부 구현을 외부에 드러내는 셈이 되는데, Controller에서 getter를 사용해
도메인 객체의 포장을 제거한 뒤 파라미터로 넘겨주는게 맞을까요?
저는 이러한 단점때문에 DTO를 사용해야 한다는 입장입니다. 1. MVC 각 계층 간의 통신 매체를 단일화하면서, 2. 불필요한 정보를 보내지 않을 수 있습니다. 서브웨이는 어떻게 생각하시는지 궁금합니다! 현 시점에서 DTO를 사용하는 건 오버엔지니어링일까요?
TDD를 처음 접해서 부족한 점이 많습니다. 마음껏 지적해주시면 감사하겠습니다 🙇♂️