-
Notifications
You must be signed in to change notification settings - Fork 53
[사다리 미션] 정상희 미션 제출합니다. #19
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: sangheejeong
Are you sure you want to change the base?
Changes from 30 commits
1196afa
8b96693
f20be9f
19995ef
49d105c
090cb0f
06a00a7
3e47a2a
07fe975
2265787
2fef17c
f7ab094
dbbb328
65d58d2
56b093f
5c6b21e
67fcc5a
74f098d
9763754
a16726a
69871ca
169ec00
ea543eb
e81fa34
047d483
2dfe464
42cdbd6
62f451d
422af42
494e2d9
b2b1220
7dbb8cd
67b09e4
5c280c4
8eac61b
beeb626
570c4d7
6f6b09a
95f8ec1
f31cacc
e43ee2d
7e4cd52
f8dce70
ac02d9c
d48bce2
ea12de3
01b8efe
7cf78a6
174ff32
6b26030
1d96fb1
0bd1928
76e1d31
6473cca
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
import domain.Ladder; | ||
import domain.LadderGame; | ||
import domain.LadderGenerator; | ||
import domain.Players; | ||
import domain.ResultTypes; | ||
import view.OutputView; | ||
import view.InputView; | ||
|
||
import java.util.List; | ||
|
||
|
||
public class Main { | ||
public static void main(String[] args) { | ||
// 사다리 게임 시작 | ||
final LadderGame ladderGame = new LadderGame(); | ||
|
||
// 플레이어 생성 | ||
final List<String> playerNames = InputView.splitString(InputView.inputNames()); | ||
final Players players = new Players(playerNames); | ||
|
||
// 실행결과 생성 | ||
final List<String> kindOfResults = InputView.splitString(InputView.inputLadderResults()); | ||
final ResultTypes resultTypes = new ResultTypes(kindOfResults, players.getPlayersSize()); | ||
|
||
// 사다리 생성 | ||
final int width = players.getPlayersSize(); | ||
final int height = InputView.inputHeight(); | ||
|
||
final LadderGenerator ladderGenerator = new LadderGenerator(); | ||
final Ladder ladder = ladderGenerator.createLadder(width, height); | ||
|
||
// 사다리 그리기 | ||
OutputView.printPlayers(playerNames); | ||
OutputView.drawLadder(ladder); | ||
OutputView.printResultTypes(resultTypes.getResultTypes()); | ||
|
||
// 사다리 게임 시작 | ||
ladderGame.runGame(ladder, players); | ||
|
||
// 결과 출력 | ||
OutputView.printResult(players, resultTypes.getResultTypes()); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package domain; | ||
|
||
import java.util.Collections; | ||
import java.util.List; | ||
|
||
public class Ladder { | ||
private final List<Line> lines; | ||
|
||
public Ladder(List<Line> lines) { | ||
this.lines = lines; | ||
} | ||
|
||
public List<Line> getLadder() { | ||
return Collections.unmodifiableList(lines); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
package domain; | ||
|
||
public class LadderGame { | ||
|
||
public void movePlayer(Line line, Player player) { | ||
if (line.canMoveRight(player.getPosition())) { | ||
player.moveRight(); | ||
return; | ||
} | ||
|
||
if (line.canMoveLeft(player.getPosition())) { | ||
player.moveLeft(); | ||
} | ||
} | ||
|
||
public void moveEachPlayer(Ladder ladder, Player player) { | ||
ladder.getLadder().forEach(line -> movePlayer(line, player)); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Request Change] 이 두 메서드는 테스트를 제외하곤 LadderGame 내부에서만 사용되네요. 만약 이 로직이 test 대상처럼 느껴진다면, 고민해볼 필요가 있겠네요.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 자료를 찾아보니 private 함수는 단위 테스트를 진행하지 않는 방향이 맞을 것 같아요
|
||
|
||
public void runGame(Ladder ladder, Players players) { | ||
players.getPlayers().forEach(player -> moveEachPlayer(ladder, player)); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Request Change] 디미터의 법칙을 들어보신 적이 있을까요? Players 객체 생성 -> Players 객체에서 List get -> List를 순회하며 move를 한다 디미터의 법칙에 대해서 한번 스터디 해보고, getter를 사용하는 것보다 객체에게 메시지를 던져서 동작하도록 하면 좋겠어요. |
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package domain; | ||
|
||
import java.util.ArrayList; | ||
import java.util.List; | ||
|
||
import static java.lang.Boolean.TRUE; | ||
|
||
public class LadderGenerator { | ||
|
||
public boolean randomTrueOrFalse() { | ||
return Math.random() < 0.5; // 0.0(포함) - 1.0(미포함) 사이의 랜덤한 실수 반환 | ||
SANGHEEJEONG marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
public boolean createValue(List<Boolean> line, int index) { | ||
SANGHEEJEONG marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if (index != 0 && line.get(index - 1) == TRUE) // 이전 값이 true 면 false 반환 | ||
return false; | ||
|
||
return randomTrueOrFalse(); | ||
} | ||
|
||
public Line createLine(int width) { | ||
List<Boolean> points = new ArrayList<>(); | ||
for (int i = 0; i < width - 1; i++) { | ||
points.add(createValue(points, i)); | ||
} | ||
|
||
return new Line(points); | ||
} | ||
|
||
public Ladder createLadder(int width, int height) { | ||
List<Line> lines = new ArrayList<>(); | ||
for (int i = 0; i < height; i++) { | ||
lines.add(createLine(width)); | ||
} | ||
|
||
return new Ladder(lines); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Comment] 상희님은 왜 Ladder를 생성하는 로직을 Ladder 스스로에게 부여하지 않고 Factory로 빼셨을까요? 어떤 장단이 있어서 빼신것인지 생각을 들려주세요. Factory로 빼는 것은 Ladder에 대한 생성 로직을 Ladder 클래스만 봐선 알기 힘들다는 단점이 있을 것 같은데요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Boolean값이 모여 Line을 만들고 Line이 모여 Ladder클래스를 만들어 결국 사다리가 완성되는 것이기 때문에 가시성의 측면에서 이를 한 곳에 모음으로써 사다리 생성의 전체 과정을 쉽게 파악하고자 했습니다. 또한 사다리 생성 방식에 변화가 있을 경우에 대처하기가 쉽고 각 클래스의 생성 로직과 기능을 분리함으로써 책임 분리를 할 수 있다는 것이 장점이라고 생각합니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Comment] 확실히 생성 로직이 복잡하고 그 변경사항이 다른 코드들과 협력이 많이 필요하거나 외부 인프라 등을 활용해야할 때 외부에 팩터리를 두는 것이 유리할때가 있죠. 하지만, 저는 여전히 해당 createLadder(29번-36번)은 그리 복잡해보이지 않은 로직이라, Ladder안에 정적팩터리매서드로 존재해도 좋다고 생각해요. 그렇게 한다면, 위에 Line과 Point를 만드는 로직 또한 각각의 클래스 내부로 넣어야겠죠.
라는 의견이 틀렸다는 말은 아니지만, 저는 꼭 그렇진 않다고 생각해요. 만약 제가 상희님 후임자로 대신 이 프로그램을 유지보수한다고 생각했을 때, 사다리 생성 요구사항에 변경이 생기면
속는 셈치고 루카 훈수 따라보기
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
package domain; | ||
|
||
import java.util.Collections; | ||
import java.util.List; | ||
|
||
import static java.lang.Boolean.TRUE; | ||
|
||
public class Line { | ||
private final List<Boolean> points; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Request Change] Boolean은 어떤 의미를 갖는 값인가요. 과연 Boolean을 100명에게 보여줬을 때, 100명 모두에게
이를 납득시킬 수 있으신가요? 더 의미있는 값으로 변경하기에 좋은 방법이 뭐가 있을까요? |
||
|
||
public Line(List<Boolean> points) { | ||
this.points = points; | ||
} | ||
|
||
public boolean canMoveRight(int ladderOrder) { | ||
return (ladderOrder < points.size()) && (points.get(ladderOrder) == TRUE); | ||
} | ||
|
||
public boolean canMoveLeft(int ladderOrder) { | ||
return (ladderOrder != 0) && (points.get(ladderOrder - 1) == TRUE); | ||
} | ||
|
||
public List<Boolean> getLine() { | ||
return Collections.unmodifiableList(points); | ||
} | ||
SANGHEEJEONG marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
package domain; | ||
|
||
public class Name { | ||
|
||
private final String name; | ||
private final static int MAX_LENGTH = 5; | ||
private final static String INVALID_NAME = "all"; | ||
|
||
public Name(String name) { | ||
validateName(name); | ||
this.name = name; | ||
} | ||
|
||
private void validateName(String name) { | ||
validateMaxLength(name); | ||
validateNotBlank(name); | ||
validateNotEqualAll(name); | ||
} | ||
|
||
private void validateNotBlank(String name) { | ||
if (name.isBlank()) { | ||
SANGHEEJEONG marked this conversation as resolved.
Show resolved
Hide resolved
|
||
throw new IllegalArgumentException("이름은 공백일 수 없습니다."); | ||
} | ||
} | ||
|
||
private void validateMaxLength(String name) { | ||
if (name.length() > MAX_LENGTH) { | ||
throw new IllegalArgumentException("이름은 5자를 초과할 수 없습니다."); | ||
} | ||
} | ||
|
||
private void validateNotEqualAll(String name) { | ||
if (name.equals(INVALID_NAME)) { | ||
throw new IllegalArgumentException("이름은 all일 수 없습니다."); | ||
} | ||
} | ||
|
||
public String getName() { | ||
return name; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
package domain; | ||
|
||
public class Player { | ||
|
||
private final Name name; | ||
private final Position position; | ||
|
||
public Player(Name name, Position position) { | ||
this.name = name; | ||
this.position = position; | ||
} | ||
|
||
public void moveLeft() { | ||
position.movePositionLeft(); | ||
} | ||
|
||
public void moveRight() { | ||
position.movePositionRight(); | ||
} | ||
|
||
public String getName() { | ||
return name.getName(); | ||
} | ||
|
||
public int getPosition() { | ||
return position.getPosition(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,47 @@ | ||
package domain; | ||
|
||
import java.util.ArrayList; | ||
import java.util.Collections; | ||
import java.util.List; | ||
import java.util.NoSuchElementException; | ||
|
||
public class Players { | ||
|
||
private final List<Player> players; | ||
private final static int MIN_PLAYER_SIZE = 2; | ||
|
||
public Players(List<String> playerNames) { | ||
validateSize(playerNames); | ||
this.players = createPlayer(playerNames); | ||
} | ||
|
||
private void validateSize(List<String> playerNames) { | ||
if (playerNames.size() < MIN_PLAYER_SIZE) { | ||
throw new IllegalArgumentException("플레이어 수는 2명 이상이어야 합니다."); | ||
} | ||
} | ||
|
||
private List<Player> createPlayer(List<String> playerNames) { | ||
List<Player> players = new ArrayList<>(); | ||
for (int i = 0; i < playerNames.size(); i++) { | ||
players.add(new Player(new Name(playerNames.get(i)), new Position(i))); | ||
} | ||
|
||
return players; | ||
} | ||
|
||
public Player findByName(String viewerName) { | ||
return players.stream() | ||
.filter(player -> player.getName().equals(viewerName)) | ||
.findFirst() | ||
.orElseThrow(() -> new NoSuchElementException("플레이어 이름 '" + viewerName + "' 이 존재하지 않습니다.")); | ||
} | ||
|
||
public List<Player> getPlayers() { | ||
return Collections.unmodifiableList(players); | ||
} | ||
Comment on lines
+53
to
+55
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Approve] 방어적 복사 👍👍 |
||
|
||
public int getPlayersSize() { | ||
return players.size(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
package domain; | ||
|
||
public class Position { | ||
|
||
private int position; | ||
|
||
public Position(int position) { | ||
this.position = position; | ||
} | ||
|
||
public void movePositionLeft() { | ||
position--; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Request Change] position이 0인 객체의 이 메서드가 호출되지 않을 것이란 보장이 있을까요? 🧐🧐🧐🧐 일단 저는 저를 포함한 동료개발자들을 그리 믿진 않아요. |
||
|
||
public void movePositionRight() { | ||
position++; | ||
} | ||
|
||
public int getPosition() { | ||
return position; | ||
} | ||
Comment on lines
+25
to
+27
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Comment] position을 getter로 꺼내는 방식 보다 조금 더 이 값에 대해서 판단을 이 Position이란 객체에게 맡겨보는 것은 어떨까요? 위의 리뷰들을 참고해서 한번 변경해보면 좋을 것 같아요 |
||
|
||
SANGHEEJEONG marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
package domain; | ||
|
||
import java.util.List; | ||
|
||
public class ResultTypes { | ||
|
||
private final List<String> resultTypes; | ||
|
||
public ResultTypes(List<String> resultTypes, int width) { | ||
validate(resultTypes, width); | ||
this.resultTypes = resultTypes; | ||
} | ||
|
||
private void validate(List<String> resultTypes, int width) { | ||
resultTypes.forEach(this::validateNotBlank); | ||
validateSize(resultTypes, width); | ||
} | ||
|
||
private void validateNotBlank(String resultType) { | ||
if (resultType.isBlank()) { | ||
throw new IllegalArgumentException("실행 결과는 공백일 수 없습니다."); | ||
} | ||
} | ||
|
||
private void validateSize(List<String> resultTypes, int width) { | ||
if (resultTypes.size() != width) { | ||
throw new IllegalArgumentException("실행 결과는 사다리의 개수와 일치해야 합니다."); | ||
} | ||
} | ||
|
||
public List<String> getResultTypes() { | ||
return resultTypes; | ||
} | ||
} | ||
Comment on lines
+5
to
+34
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [�Comment] 이 List은 List 같이 의도가 분명한 Wrapper클래스로 감싸지 않은 이유가 있나요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Result에 대한 속성이 단순히 문자형 결과이고 이에 따른 책임이 유효성 검증밖에 없다고 생각해서 굳이 나누지 않았습니다 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Comment] 사실 위에서 한 유효성 검증은 ResultType 하나의 요소에 대한 검증이지, List 자체에 대한 검증은 아닌 것 같기도 하네요. 그치만, 저도 상희님이 말씀하신 것 처럼 꼭 래핑할 이유는 없다고 생각해요 해당 클래스만으로도 충분히 의미전달이 된다고 생각합니다 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
package view; | ||
|
||
public class InputException { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Comment] Input에 대한 Validate를 따로 분리시킨 이유가 있을까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 원래는 Input에 대한 예외가 많을 줄 알고 나눴었는데 2개 밖에 찾아내지 못해서 그냥 합치는 게 나을 것 같네요 수정했습니다! |
||
|
||
public static void validateHeightSize(int height) { | ||
if (height < 1) { | ||
throw new IllegalArgumentException("사다리 높이는 1 이상이어야 합니다."); | ||
} | ||
} | ||
|
||
public static void validateViewerNameNotBlank(String viewerName) { | ||
if (viewerName.isBlank()) { | ||
throw new IllegalArgumentException("이름은 공백일 수 없습니다."); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,38 @@ | ||
package view; | ||
|
||
import java.util.Arrays; | ||
import java.util.List; | ||
import java.util.Scanner; | ||
|
||
public class InputView { | ||
private static final Scanner input = new Scanner(System.in); | ||
|
||
public static List<String> splitString(String string) { | ||
return Arrays.asList(string.split(",")); | ||
} | ||
|
||
public static String inputNames() { | ||
System.out.println("참여할 사람 이름을 입력하세요. (이름은 쉼표(,)로 구분하세요)"); | ||
return input.nextLine(); | ||
} | ||
|
||
public static String inputLadderResults() { | ||
System.out.println("실행 결과를 입력하세요. (결과는 쉼표(,)로 구분하세요)"); | ||
return input.nextLine(); | ||
} | ||
|
||
public static int inputHeight() { | ||
System.out.println("최대 사다리 높이는 몇 개인가요?"); | ||
int height = input.nextInt(); | ||
InputException.validateHeightSize(height); | ||
input.nextLine(); | ||
return height; | ||
} | ||
|
||
public static String inputViewerName() { | ||
System.out.println("\n결과를 보고 싶은 사람은?"); | ||
String viewerName = input.nextLine(); | ||
InputException.validateViewerNameNotBlank(viewerName); | ||
return viewerName; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Request Change] 유저는 실수로 잘못 입력한건데 프로그램이 에러로 꺼지는 것은 너무 가혹한 것 같아요. 재입력을 받아보게 바꿀 수 있을까요? |
||
} |
Uh oh!
There was an error while loading. Please reload this page.