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

Step3 #2027

Open
wants to merge 5 commits into
base: kimbro97
Choose a base branch
from
Open

Step3 #2027

Show file tree
Hide file tree
Changes from all commits
Commits
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
18 changes: 13 additions & 5 deletions src/main/java/nextstep/ladder/controller/LadderController.java
Original file line number Diff line number Diff line change
@@ -1,19 +1,27 @@
package nextstep.ladder.controller;

import nextstep.ladder.domain.Height;
import nextstep.ladder.domain.LadderGame;
import nextstep.ladder.domain.Names;
import nextstep.ladder.domain.*;
import nextstep.ladder.domain.strategy.RandomLadderPoint;
import nextstep.ladder.view.InputView;
import nextstep.ladder.view.OutputView;

import java.util.List;

public class LadderController {
public static void main(String[] args) {
Names names = InputView.inputNames();
List<LadderResult> inputLadderResults = InputView.inputLadderResults();
Height height = InputView.inputHeight();
Lines lines = new Lines();
lines.initialize(names.size(), height, new RandomLadderPoint());

OutputView.printNames(names);
OutputView.printLadders(lines);
Copy link

Choose a reason for hiding this comment

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

참가자 수와 결과 수의 갯수가 다른 경우(유효성이 안맞는 경우) 잘못된 사다리를 만들기전에 유효성 검증을 하고 출력하지 않도록 하면 어떨까요? 현재는 다음과 같이 노출되네요!
image

OutputView.printInputResults(inputLadderResults);

LadderGame ladderGame = new LadderGame(names, new RandomLadderPoint(), height);
LadderGame ladderGame = new LadderGame(names, lines, inputLadderResults);
LadderResults ladderResults = ladderGame.play();

OutputView.printNamesAndLadders(ladderGame);
OutputView.printResultByName(InputView.inputNameForResult(), ladderResults);
}
}
38 changes: 25 additions & 13 deletions src/main/java/nextstep/ladder/domain/LadderGame.java
Original file line number Diff line number Diff line change
@@ -1,29 +1,25 @@
package nextstep.ladder.domain;

import nextstep.ladder.domain.strategy.GenerateLadderPoint;

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

