Step2 - 사다리(생성) #1438
Step2 - 사다리(생성) #1438
Conversation
|
|
||
| import java.util.Scanner; | ||
|
|
||
| public class LadderScanner { |
There was a problem hiding this comment.
LadderScanner 가 하는 일을 InputView에서 해도 좋을 것 같은데 별도의 클래스로 분리해주신 이유가 있나요?
| import java.util.Scanner; | ||
|
|
||
| public class LadderScanner { | ||
| public static Scanner scanner = new Scanner(System.in); |
There was a problem hiding this comment.
자바 상수 컨벤션에 맞게 수정해볼까요?
접근제한자는 private이 적절하지 않을까요?
| public class Ladder { | ||
| private List<Line> lines = new ArrayList<>(); | ||
|
|
||
| public Ladder(int width, int height) { |
There was a problem hiding this comment.
width, height은 1이상 이어야 하지 않나요?
검증이 필요하지 않을까요?
| import java.util.ArrayList; | ||
| import java.util.List; | ||
|
|
||
| public class Line { |
| this.points = points; | ||
| } | ||
|
|
||
| static Line of(int size) { |
There was a problem hiding this comment.
정적 팩토리 메소드를 사용해주셨네요 👍👍
추가적으로 전체적으로 접근제한자 default, public 를 혼용해서 사용하셨는데 이유가 있을까요?? :)
| List<String> participants = List.of(input.split(",")); | ||
| int height = LadderScanner.insertLadderHeight(); | ||
|
|
||
| Ladder ladder = new Ladder(participants.size() - 1, height); |
There was a problem hiding this comment.
participants.size() - 1 에서 -1 은 도메인 로직이라고 생각되는데요.
해당 로직은 도메인으로 이동시키는건 어떨까요?
| } | ||
|
|
||
| Point nextPoint() { | ||
| if (isExist()) { |
| private final boolean exist; | ||
|
|
||
| Point() { | ||
| this(new Random().nextBoolean()); |
There was a problem hiding this comment.
랜덤한 값이 있다면 테스트할 수 없고
Point를 테스트할 수 없으니 하니 상위 도메인 객체인 Line과 Ladder도 테스트할 수 없는 구조인데요 🥲
전략 패턴을 사용해서 테스트 가능한 구조로 변경해보는건 어떨까요? :)
| participants.forEach(it -> { | ||
| it = " " + it; | ||
| it = it.substring(it.length() - 6); | ||
| System.out.print(it); | ||
| }); |
There was a problem hiding this comment.
스트림을 활용해주셨네요 👍
| participants.forEach(it -> { | |
| it = " " + it; | |
| it = it.substring(it.length() - 6); | |
| System.out.print(it); | |
| }); | |
| participants.forEach(ResultView::printParticipant); |
11 ~ 13 라인을 메소드로 추출한다면 좀 더 간결한 코드를 작성할 수 있을 것 같아요 :)
|
|
||
| private static void printLine(List<Point> points) { | ||
| System.out.print(" "); | ||
| points.forEach(it -> { |
There was a problem hiding this comment.
it 대신 적절하게 point 로 네이밍을 변경하는 것도 좋을 것 같아요 :)
안녕하세요! step2 리뷰 요청드립니다
요구사항에 "자바 8의 스트림과 람다를 적용해 프로그래밍한다."가 있어서 고민하다가 결과출력 부분에 적용해보았습니다...🥲
step1에서 머지할 때 코멘트 주셨던 부분도 수정하였습니다!