Skip to content
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

[2단계 - 사다리 생성] 제이미(임정수) 미션 제출합니다. #201

Merged
merged 42 commits into from
Feb 27, 2023
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
0faa657
docs: 요구사항 및 기능 목록 추가
JJ503 Feb 22, 2023
c337052
feat: 결과 기능 추가
JJ503 Feb 22, 2023
f417b0d
feat: 결과들 기능 추가
JJ503 Feb 22, 2023
00aac30
feat: 사다리 게임 전체 결과 인덱스 가져오기 기능 추가
JJ503 Feb 22, 2023
acfe36e
rename: 파일명 오타로 인한 수정
JJ503 Feb 22, 2023
82f126d
feat: 사다리 게임의 결과 판단 기능 추가
JJ503 Feb 22, 2023
da00181
feat: 요청 값에 따른 결과 반환 기능 추가
JJ503 Feb 22, 2023
1f79bc4
fix: 출력 포맷 수정
JJ503 Feb 22, 2023
1043538
feat: 결과 입력 및 출력 기능 추가
JJ503 Feb 22, 2023
85d183c
refactor: GameMap 제거 및 Ladder로 기능 전달
JJ503 Feb 22, 2023
1b76f30
refactor: 메서드 명으로 변경
JJ503 Feb 22, 2023
90bd727
refactor: 사용하지 않는 패키지 제거
JJ503 Feb 22, 2023
6df7c53
refactor: 메서드 및 변수명 변경
JJ503 Feb 22, 2023
5203156
feat: 보고 싶은 사람 입력 기능 추가
JJ503 Feb 23, 2023
c94305b
feat: 보고 싶은 사람의 실행 결과 출력 기능 추가
JJ503 Feb 23, 2023
de78b60
refactor: 사용하지 않는 패키지 제거
JJ503 Feb 23, 2023
0d3c367
fix: 기능 추가에 따른 입출력 수정
JJ503 Feb 23, 2023
1107fff
refactor: private 생성자 추가
JJ503 Feb 23, 2023
3d21b32
refactor: outputview에서 domain을 파라미터로 받지 않도록 수정
JJ503 Feb 23, 2023
93a8476
refactor: 게임 결과 도메인 명 변경
JJ503 Feb 23, 2023
765f14c
feat: 게임을 실행하는 도메인 추가
JJ503 Feb 23, 2023
c0be9a4
refactor: 이름과 결과 입력 시 양옆의 공백 허용
JJ503 Feb 23, 2023
f1ddbef
refactor: 컨벤션 적용
JJ503 Feb 23, 2023
7148ab8
rename: ErrorMessage 경로 수정
JJ503 Feb 23, 2023
cf546ca
docs: 요구사항 완료 체크
JJ503 Feb 23, 2023
cf0409e
fix: 로직 문제에 대한 수정
JJ503 Feb 24, 2023
a6065ca
refactor: 라인 상태에 대한 객체 추가
JJ503 Feb 26, 2023
2eb1d2a
rename: view 내부 패키지 제거
JJ503 Feb 26, 2023
1a15d38
refactor: 파라미터 변수명 수정
JJ503 Feb 26, 2023
3cf3a5b
refactor: 컨트롤러의 생성자 추가
JJ503 Feb 27, 2023
615707c
refactor: 예외 메시지 출력 기능 outputView로 이동
JJ503 Feb 27, 2023
4ba26ca
refactor: 재입력 인터페이스 생성 및 적용
JJ503 Feb 27, 2023
90eb49c
refactor: 사용하지 않는 메서드 제거
JJ503 Feb 27, 2023
81551b1
refactor: 변수명을 명확하게 수정
JJ503 Feb 27, 2023
313e459
refactor: 객체 비교 기능 추가
JJ503 Feb 27, 2023
facbee6
feat: TestGenerator 추가
JJ503 Feb 27, 2023
4caaff5
docs: 네이밍 관련 수정
JJ503 Feb 27, 2023
32cf4e8
docs: 완료 기능 체크
JJ503 Feb 27, 2023
0815728
rename: readme 파일 합침
JJ503 Feb 27, 2023
5281476
fix: 잘못된 조건 수정
JJ503 Feb 27, 2023
0a4a092
refator: 사용하지 않는 패키지 제거
JJ503 Feb 27, 2023
6a8768f
feat: List 미션 수행
JJ503 Feb 27, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
46 changes: 39 additions & 7 deletions docs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@

