Conversation
jinyoungchoi95
left a comment
jinyoungchoi95
left a comment
There was a problem hiding this comment.
안녕하세요 예슬님 😄
몇가지 코멘트 남겨두었으니 확인부탁드려요 :)
궁금하거나 고민이 되는 부분이 있으시다면 언제든 pr 코멘트 또는 dm으로 요청 부탁드립니다.
감사합니다 🙇♂️
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| public class Players { |
|
|
||
| public static Point create(boolean previousHasLine) { | ||
| if (previousHasLine) return new Point(false); | ||
| return new Point(Math.random() > 0.5); |
There was a problem hiding this comment.
Math.random()으로 인해서 테스트하기 힘든 구조가 될 것 같아요! 자동차때처럼 테스트가능하도록 해보면 어떨까요?
|
|
||
| public Ladder(int height, int countOfPlayers) { | ||
| lines = new ArrayList<>(); | ||
| IntStream.range(0, height) |
There was a problem hiding this comment.
stream의 종단 연산은 안티패턴입니다 😄
lines = new ArrayList<>()로 별도 선언하고 추가하기보다는 map으로 바로 만들어 할당해볼 수 있을 것 같아요 :)
| @@ -0,0 +1,4 @@ | |||
| package ladder.view.input; | |||
|
|
|||
| public abstract class BaseInputView implements InputView { | |||
There was a problem hiding this comment.
후에 리팩토링을 위해 만들어 둔 것입니다.
동일한 로직이 있을 경우 이곳에 추가하기 위해 만들어 두었습니다.
| } | ||
|
|
||
| @Test | ||
| @DisplayName("참여자의 이름은 쉼표가 아닌 다른 구분자가 사용되면 예외가 발생한다.") |
There was a problem hiding this comment.
Players테스트가 여기있었네요. 테스트의 경우에는 클래스별로 만들어주셔야 어떤 테스트를 하려고하는지가 조금 더 명시적일 것 같아요. 클래스마다 테스트를 분리해보면 좋겠네요 😄
| @Test | ||
| @DisplayName("참여자의 이름은 쉼표가 아닌 다른 구분자가 사용되면 예외가 발생한다.") | ||
| void getSplitter() { | ||
| FakeInputView inputView = new FakeInputView("yeseul,pobi|crong"); |
There was a problem hiding this comment.
new Players("yeseul,pobi|crong")로도 충분히 테스트가 되지 않을까요? 단위테스트는 최대한 불필요한 다른 의존성은 지우고 테스트하고자하는 대상에 집중해보면 좋을 것 같아요!
2 participants
안녕하세요!
오랜만에 인사드립니다 ... :)
이번 미션은 사다리 타기 게임의 생성(?)과 관련된 기능을 구현하기였습니다.
도메인 패키지는 사용자와 사다리 두 가지로 나누어 설계해보았습니다.
테스트 코드도 작성해보려 했지만, 사용자 입력에 대한 검증 정도만 구현되어 있어 아쉬움이 남습니다.
이번 리뷰도 잘 부탁드립니다!