-
Notifications
You must be signed in to change notification settings - Fork 53
[사다리] 최보현 미션 제출합니다. #64
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
base: main
Are you sure you want to change the base?
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.
보현님 그래도 미션 최대한 진행해주셔서 감사해요!
감단히 리뷰 남겨두었습니다!
다음 미션 진행 전에는 반영해보시고 진행해보면 좋을 것 같아요 홧팅🚀
public class Size { | ||
|
||
private final int height; | ||
private final int width; | ||
private static final String LADDER_SIZE_INVALID_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.
height와 width를 관리하는 Size 객체를 만들어주셨군요👍
멤버변수와 상수의 선언 순서를 신경써주시면 좋을 것 같네요!
} | ||
|
||
private void validate(int height, int width) { | ||
if (height < 1 || width < 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.
매직넘버는 상수로 관리해보면 좋을 것 같아요!
public class LadderService { | ||
private final LadderGenerator generator; | ||
private final Size size; | ||
|
||
public LadderService(Size size) { | ||
this.generator = new LadderGenerator(); | ||
this.size = size; | ||
} |
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를 생성하는 LadderService를 만들어주셨군요!
이름을 LadderService로 지어주신 이유가 궁금해요!
추가적으로 LadderGenerator, Size를 파라미터로 받는 생성자를 만들고,
Size만 파라미터로 받는 정적 팩토리 메서드를 만들어 사다리를 자동으로 만든다는 의미를 나타내보면 어떨까요?
public class Ladder { | ||
private final List<Line> lines; | ||
private final int width; |
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.
Size를 객체로 관리하고 있으니, Size 자체를 필드로 갖게해보면 어떨까요?
지금과 같은 구조라면 Ladder의 사이즈에 대한 검증을 추가적으로 해야할걸로 보여요.
@@ -0,0 +1,21 @@ | |||
package domain; |
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.
domain 패키지 내에, model 또는 service 패키지로 분리된 클래스들도 있고, 따로 분리되지 않은 클래스들(ex. Line)도 있는데요.
이렇게 구분하신 이유가 궁금해요!
안녕하세요 엘리님 !
정말 오래걸렸는데도 많은 단계를 완성하지 못했네요.
자바 마지막 미션을 진행하며 느낀 점을 작성해 보겠습니다.
1. 도메인에 대한 이해
처음에는 사다리 전체 구조를 하나의 객체로 생각해서 게임의 결과를 어떻게 도출하는지에 대해 결론을 내리지 못해서 고민하는 시간이 길어졌습니다. 다른 분들의 pr을 참고하고,
Line
이 단지 한 행의 좌우 연결만 담고 있다는 점을 이해하고 나서부터 흐름이 단순화됐습니다. 사다리는 위에서 아래로, 각 행(Line)마다 좌우 이동 여부만 판단하며 내려오는 구조라는 것을 깨닫게 되었습니다. 향후 단계에서 도착 지점을 구하는 로직이나, 결과 매핑을 구현할 때 각 좌표 위치가 더 중요해질 것으로 생각하는데 이 부분은 더 고민해보겠습니다.2. domain 패키지 분리
이전 미션에는 domain 패키지에 모든 클래스들을 분리 없이 두었습니다. 이번에는 enum인
LadderSymbol
은 사다리 구조를 구성하는 도메인 요소라 생각해서 model 패키지로 분리하고,LadderGenerator
는 사다리 라인과 연결 정보를 생성하는 책임을 갖기 때문에 도메인 객체와 분리하여 service 패키지로,LadderService
는 사용자 입력기반(Size) 으로 사다리 생성하는 책임을 갖기 때문에 도메인 객체와 분리해 마찬가지로 service 패키지로 분리했습니다.3. 개선점
현재까지 테스트 코드를 충분히 작성하지 못했습니다. 각 도메인 객체(Line, Ladder, LadderGenerator)에 대한 단위 테스트를 추가하겠습니다.
또한 OutputView에 직접적으로 사다리 출력 형식을 포함하고 있어 향후 출력 형식을 변경할 경우 수정 범위가 커질 수 있습니다. 출력 포맷을 분리하는 방법을 고민해보겠습니다.
감사합니다!