## 📚 도메인 모델 네이밍 사전

| 한글명 | 영문명 | 설명 |
|-------|--------------|---------------------|
| 참가자 | Participants | 사다리 게임에 참가하는 사람들을 지칭 |
| 사람 | Person | 사람을 지칭 |
| 맵 | Map | 맵 정보 지칭 |
| 사다리 | Ladder | 사다리 지칭 |
| 라인 | Line | 사다리의 한줄을 지칭 |
| 한글명 | 영문명 | 설명 |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

도메인을 OutputView에서 출력할 때 필요한 형태로 변환해 주는 기능에 대한 역할 부여를 어디에 해야 할지 고민인 상태입니다.
처음에는 도메인 자체에서 값을 반환할 때 변환해 전달해 주었습니다.
하지만, 해당 기능은 단지 출력해 주기 위한 기능이라는 생각이 들어 도메인에서는 값을 처리 없이 반환하도록 수정했습니다.
그리고 게임 전체를 관리하는 도메인을 생성해 해당 도메인에서 모든 값 변환을 수행해 주었습니다.
그런데 제가 역할을 제대로 부여했는지 잘 모르겠습니다.

넵, 잘해주셨어요! Controller가 말 그대로 뷰와 모델의 중간에서 컨트롤하는 역할이라고 보시면 됩니다. (링크)

또한, 1단계 리뷰 중 “OutputView가 도메인을 모른다면 훨씬 유연해질 수 있을 것 같은데요." 라는 의견을 주셨는데,
제가 수행한 방식이 생각한 방식이 맞는지 궁금합니다.

네 맞습니다! 👍 OutputView가 도메인을 아예 모르는 상태가 되면 도메인을 의존하지 않고 확장가능하다는 이야기를 하고싶었습니다.

기존 제 코드에서는 Height과 Weight을 하나의 도메인에서 사용했었습니다.
하지만, 이번에 코드가 변경되며 Height과 Weight을 사용은 하지만, 굳이 저장까지는 필요 없어진 상태입니다.
일단 검증 로직을 위해 각각의 도메인을 남겨 두었는데, 저장까지 필요가 없으니 validator 클래스를 생성해 검증만 수행하는 클래스로 변경해 주는 게 좋을지 궁금합니다.

현재도 충분히 객체지향에 맞는 코드라고 생각합니다. 굳이 Validator 클래스로 빼지 않아도 될 것 같아요. (빼도 좋고 안 빼도 좋습니다)

보고 싶은 사람을 입력할 때 사람의 이름 혹은 ‘all’이라는 문자열을 입력하게 됩니다.
현재 저는 한 사람에 대한 결과를 원하는지 전체 결과를 원하는지에 대한 판단을 컨트롤러에서 수행하고 있습니다.
그런데 과연 해당 판단을 수행하는 것이 컨트롤러의 역할이 맞는지에 대한 의문이 들고 있습니다.
해당 판단에 대해선 어느 위치에서 수행하는 것이 이상적일지 궁금합니다.

Controller의 역할이 커지는 것 같지만 현재 미션에서는 Controller에서 해도 괜찮다고 생각합니다. 다른 입력을 통해 결과물을 달리 준다는 역할을 분리한 하나의 클래스에서 진행하게 된다면 DispatcherServlet가 하는 역할과 비슷할 것 같은데요. DispatcherServlet처럼 다른 클래스로 또 나눌 수도 있을 것 같지만 현재 미션에서 적용하기보단 추후 웹 미션에서 적용해보면 좋을 것 같습니다.

도메인 중 GameResult는 List 형태의 값을 저장하고 있습니다.
해당 도메인에 대한 테스트로 객체를 제대로 생성했다는 테스트 및 값을 제대로 가져오는지에 대한 수행을 하려고 합니다.
그런데 해당 도메인에서 getter 수행 시 List 형태로 바로 반환하도록 했습니다.
그런데 값을 제대로 가져오는지 확인하기 위해선 Person 객체로 검사해야 하는데 new Person()을 수행하더라도 이는 다른 객체로 인식되기에 검증이 어려운 상태입니다.
이러한 경우 어떻게 검증할 수 있는지 궁금합니다.

