[사다리 - 4단계] 정민주 미션 제출합니다. #25
Conversation
src/main/java/domain/Line.java
Outdated
| private final String name; | ||
| private final String outcome; | ||
|
|
There was a problem hiding this comment.
해당 부분은 player 객체를 따로 만들어서 관리를 해야할 것 같습니다!
현재 line의 책임이 큰 것 같습니다! (현재 요구사항 중 이름 글자수 검증은 따로 코드에 구현이 안된 것 같은데 이는 사다리의 요소인 line이 책임져야할 부분은 아닌 것 같습니다)
There was a problem hiding this comment.
아 이름 조건을 확인못했습니다! 구현하도록 하겠습니다
| List<String> names = List.of("일", "이", "삼"); | ||
| List<String> outcomes = List.of("1000", "2000", "3000"); | ||
| // when | ||
| final CountOfLine countOfLine = laddersService.getcountOfLine(names, outcomes); |
There was a problem hiding this comment.
혹시 final을 붙이고 안 붙이고의 차이는 어떤 이유인가요?
There was a problem hiding this comment.
단축키로 자동완성할 때 final이 붙어서 몇군데에만 붙었습니다..! 통일하도록 하겠습니당
| List<String> names = getNames(); | ||
| List<String> outcomes = getOutcomes(); | ||
| CountOfLine countOfLine = laddersService.getcountOfLine(names, outcomes); | ||
| Height height = getHeight(); | ||
|
|
||
| Ladder ladder = laddersService.createLadder(countOfLine, height); | ||
| outputView.printStatusOfLadders(ladder.getRightRungStatus(), height.value()); | ||
| outputView.printResult(ladder.getResult()); | ||
| Ladder ladder = laddersService.createLadder(countOfLine, height, names, outcomes); |
There was a problem hiding this comment.
현재 사다리를 생성하는 과정에서 countOfLine, heigth, names과 outcomes를 인자로 받고 있는 상태입니다.
countOfLine은 names과 outcomes에서 파생되는 결과로 보입니다. (이는 사다리를 생성하는 과정에서 필요한 과정으로 보입니다)
countOfLine을 생성하는 과정을 굳이 controller라는 외부에서 드러낼 필요 없이 createLadder의 내부 과정으로 넣는 것이 더 사다리 생성이라는 기능관점에서 볼 때에서 응집도가 높을 것 같습니다.
| private boolean isAllMode(String targetName) { | ||
| return targetName.equals("all"); | ||
| } |
There was a problem hiding this comment.
targetName에 null이 들어오는 경우 NullPointerException이 발생하기에 "all".equals(targetName)으로 하는 것이 null safe합니다.
| if (isAllMode(targetName)) { | ||
| return result; | ||
| } |
There was a problem hiding this comment.
해당 result를 View에 제공할 때 방어적 복사를 해주는 것이 안전할 것 같습니다. (외부의 변화가 내부의 변화로 이어지지 않습니다)
There was a problem hiding this comment.
민주님이 리뷰 반영하신 것 모두 확인했습니다.
코드가 동작하는 것에는 큰 문제가 없고 다음 단계가 리팩토링 단계인 만큼 마지막 리뷰는 설계에 대해서 이야기를 나누어 보고자 합니다.
현재 이는 민주님의 객체 및 패키지 간의 의존도 입니다. (전체를 나타낸 것은 아니고 service와 domain 간을 중심으로 나타냈습니다.)
LadderService는 크게 사다리를 생성하는 것과 사다리 결과를 생성하는 것을 책임지고 있습니다. (실선은 직접 의존이고 점선은 간접 의존입니다.) 설계 부분은 정답이 없고 개인적인 성향을 많이 반영하지만 그럼에도 제가 느낀 민주님의 설계에 대한 제 피드백을 남기려고 합니다.
사다리를 생성하는 것
노란색 부분은 현재 사다리를 생성하기 위해서 LadderService가 직접 혹은 간접적으로 의존을 맺고 있는 부분입니다. 이 부분에서 보면 알 수 있듯이 사다리가 생성하기 위한 정보들이 모두 외부에서 작업되고 정작 사다리(Ladder)는 자신이 생성되는 과정에서는 협력이 이루어지지 않고 있음을 볼 수 있습니다. 제가 느끼기에 Ladder는 현재 생성과정에서 자율성이 떨어지고 수동적인 객체로 느껴집니다.
물론 이 부분은 사다리를 생성하는 과정이 복잡해서 이를 따로 분리해서 처리한 것을 수 있습니다.
사다리 결과를 생성하는 것
보라색 부분이 사다리 결과를 만들어내는 과정에서 간접적으로 의존하고 있는 부분입니다. 현재 민주님이 설계하신 도메인을 보면 Ladder는 line에 의존하고 line은 player에 의존하고 있습니다. 즉, player 할 수 있는 작업은 line에서도 할 수 있고 line이 할 수 있는 작업은 Ladder에서도 할 수 있습니다. 결과적으로 ladder를 통해서 사다리를 타는 것이 충분히 가능하다는 것을 의미합니다.
하지만 현재 사다리의 결과를 만들어내는 코드를 보면 service에서 line, player를 불러오면서 Ladder에서 메세지를 던지지 않고 절차적으로 코드가 동작하는 것 같습니다.
결과적으로 현재 Ladder 객체의 자율성이 떨어지는 것으로 느껴지고 있습니다. 이 부분은 5단계를 진행해 보면서 개선해 나가는 것이 좋을 것 같습니다!
"이름"과 "결과"를 담을 수 있는 name과 outcome 변수를 추가했습니다!
확인부탁드리겠습니다. 감사합니다 :)