Conversation
seondongpyo
left a comment
seondongpyo
left a comment
There was a problem hiding this comment.
2단계와 3단계를 같이 진행해주셨네요!
다만 하나의 커밋에 너무 많은 변경사항이 포함되어 있는 것으로 보여요 😢
이번 과정의 주요 학습 목표가 TDD인 만큼, TDD 사이클에 따라 기능을 하나씩 구현해가면서
커밋 단위를 작게 가져가는 습관을 들여보시는 걸 추천 드립니다.
몇 가지 코멘트 드렸는데, 확인 후 다시 리뷰 요청을 부탁 드리겠습니다 🔥
|
|
||
| public class LadderApplication { | ||
|
|
||
| public static void main(String[] args) { |
There was a problem hiding this comment.
메서드 라인 수가 길어 보이는데, 자동차 경주 미션 때처럼 메서드의 라인 수를 줄여보면 어떨까요?
이 같은 원칙 아래에서 메소드의 라인 수를 15라인이 넘지 않도록 구현한다.
| if (name.equals("all")) { | ||
| OutputView.announceAllUsers(usersAfter, ladderResults); | ||
| return; | ||
| } | ||
| OutputView.announceUser(name, usersAfter, ladderResults); |
|
|
||
| @FunctionalInterface | ||
| public interface BridgeStrategy { | ||
| Boolean bridgeBuild(); |
There was a problem hiding this comment.
- wrapper 클래스를 사용하신 이유가 있으실까요?
BridgeStrategy라는 이름을 통해 Bridge를 생성하는 전략임을 유추할 수 있기 때문에,
메서드 이름은build()정도로 지어도 충분히 의미를 전달할 수 있을 것 같습니다.
| public static Ladder createLadder(int countOfUsers, int height, BridgeStrategy strategy) { | ||
| return new Ladder(countOfUsers, height, strategy); | ||
| } | ||
|
|
||
| private Ladder(int countOfUsers, int height, BridgeStrategy strategy) { | ||
| for (int i = 0; i < height; i++) { | ||
| Line line = Line.createLine(countOfUsers, strategy); | ||
| lines.add(line); | ||
| } | ||
| } |
There was a problem hiding this comment.
다음 클래스 작성 순서 가이드를 참고해보시면 좋을 것 같습니다.
https://www.oracle.com/java/technologies/javase/codeconventions-fileorganization.html
| return strategy.bridgeBuild(); | ||
| } | ||
|
|
||
| private Boolean decide(Optional<Integer> prevIndex, Boolean expected) { |
There was a problem hiding this comment.
매개변수의 타입으로 Optional이 사용되고 있는데, 다음 링크를 참고해보시면 좋을 것 같습니다.
https://yeonyeon.tistory.com/224
| private static final Scanner scanner = new Scanner(in); | ||
| public static final String ASKING_NAMES = "참여할 사람 이름을 입력하세요. (이름은 쉼표(,)로 구분하세요)"; |
There was a problem hiding this comment.
접근 제어자 순서는 public부터 private 순으로 작성해보면 좋을 것 같아요.
https://www.oracle.com/java/technologies/javase/codeconventions-fileorganization.html
| public static List<String> inputUserNames() { | ||
| System.out.println(ASKING_NAMES); | ||
| String next = scanner.next(); | ||
| return Arrays.stream(next.split(",")) |
There was a problem hiding this comment.
문자 ,은 상수로 선언하여 의미를 부여해보면 어떨까요? 아래 32라인에서도 재활용이 가능할 것 같습니다.
|
|
||
| public class OutputView { | ||
|
|
||
| public static void display(Users users, Ladder ladder, LadderResults ladderResults) { |
There was a problem hiding this comment.
참여자 이름과 실행 결과를 출발 지점과 도착 지점 위치에 맞게 출력하여 가독성을 높여보면 어떨까요?
| public class OutputView { | ||
|
|
||
| public static void display(Users users, Ladder ladder, LadderResults ladderResults) { | ||
| System.out.println("사다리 결과\n"); |
There was a problem hiding this comment.
플랫폼에 독립적인 줄바꿈 문자를 사용해보면 어떨까요?
https://www.baeldung.com/java-string-newline#2-using-platform-independent-line-separators
| assertThat(ladder.findLastPosition(3)).isEqualTo(2); | ||
| assertThat(ladder.findLastPosition(4)).isEqualTo(4); | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
파일의 끝에는 개행을 추가해보면 어떨까요?
https://velog.io/@doondoony/posix-eol
3 participants
사다리 생성 & 게임실행 관련 PR입니다.