GameResult는 Map 형태로 저장하고 있는 것 같은데요. GameResult가 맞을까요? 다른 객체로 인식되는 부분은 아래 답변처럼 equals와 hashcode를 재정의하면 해결되는 문제같습니다. 현재 상황에서는 재정의해도 된다고 생각되고요.

또한, Results에 대한 테스트를 수행할 때 역시 값이 제대로 저장되는지 확인하려고 합니다.
이 역시 값이 아닌 객체를 반환하는 메서드이기에 아래와 같이 검증을 수행했습니다.
그런데 이와 같이 Result 도메인에 대한 메서드를 수행하면서 테스트를 해도 무관한 것인지 궁금합니다.

Result 자체도 하나의 value로서 특정될 수 있는 것 같은데요. equals와 hashcode를 재정의하면될 것 같습니다.

|-----|--------------|----------------------|
| 참가자 | Participants | 사다리 게임에 참가하는 사람들을 지칭 |
| 사람 | Person | 사람을 지칭 |
| 맵 | Map | 맵 정보 지칭 |
| 사다리 | Ladder | 사다리 지칭 |
| 라인 | Line | 사다리의 한줄을 지칭 |
| 결과 | Result | 실행 결과를 지칭 |

<br>

Expand All @@ -28,6 +29,12 @@
- [x] 이름은 빈문자 혹은 공백으로만 이루어지면 안 된다


- 사다리 게임
- [x] 사다리를 생성한다.
- [x] 사다리 게임의 결과를 판단한다.
- [ ] 보고싶은 사람의 사다리 게임의 결과를 전달한다.


- 맵 (사다리)
- [x] 사다리의 높이는 숫자여야 한다.
- [x] 사다리의 최소 높이는 1이다.
Expand All @@ -43,13 +50,34 @@
- [x] 연속으로 연결상태를 가질 수 없다.


- 참가자
- [x] 사람은 10명까지 ( 테스트 원할한 범위 )
- [x] 사람은 2명 이상이다.
- [x] 사람이름은 ',' 로 구분한다.
- [x] 중복된 이름을 가질 수 없다.


- 결과들

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • README가 중복된 부분이 있는 것 같습니다. 확인부탁드려요!
  • docs에 README를 따로 넣으신 이유가 있을까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README 중복이라는 것은 README 파일이 두 개라는 의미가 맞을까요?

docs에 넣은 이유는 크게 없었습니다...!
기존 있는 README를 건드리지 않으려 한 것인데, 질문을 보니 오히려 함께 있는 것이 다른 사람들로 하여금 제 프로젝트를 확인하는 데 용이하겠다는 생각이 드네요.
이러한 이유로 현재 합쳐두었습니다!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

README 중복이라는 것은 README 파일이 두 개라는 의미가 맞을까요?

아뇨, cf546ca 커밋 기준으로

사진과 같이 작성되어있어서 말씀드린 부분이었습니다.

기존 있는 README를 건드리지 않으려 한 것인데, 질문을 보니 오히려 함께 있는 것이 다른 사람들로 하여금 제 프로젝트를 확인하는 데 용이하겠다는 생각이 드네요.
이러한 이유로 현재 합쳐두었습니다!

👍

- [x] 결과는 사람 수 만큼이다.
- [x] 결과는은 ',' 로 구분한다.
- [x] all을 받은 경우 전체 결과를 반환한다.
- [x] 사람 이름을 받은 경우 해당 사람의 결과를 반환한다.


- 결과
- [x] 사다리의 결과는 최대 5글자까지 가능하다
- [x] 결과는 빈문자 혹은 공백으로만 이루어지면 안 된다

- 출력
- [x] 사람 이름은 5자 기준으로 출력하기에 폭도 이에 맞춘다.
- [x] 보고 싶은 사람의 실행 결과를 출력한다.


- 입력
- [x] 참여자 이름을 입력한다.
- [x] 최대 사다리 높이를 입력한다.
- [x] 실행 결과를 입력한다.
- [x] 결과를 보고 싶은 사람의 이름을 입력한다.

## 요구 사항