public class LadderGame {
Copy link

Choose a reason for hiding this comment

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

이 객체는 도메인인가요 서비스인가요? 🤔

private Names names;
private Lines lines;
Comment on lines 9 to 10
Copy link

Choose a reason for hiding this comment

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

불변값이라면 final 키워드를 붙혀도 좋을 것 같네요

private final List<LadderResult> results;

public LadderGame(Names names, GenerateLadderPoint generateLadderPoint, Height height) {
public LadderGame(Names names, Lines lines, List<LadderResult> results) {
this.names = names;
initialize(height, generateLadderPoint);
}

private void initialize(Height height, GenerateLadderPoint generateLadderPoint) {
this.lines = createLines(height, generateLadderPoint);
this.lines = lines;
validateResults(results);
Copy link

Choose a reason for hiding this comment

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

유효성검증은 항상 최상단에 위치시키는걸 권장드려요. 이펙티브 자바에서도 말하지만, 유효성검증은 비즈니스 로직에서 항상 최우선하라고 하거든요

this.results = results;
}

private Lines createLines(Height height, GenerateLadderPoint generateLadderPoint) {
List<Line> lines = IntStream.range(0, height.getPoint())
.mapToObj(i -> new Line(names.size(), generateLadderPoint))
.collect(Collectors.toList());
return new Lines(lines);
public LadderResults play() {
Map<Name, LadderResult> gameResults = generateGameResults();
return new LadderResults(gameResults);
}

public Names getNames() {
Expand All @@ -33,4 +29,20 @@ public Names getNames() {
public Lines getLines() {
return lines;
}

private Map<Name, LadderResult> generateGameResults() {
return IntStream.range(0, names.size())
.boxed()
.collect(Collectors.toMap(names.getNames()::get, this::calculateResult));
}

private LadderResult calculateResult(int index) {
return results.get(lines.move(index));
}

private void validateResults(List<LadderResult> results) {
if (results.size() != names.size()) {
throw new IllegalArgumentException("참가자 수와 결과 수가 일치하지 않습니다.");
}
}
}
22 changes: 22 additions & 0 deletions src/main/java/nextstep/ladder/domain/LadderResult.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
package nextstep.ladder.domain;

public class LadderResult {
private final String result;

public LadderResult(String result) {
validateResult(result);

this.result = result;
}

public String getResult() {
return result;
}

private void validateResult(String result) {
if (result == null || result.isEmpty()) {
throw new IllegalArgumentException("결과가 입력되지 않았습니다.");
}
}

}
21 changes: 21 additions & 0 deletions src/main/java/nextstep/ladder/domain/LadderResults.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package nextstep.ladder.domain;

import java.util.Map;
import java.util.Optional;

public class LadderResults {
private final Map<Name, LadderResult> ladderResults;

public LadderResults(Map<Name, LadderResult> ladderResults) {
this.ladderResults = ladderResults;
}

public LadderResult getResultForParticipant(Name name) {
return Optional.ofNullable(ladderResults.get(name))
.orElseThrow(() -> new IllegalArgumentException(name.getName() + " 참가자가 존재하지 않습니다."));
}

public Map<Name, LadderResult> getAllResults() {
return ladderResults;
}
}
18 changes: 18 additions & 0 deletions src/main/java/nextstep/ladder/domain/Line.java
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,25 @@ public Line(int namesSize, GenerateLadderPoint generateLadderPoint) {
}
}

public int move(int position) {
if (isMoveRight(position)) {
return position + 1;
}
if (isMoveLeft(position)) {
return position - 1;
}
return position;
}

public List<Boolean> getPoints() {
return points;
}

private boolean isMoveRight(int position) {
return position < points.size() && points.get(position);
}

private boolean isMoveLeft(int position) {
return position > 0 && points.get(position - 1);
}
}
26 changes: 24 additions & 2 deletions src/main/java/nextstep/ladder/domain/Lines.java
Original file line number Diff line number Diff line change
@@ -1,12 +1,34 @@
package nextstep.ladder.domain;

import nextstep.ladder.domain.strategy.GenerateLadderPoint;
import nextstep.ladder.domain.strategy.RandomLadderPoint;

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

public class Lines {
private List<Line> lines;

public Lines(List<Line> lines) {
this.lines = lines;
public Lines() {
}

public void initialize(int size, Height height, GenerateLadderPoint generateLadderPoint) {
this.lines = createLines(size, height, generateLadderPoint);
}

public int move(int position) {
for (Line line : lines) {
position = line.move(position);
}
return position;
}

private List<Line> createLines(int size, Height height, GenerateLadderPoint generateLadderPoint) {
Copy link

Choose a reason for hiding this comment

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

사용자가 매번 new로 객체를 생성하고 초기화로직을 실행시키는 것과
정적 팩토리 함수를 활용하는것중 어떤게 좀 더 나을까요?

return IntStream.range(0, height.getPoint())
.mapToObj(i -> new Line(size, generateLadderPoint))
.collect(Collectors.toList());
}

public List<Line> getLines() {
Expand Down
21 changes: 21 additions & 0 deletions src/main/java/nextstep/ladder/view/InputView.java
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package nextstep.ladder.view;

import nextstep.ladder.domain.Height;
import nextstep.ladder.domain.LadderResult;
import nextstep.ladder.domain.Name;
import nextstep.ladder.domain.Names;

Expand All @@ -27,10 +28,30 @@ public static Height inputHeight() {
return new Height(SCANNER.nextInt());
}

public static List<LadderResult> inputLadderResults() {
System.out.println("실행 결과를 입력하세요. (결과는 쉼표(,)로 구분하세요)");

return parseLadderResults(SCANNER.nextLine());
}

public static Name inputNameForResult() {
System.out.println("결과를 보고 싶은 사람은?");
String inputNames = SCANNER.nextLine();

return new Name(inputNames);
}

private static Names splitNames(String inputNames) {
List<Name> names = Arrays.stream(inputNames.split(COMMA))
.map(Name::new)
.collect(Collectors.toList());
return new Names(names);
}

private static List<LadderResult> parseLadderResults(String inputLadderResults) {
return Arrays.stream(inputLadderResults.split(COMMA))
.map(String::trim)
.map(LadderResult::new)
.collect(Collectors.toList());
}
}
63 changes: 45 additions & 18 deletions src/main/java/nextstep/ladder/view/OutputView.java
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
package nextstep.ladder.view;

import nextstep.ladder.domain.LadderGame;
import nextstep.ladder.domain.Line;
import nextstep.ladder.domain.Lines;
import nextstep.ladder.domain.Names;
import nextstep.ladder.domain.*;

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

public class OutputView {
Expand All @@ -15,39 +13,68 @@ public class OutputView {
private static final String LADDER_SPACE = " ";
private static final String NEWLINE = System.lineSeparator();

public static void printNamesAndLadders(LadderGame ladderGame) {
System.out.println("실행 결과" + NEWLINE);
private static final String PARTICIPANT_NAME_ALL = "all";
private static final int MAX_NAME_LENGTH = 5;

String output = formatParticipantNames(ladderGame.getNames()) +
formatLines(ladderGame.getNames().getFirstNameLength(), ladderGame.getLines());
private OutputView() {
}

public static void printInputResults(List<LadderResult> inputLadderResults) {
System.out.println(formatLadderResults(inputLadderResults) + NEWLINE);
}

public static void printResultByName(Name inputName, LadderResults ladderResults) {
printHeader();
if (inputName.getName().equals(PARTICIPANT_NAME_ALL)) {
printAllParticipantResults(ladderResults);
} else {
printSingleParticipantResult(inputName, ladderResults);
}
Comment on lines +28 to +32
Copy link

Choose a reason for hiding this comment

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

view 계층이라도 if-else는 안티패턴입니다!

}

private static void printHeader() {
System.out.println("실행 결과");
}

System.out.println(output);
private static void printAllParticipantResults(LadderResults ladderResults) {
ladderResults.getAllResults().forEach((participant, result) ->
System.out.println(participant.getName() + " : " + result.getResult()));
Copy link

Choose a reason for hiding this comment

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

StringBuilder를 활용해서 무분별한 System.out을 줄여보면 어떨까요?

}

private static String formatParticipantNames(Names names) {
int lastIndex = names.getNames().size() - 1;
private static void printSingleParticipantResult(Name name, LadderResults ladderResults) {
System.out.println(ladderResults.getResultForParticipant(name).getResult());
}

public static void printNames(Names names) {
String strNames = names.getNames().stream()
.limit(lastIndex)
.map(name -> String.format(NAME_FORMAT, name.getName()))
.collect(Collectors.joining());

return strNames + String.format(NAME_FORMAT, names.getNames().get(lastIndex).getName()) + NEWLINE;
System.out.println(strNames);
}

public static void printLadders(Lines lines) {
System.out.println(formatLines(lines));
}

private static String formatLines(int firstNameLength, Lines lines) {
private static String formatLines(Lines lines) {
return lines.getLines().stream()
.map(line -> formatSingleLine(firstNameLength, line))
.collect(Collectors.joining(NEWLINE)) + NEWLINE;
.map(OutputView::formatSingleLine)
.collect(Collectors.joining(NEWLINE));
}

private static String formatSingleLine(int firstNameLength, Line line) {
StringBuilder ladder = new StringBuilder(LADDER_SPACE.repeat(firstNameLength) + LADDER_VERTICAL_LINE);
private static String formatSingleLine(Line line) {
StringBuilder ladder = new StringBuilder(LADDER_SPACE.repeat(MAX_NAME_LENGTH) + LADDER_VERTICAL_LINE);

line.getPoints().stream()
.map(point -> point ? LADDER_HORIZONTAL_LINE : LADDER_EMPTY_SPACE)
.forEach(segment -> ladder.append(segment).append(LADDER_VERTICAL_LINE));

return ladder.toString();
}
private static String formatLadderResults(List<LadderResult> inputLadderResults) {
return inputLadderResults.stream()
.map(ladderResult -> String.format(NAME_FORMAT, ladderResult.getResult()))
.collect(Collectors.joining());
}
}
25 changes: 25 additions & 0 deletions src/test/java/nextstep/ladder/domain/LadderGameTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package nextstep.ladder.domain;

import nextstep.ladder.domain.strategy.RandomLadderPoint;
import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

import java.util.List;

import static org.assertj.core.api.Assertions.assertThatThrownBy;

class LadderGameTest {

@Test
@DisplayName("참가자 수와 결과 수가 일치하지 않으면 예외가 발생한다")
void validateLadderResults_exception() {
Names names = new Names(List.of(new Name("a")));
List<LadderResult> inputLadderResults = List.of(new LadderResult("꽝"), new LadderResult("1000"));
Lines lines = new Lines();
lines.initialize(1, new Height(1), new RandomLadderPoint());
Assertions.assertThatIllegalArgumentException().isThrownBy(() -> {
new LadderGame(names, lines, inputLadderResults);
});
}
}
16 changes: 16 additions & 0 deletions src/test/java/nextstep/ladder/domain/LadderResultTest.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package nextstep.ladder.domain;

import org.assertj.core.api.Assertions;
import org.junit.jupiter.api.DisplayName;
import org.junit.jupiter.api.Test;

class LadderResultTest {
@Test
@DisplayName("결과가 입력되지않으면 예외가 발생한다")
Copy link

Choose a reason for hiding this comment

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

예외는 발생한다기보단 던져지는게(throw) 적절한 표현입니다.

void create_exception() {
Assertions.assertThatIllegalArgumentException().isThrownBy(() -> {
new LadderResult("");
new LadderResult(null);
});
}
}
Copy link

Choose a reason for hiding this comment

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

현재 테스트를 전체적으로 확인해봤는데,

  • LadderGame : 전체적으로 사용하지 않는 함수 제거 필요, private 함수들에 대한 테스트 없음(private 함수를 직접 호출하라는게 아니라 public함수에서 분기검증이 되지 않았다는 의미입니다. )
  • LadderResult( with LadderResults): 일급 컬렉션 생성자 테스트 및 사다리 결과 조회 테스트 부족
  • Line : 전체적으로 테스트 부족
  • Lines: getter와 move에 대한 테스트 부족
  • Name, Names: getter 테스트 및 불필요 함수 존재

TDD인만큼 테스트에 좀 더 신경써보는건 어떨까요?