-
Notifications
You must be signed in to change notification settings - Fork 748
Step2 #2378
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: kjy2844
Are you sure you want to change the base?
Step2 #2378
Conversation
- Conditional -> Predicate
public Ladder(int height, int width) { | ||
for (int i = 0; i < height; i++) { | ||
ladderRows.add(new LadderRow(width, () -> RANDOM.nextBoolean())); | ||
} | ||
} |
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.
이러면 여기에 대한 테스트가 불가할것으로 보입니다.
인터페이스를 통한 generator를 만들어서 인자로 넣는 방식을 이용하면 어떨까여?
팩터리 메서드가 나은거 같습니다!
당연히 collection이라면 forEach가 있는게 어색하지 않습닝다. getter를 만들지 않는 방법이라면 나쁘지 않을것으로 판단됩니다. |
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.
간단한 구조에 대한 리뷰 남겼습니다! 확인해보시면 좋을거 같습니다.
언제든지 궁금한게 있다면 DM주세요!
throw new IllegalArgumentException("이름은 필수입니다."); | ||
} | ||
|
||
if (name.length() > 5) { |
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.
상수를 통해 의미 전달을 해보시면 좋겠습니다.
import static org.assertj.core.api.Assertions.assertThat; | ||
import static org.assertj.core.api.Assertions.assertThatThrownBy; | ||
|
||
public class PlayerTest { |
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 Players(List<String> playerNames) { | ||
if (playerNames == null) { | ||
throw new IllegalArgumentException("플레이어가 존재하지 않습니다."); | ||
} | ||
this.players = playerNames.stream() | ||
.map(Player::new) | ||
.collect(Collectors.toList()); | ||
} |
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.
여기 테스트가 미흡합니다.
질문
사다리 게임 기능 목록