Expand All @@ -60,6 +88,10 @@
- [x] 사다리 타기가 정상적으로 동작하려면 라인이 겹치지 않도록 해야 한다.
- [x] |-----|-----| 모양과 같이 가로 라인이 겹치는 경우 어느 방향으로 이동할지 결정할 수 없다.
<br>
- [x] 실행 결과를 쉼표(,)를 기준으로 구분한다.
- [x] 사다리 결과 출력 시 결과도 함께 출력한다.
- [x] 결과를 보고 싶은 사람 이름을 입력한다. (all 입력 시 전체 결과 출력)
- [x] all이 아닐 경우 결과 보고 싶은 사람 이름을 다시 물어야 한다.

## 📌 Commit Convention

Expand Down
6 changes: 3 additions & 3 deletions src/main/java/Application.java
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import controller.RadderGameController;
import controller.LadderGameController;
import util.RandomBooleanGenerator;
import view.input.InputView;
import view.output.OutputView;
Expand All @@ -9,7 +9,7 @@ public static void main(String[] args) {
InputView inputView = new InputView();
OutputView outputView = new OutputView();
RandomBooleanGenerator randomBooleanGenerator = new RandomBooleanGenerator();
RadderGameController radderGameController = new RadderGameController();
radderGameController.play(inputView, outputView, randomBooleanGenerator);
LadderGameController ladderGameController = new LadderGameController();
ladderGameController.play(inputView, outputView, randomBooleanGenerator);
}
}
86 changes: 86 additions & 0 deletions src/main/java/controller/LadderGameController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
package controller;

import domain.*;
import util.BooleanGenerator;
import view.input.InputView;
import view.output.OutputView;

public class LadderGameController {

private static final String GET_RESULT_ALL = "all";

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

GET은 붙이지 않아도 되지 않을까요?

Suggested change
private static final String GET_RESULT_ALL = "all";
private static final String RESULT_ALL = "all";


public void play(InputView inputView, OutputView outputView, BooleanGenerator booleanGenerator) {
Participants participants = setParticipants(inputView);
Results results = setResults(inputView, participants);
Ladder ladder = generateLadder(inputView, participants, booleanGenerator);

LadderGame ladderGame = new LadderGame(participants, results, ladder);
printLadder(outputView, ladderGame);

printResult(inputView, outputView, ladderGame);
}

private Participants setParticipants(InputView inputView) {
try {
String participantNames = inputView.enterParticipantNames();
return new Participants(participantNames);
} catch (IllegalArgumentException exception) {
inputView.printErrorMessage(exception);
return setParticipants(inputView);
}
}

private Results setResults(InputView inputView, Participants participants) {
try {
String results = inputView.enterResults();
return new Results(results, participants.getParticipantCount());
} catch (IllegalArgumentException exception) {
inputView.printErrorMessage(exception);
return setResults(inputView, participants);
}
}

private Ladder generateLadder(InputView inputView, Participants participants, BooleanGenerator booleanGenerator) {
try {
Height height = new Height(inputView.enterHeight());
Weight weight = new Weight(participants.getParticipantCount());
return new Ladder(height, weight, booleanGenerator);
} catch (IllegalArgumentException exception) {
inputView.printErrorMessage(exception);
return generateLadder(inputView, participants, booleanGenerator);
}
}

private void printLadder(OutputView outputView, LadderGame ladderGame) {
outputView.printLadder(ladderGame.getParticipants(), ladderGame.getLadder(),
ladderGame.getResults());
}

private void printResult(InputView inputView, OutputView outputView, LadderGame ladderGame) {
boolean isContinue = true;
while (isContinue) {
isContinue = getResult(inputView, outputView, ladderGame);
}
}

private boolean getResult(InputView inputView, OutputView outputView, LadderGame ladderGame) {
try {
String inputResult = inputView.enterGetResult();
return getResultByInput(outputView, ladderGame, inputResult);
} catch (IllegalArgumentException exception) {
inputView.printErrorMessage(exception);
return true;
}
}

private boolean getResultByInput(OutputView outputView, LadderGame ladderGame, String inputValue) {
if (GET_RESULT_ALL.equals(inputValue)) {
outputView.printMatchAllResult(ladderGame.getGameAllResult(),
ladderGame.getParticipants());
return false;
}

outputView.printMatchResult(ladderGame.getGameResultByName(inputValue));
return true;
}
}
43 changes: 0 additions & 43 deletions src/main/java/controller/RadderGameController.java

This file was deleted.

31 changes: 0 additions & 31 deletions src/main/java/domain/GameMap.java

This file was deleted.

30 changes: 30 additions & 0 deletions src/main/java/domain/GameResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
package domain;

