-
Notifications
You must be signed in to change notification settings - Fork 12
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
[사다리 타기] 김나윤 미션 제출합니다. #11
base: bbggr1209
Are you sure you want to change the base?
Conversation
bbggr1209
player 객체
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.
안녕하세요, 나윤! 👋
중간고사 끝나고 미션하느라 고생 많았습니다!
테스트 단위를 어느 정도 범위까지 작성하는 것이 적절할까요?
결론부터 말씀드리자면 테스트 범위는 개발자의 선택에 달렸습니다. 즉, 정답은 없습니다! 내가 무엇을 테스트하고 싶은지, 내가 어디까지 테스트해야 내 코드를 신뢰할 수 있는지 같은 기준을 세우고 테스트를 작성하면 자연스럽게 나만의 테스트 범위가 생깁니다. 저는 주로, 도메인과 비즈니스 로직, 예외 발생은 무조건 테스트를 작성합니다.
현재 저는 우테코 레벨 2에서는 Spring을 학습하고 있습니다. Spring에는 도메인 뿐만 아니라 Controller, Service, Repository 같은 Layered Architecture 객체들도 있습니다.(나윤도 잘 알다시피) 이때는 더욱 어디까지 테스트해야 할지 감이 잘 잡히지 않을 수 있습니다. 그래서 나만의 테스트 기준을 세우고 기준에 맞게 테스트가 잘 작성 되었는지 여러번 검토해야 합니다.
물론, 테스트 범위에 정답은 없으나 오답은 있습니다. 아예 테스트를 작성하지 않거나, 테스트하기 어렵다는 이유로 도메인 생성 로직이나 서비스의 핵심 비즈니스 로직을 테스트하지 않으면 결코 좋은 테스트라고 할 수 없습니다. 기본적으로 나윤이 구현하고자 하는 기능은 모두 테스트를 하는 것이 좋고, Java가 구현 해준 표준 입출력과 랜덤 값 생성 같은 기능은 따로 테스트하지 않아도 됩니다.
쉽게 말해서 언어에 대한 도전(테스트 작성)은 무의미하다는 말입니다. 나윤이 짠 코드를 테스트하는 것만으로 충분합니다. 이번 미션에서는 주어진 기능 요구 사항을 모두 만족했는가를 기준으로 테스트 범위 기준을 세워보면 좋겠네요. 리팩토링 파이팅!
p.s. 테스트 커버리지를 맹신하는 것은 좋지 않지만 참고 지표는 될 수 있습니다! 전 도메인 패키지의 클래스들은 메소드 커버리지가 합당한 사유가 없는 한, 100% 달성을 목표로 놓친 테스트가 없는지 검토합니다.
## 구현할 기능 목록 | ||
|
||
- [x] 플레이어 이름 입력 | ||
- [x] 이름을 입력 받을 메세지 출력 | ||
- [x] 이름을 입력 받음 | ||
- [x] 이름을 쉼표(,)를 기준으로 분리 | ||
- [x] 이름의 유효성 검사 | ||
- [x] 이름이 5글자 이하여야 한다. | ||
- [x] 이름에는 공백이 들어갈 수 없다. | ||
- [x] 이름이 중복되어서는 안된다. | ||
|
||
- [x] 사다리 높이 입력 | ||
- [x] 사다리 높이 입력 메세지 출력 | ||
- [x] 사다리 높이 입력 받음 | ||
- [x] 사다리 높이의 유효성 검사 | ||
- [x] 사다리 높이는 1 이상이어야 한다. | ||
- [x] 사다리 높이는 0~9 사이의 숫자여야 한다. | ||
|
||
- [x] 결과 출력 | ||
- [x] 플레이어 이름 출력 | ||
- [x] 사다리 출력 | ||
|
||
--- | ||
|
||
## 필요한 객체 | ||
|
||
### Player | ||
- 이름을 입력받고 유효성 검사를 수행하는 객체 | ||
|
||
### Players | ||
- Player 객체를 관리하는 객체 | ||
|
||
### Line | ||
- 사다리의 선을 관리하는 객체 | ||
|
||
### Point | ||
- 선의 시작점과 끝점을 관리하는 객체 | ||
|
||
### Line Generator | ||
- Line을 랜덤으로 생성하는 객체 | ||
|
||
### Ladder | ||
- 여러 개의 선 객체를 관리하는 객체 | ||
|
||
### 입출력 | ||
- 입력을 받고 출력하는 객체 |
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 Ladder { | ||
|
||
private final List<Line> lines; | ||
|
||
public Ladder(int numberOfPlayers, int height) { | ||
lines = new ArrayList<>(); | ||
for (int i = 0; i < height; i++) { | ||
lines.add(LineGenerator.generateLine(numberOfPlayers)); | ||
} | ||
} | ||
|
||
public List<Line> getLines() { | ||
return lines; | ||
} | ||
|
||
} |
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 List<String> getPlayerNames() { | ||
return players.stream() | ||
.map(Player::getName) | ||
.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.
toList()
만 사용해도 List
전환 가능합니다!
public static Line generateLine(int numberOfPlayers) { | ||
List<Point> points = new ArrayList<>(); | ||
for (int i = 0; i < numberOfPlayers - 1; i++) { | ||
boolean next = random.nextBoolean() && !previous; |
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 static void printLadder(List<Line> ladder, List<String> playerNames) { | ||
System.out.println("실행 결과"); | ||
printPlayerNames(playerNames); | ||
for (Line line : ladder) { | ||
printLine(line); | ||
} | ||
} |
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
라는 일급 컬렉션을 구현했음에도 List<Line>
으로 매개변수를 전달한 이유가 있을까요?
또한, View에 직접 Domain을 전달했을 때는 어떤 문제가 일어날 수 있나요?
package model; | ||
|
||
public class Player { | ||
private final static int MAX_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.
Java Language Specification에 따르면 private static final
이 더 올바른 순서입니다!
@Test | ||
@DisplayName("사다리의 높이를 확인합니다.") | ||
void ladderHeight() { | ||
// given | ||
int numberOfPlayers = 4; | ||
int height = 5; | ||
|
||
// when | ||
Ladder ladder = new Ladder(numberOfPlayers, height); | ||
List<Line> lines = ladder.getLines(); | ||
|
||
// then | ||
assertEquals(height, lines.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.
해당 테스트는 "사다리의 높이를 확인합니다."라기보단 "주어진 높이에 따라 사다리를 생성한다."가 더 어울릴 것 같습니다.
테스트 코드가 일종의 기능 명세의 역할도 한다는 것을 생각하면 @DisplayName
은 코드 이해를 위한 중요한 단서가 됩니다.
따라서, 모호한 표현보다는 좀 더 확실하게 해당 테스트가 어떤 기능을 테스트하고 있는지 명시해줘야 합니다.
@Test | ||
@DisplayName("라인을 생성합니다.") | ||
void createLine() { | ||
List<Point> points = List.of(new Point(true), new Point(false), new Point(true)); | ||
Line line = new Line(points); | ||
|
||
assertThat(line.getPoints()).isEqualTo(points); | ||
} |
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.
사다리 생성 시, 랜덤한 값의 영향을 받는 부분을 외부에서 주입하도록 해결했네요! 👍
랜덤 때문에 테스트하기 어려웠을 것 같은데 잘 하셨습니다!
@Test | ||
@DisplayName("플레이어의 이름이 5글자를 초과하면 예외가 발생합니다.") | ||
void validateNameLength() { | ||
// given | ||
String name = "pobibibibi"; | ||
|
||
// when & then | ||
assertThatThrownBy(() -> new Player(name)) | ||
.isInstanceOf(IllegalArgumentException.class); | ||
} |
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.
예외에 대한 테스트 👍
테스트에서 가장 중요한 것은 해피 케이스보다도 경계값 테스트를 통한 예외 발생 여부입니다.
아쉽게도 테스트는 경계값 테스트까진 아닙니다. 경계값 테스트를 작성해봅시다!
어려웠던 점
궁금한 점