import exception.NotFindPersonException;

import java.util.HashMap;
import java.util.Map;

public class GameResult {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

게임 결과라는 객체 👍


private final Map<Person, Result> ladderResult;

public GameResult(Map<Person, Result> ladderResult) {
this.ladderResult = new HashMap<>(ladderResult);
}

public Map<Person, Result> getAllResult() {
return ladderResult;
}

public Result getResultByName(String name) {
Person findPerson = new Person(name);
return ladderResult.entrySet()
.stream()
.filter(resultEntry -> resultEntry.getKey()
.equals(findPerson))
.findFirst()
.orElseThrow(NotFindPersonException::new)
.getValue();
}
}
36 changes: 33 additions & 3 deletions src/main/java/domain/Ladder.java
Original file line number Diff line number Diff line change
@@ -1,16 +1,46 @@
package domain;

import util.BooleanGenerator;

import java.util.List;
import java.util.stream.Collectors;
import java.util.stream.IntStream;

public class Ladder {

private final List<Line> lines;

public Ladder(List<Line> lines) {
this.lines = lines;
public Ladder(Height height, Weight weight, BooleanGenerator booleanGenerator) {
this.lines = generate(height, weight, booleanGenerator);
}

private List<Line> generate(Height height, Weight weight, BooleanGenerator booleanGenerator) {
return IntStream.range(0, height.getHeight())
.mapToObj((count) -> new Line(weight.getWeight(), booleanGenerator))
.collect(Collectors.toList());
}

public List<Line> getLines() {
return List.copyOf(lines);
return lines;
}

public int getResultIndex(int index) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

단순히 index라는 변수명이 아니라 좀 더 명확하게 어떤 인덱스인지 말해주면 좋지 않을까요?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

의견을 보고 participnatIndex로 했지만, Ladder에서는 참여자 인덱스인 것은 알 필요가 없다고 판단해 startIndex로 결정했습니다.
해당 변수명은 괜찮을까요?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

저는 participantIndex로 하는 게 좋지 않을까?라는 생각이었는데요. 말씀대로 Ladder 도메인에서 참여자 인덱스는 굳이 알 필요 없는 게 더 나을 것 같습니다. 👍

int resultIndex = index;
for (Line line : lines) {
resultIndex = getNextIndex(index, line);
Copy link

@aegis1920 aegis1920 Feb 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

도메인의 중요한 부분이 잘못된 것 같습니다. getNextIndex의 인자로 index를 받고 있어서 매번 같은 인자로 받게 됩니다.

이 부분 때문에 Ladder의 lines 필드를 아래와 같이 넣고 테스트해보면 결과값이 틀리게 나옵니다.

// a,b,c,d
// x,100,200,x
// 4
// 정답 : a : 200, b : x, c : 100, d : x

lines.add(new Line(List.of(true, false, false)));  
lines.add(new Line(List.of(false, false, true)));  
lines.add(new Line(List.of(false, false, true)));  
lines.add(new Line(List.of(false, true, false)));

아래처럼 수정이 필요하지 않을까요?

Suggested change
resultIndex = getNextIndex(index, line);
resultIndex = getNextIndex(resultIndex, line);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

정말 바보 같은 실수네요... 🥲
해당 부분은 수정해 두었습니다.

테스트 때 잡히지 않는 것을 보니 테스트도 부실했었다는 생각이 듭니다...
이를 해결하기 위해 TestGenerator를 만들어 좀 더 자세한 테스트가 가능하도록 수정했습니다!

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

테스트 때 잡히지 않는 것을 보니 테스트도 부실했었다는 생각이 듭니다...

개발자로서 좋은 태도를 가지셨군요 👍 테스트 작성 시, 복잡한 도메인 혹은 비즈니스적으로 중요한 도메인은 가급적 테스트에 많은 노력을 기울이시면 좋을 것 같습니다. 추후 이상적인 테스트는 어떤 것일까 고민하게 될텐데요. (시간적 여유가 있다면 단위테스트 라는 책 추천드립니다.)

}

return resultIndex;
}

private int getNextIndex(int index, Line line) {
if (line.canMoveLeft(index)) {
return index - 1;
}
if (line.canMoveRight(index)) {
return index + 1;
}

return index;
Comment on lines +37 to +44

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

로직이 깔끔해서 좋았어요 👍

}
}
Loading