Skip to content

[사다리 미션] 정상희 미션 제출합니다. #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

Open
wants to merge 54 commits into
base: sangheejeong
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 42 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
1196afa
[FEAT] Ladder & Line 클래스 생성
SANGHEEJEONG Oct 24, 2024
8b96693
[FEAT] Line 생성 메서드 작성 (1단계)
SANGHEEJEONG Oct 24, 2024
f20be9f
[FEAT] LineView Enum 클래스 생성 (1단계)
SANGHEEJEONG Oct 24, 2024
19995ef
[REFACTOR] domain과 view 분리 (1단계)
SANGHEEJEONG Oct 24, 2024
49d105c
[FEAT] Ladder 생성 메서드 작성 (1단계)
SANGHEEJEONG Oct 24, 2024
090cb0f
[FEAT] OutputView 메서드 작성 및 Main 구현 (1단계)
SANGHEEJEONG Oct 24, 2024
06a00a7
[FEAT] InputView 메서드 작성 및 Main 구현 (2단계)
SANGHEEJEONG Oct 25, 2024
3e47a2a
[FEAT] 사다리 방향 확인 메서드 작성 (3단계)
SANGHEEJEONG Oct 26, 2024
07fe975
[FEAT] 사다리 게임 실행 메서드 작성 (3단계)
SANGHEEJEONG Oct 26, 2024
2265787
[FEAT] 사다리 게임 실행 메서드 수정 (3단계)
SANGHEEJEONG Oct 26, 2024
2fef17c
[FIX] 사다리 방향 결정 인덱스 오류 수정(3단계)
SANGHEEJEONG Oct 26, 2024
f7ab094
[FEAT] 4단계 입력 메서드 작성 (4단계)
SANGHEEJEONG Oct 26, 2024
dbbb328
[FEAT] Players 클래스 생성 (4단계)
SANGHEEJEONG Oct 26, 2024
65d58d2
[FEAT] GameResult 클래스 생성 및 OutputView 메서드 작성 (4단계)
SANGHEEJEONG Oct 26, 2024
56b093f
[FEAT] main 작성 (4단계)
SANGHEEJEONG Oct 26, 2024
5c6b21e
[REFACTOR] decideWhereToGo 메서드 분리 (5단계)
SANGHEEJEONG Oct 26, 2024
67fcc5a
[REFACTOR] Player 객체 분리 (5단계)
SANGHEEJEONG Oct 29, 2024
74f098d
[REFACTOR] LadderGenerator 메서드 수정 (5단계)
SANGHEEJEONG Oct 29, 2024
9763754
[REFACTOR] LadderGame forEach 람다 형식으로 바꿔보기(5단계)
SANGHEEJEONG Oct 29, 2024
a16726a
[REFACTOR] OutputView 메서드 순서 수정(5단계)
SANGHEEJEONG Oct 29, 2024
69871ca
[REFACTOR] findPlayerByName 메서드를 Players로 책임 분리(5단계)
SANGHEEJEONG Oct 30, 2024
169ec00
[REFACTOR] OutputView 주석 및 수정 (5단계)
SANGHEEJEONG Oct 30, 2024
ea543eb
[REFACTOR] Name, Position 원시값 포장 (5단계)
SANGHEEJEONG Oct 30, 2024
e81fa34
[REFACTOR] Name 예외처리 (5단계)
SANGHEEJEONG Oct 30, 2024
047d483
[REFACTOR] ResultTypes 예외처리 (5단계)
SANGHEEJEONG Oct 30, 2024
2dfe464
[REFACTOR] Players 예외처리 (5단계)
SANGHEEJEONG Oct 30, 2024
42cdbd6
[REFACTOR] 입력 예외처리 (5단계)
SANGHEEJEONG Oct 30, 2024
62f451d
[REFACTOR] 불변객체 적용 (5단계)
SANGHEEJEONG Oct 30, 2024
422af42
[REFACTOR] 단위테스트 작성 (5단계)
SANGHEEJEONG Oct 30, 2024
494e2d9
[REFACTOR] 코드 정렬 (5단계)
SANGHEEJEONG Oct 30, 2024
b2b1220
Create README.md
SANGHEEJEONG Nov 2, 2024
7dbb8cd
[REFACTOR] 리뷰 반영
SANGHEEJEONG Nov 2, 2024
67b09e4
[REFACTOR] README 작성
SANGHEEJEONG Nov 2, 2024
5c280c4
[REFACTOR] README 수정
SANGHEEJEONG Nov 6, 2024
8eac61b
[REFACTOR] Controller 메서드 분리
SANGHEEJEONG Nov 6, 2024
beeb626
[REFACTOR] 미사용 import 정리
SANGHEEJEONG Nov 6, 2024
570c4d7
[REFACTOR] LadderGame 메서드 책임에 따른 클래스 분리
SANGHEEJEONG Nov 7, 2024
6f6b09a
[REFACTOR] 접근지정자 수정 public -> private
SANGHEEJEONG Nov 7, 2024
95f8ec1
[REFACTOR] Boolaen -> Point ENUM 사용
SANGHEEJEONG Nov 8, 2024
f31cacc
[REFACTOR] position 메서드 수정
SANGHEEJEONG Nov 8, 2024
e43ee2d
[REFACTOR] 입력 메서드 수정
SANGHEEJEONG Nov 8, 2024
7e4cd52
[REFACTOR] LadderController 수정
SANGHEEJEONG Nov 9, 2024
f8dce70
[REFACTOR] LadderGenerator 삭제 및 Controller 메서드 분리
SANGHEEJEONG Nov 16, 2024
ac02d9c
[REFACTOR] moveLeft & moveRight 예외 처리
SANGHEEJEONG Nov 16, 2024
d48bce2
[REFACTOR] Controller 와 Application 책임 분리
SANGHEEJEONG Nov 18, 2024
ea12de3
[REFACTOR] Ladder 클래스 책임 부여
SANGHEEJEONG Nov 18, 2024
01b8efe
[REFACTOR] decideWhereToGo 메서드 line 클래스 이동
SANGHEEJEONG Nov 18, 2024
7cf78a6
[REFACTOR] Point -> LadderStep 이름 변경 및 학습테스트(익명 클래스 & 인터페이스) 적용
SANGHEEJEONG Nov 18, 2024
174ff32
[REFACTOR] nextStep 미사용 인자 삭제
SANGHEEJEONG Nov 19, 2024
6b26030
[REFACTOR] 동명이인 예외 처리
SANGHEEJEONG Nov 19, 2024
1d96fb1
[REFACTOR] PlayerName VO로 만들기
SANGHEEJEONG Nov 20, 2024
0bd1928
[REFACTOR] test 수정
SANGHEEJEONG Nov 20, 2024
76e1d31
[REFACTOR] test 수정
SANGHEEJEONG Nov 20, 2024
6473cca
[REFACTOR] 코드 정렬
SANGHEEJEONG Nov 20, 2024
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
24 changes: 24 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# 기능 목록

### 1. 플레이어 및 사다리 정보 입력 기능
+ 플레이어 이름과 결과 종류를 입력한다.
+ 사다리 넓이 : (플레이어 수 - 1)
+ 사다리 높이 : 사용자 입력
### 2. 사다리 실행 기능
+ |-----|-----| 가로 라인이 겹치지 않게 사다리를 생성한다.
=> 즉, 앞 라인이 true면 다음 라인은 무조건 false이다.
### 3. 사다리 출력 기능
+ 플레이어 이름 입력 순으로 출력
+ 생성된 사다리 출력
+ 결과 종류 입력 순으로 출력
+ 보고 싶은 실행 결과 출력 ("all"일 때는 모두 출력)
Comment on lines +3 to +14

Choose a reason for hiding this comment

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

[Comment]

🎮 컨트롤러 나누기 5

다시 리드미로 돌아와서 생각해보면 좋을 것 같아요.

  • 리드미의 기능 목록이 잘 나눠져있는지? (제가 예시로 든 내용을 믿지 마세요. 상희님을 믿으세요.)
  • 잘 작성된 리드미로 어떤 컨트롤러 동작들이 존재해야하는지 (이게 웹 어플리케이션으로 가면 API 명세가 될거에요)


### <사다리 구현>
+ Boolean (다리)
+ ture : 연결
+ false : 연결 X
+ List&lt;Boolean&gt (한 층의 다리 모음)
+ 사다리의 넒이만큼
+ 다리가 연결되어 있으면(=true) 수평 방향으로 이동이 가능하다.
+ List&lt;Line&gt (모든 층의 다리 모음)
+ 사다리의 높이만큼
Comment on lines +1 to +24

Choose a reason for hiding this comment

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

[Comment]

제가 코드를 잘 이해할 수 있도록 리드미에 의도를 잘 담아주셔서 감사합니다.😄

이 리드미는 훌륭한 가치와 내용을 담은 리드미라고 생각해요.

기능을 명세화할 떄 여러가지 방법 있겠죠?

  1. 유저 시나리오 대로 동작을 정의한다 (1-3번)
  2. 각각의 요소(모델)들이 갖는 규칙과 동작을 정의한다 (<사다리 구현>)

이런 측면에서 잘 구조화 해주신 것 같습니다.
개선할 점이라고 한다면, 조금 더 코드보단 비즈니스 규칙에 가치를 두는 것이 읽는 사람의 이해를 도울 수 있을 것입니다.
코드는 개발자의 의도에 따라 설계 등이 바뀔 수 있지만, 더 명확하게 전달해야되는 것은 비즈니스 요구사항이니까요.

8 changes: 8 additions & 0 deletions src/main/java/Application.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import controller.LadderController;

public class Application {
public static void main(String[] args) {
LadderController ladderController = new LadderController();
ladderController.run();
}
}
50 changes: 50 additions & 0 deletions src/main/java/controller/LadderController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
package controller;

import domain.Ladder;
import domain.LadderGenerator;
import domain.Players;
import domain.ResultTypes;
import view.InputView;
import view.OutputView;

import java.util.List;

public class LadderController {

public void run() {
List<String> playerNames = InputView.splitString(InputView.inputNames());
List<String> kindOfResults = InputView.splitString(InputView.inputLadderResults());

// 플레이어 생성
Players players = new Players(playerNames);
// 결과 종류 생성
ResultTypes resultTypes = new ResultTypes(kindOfResults, players.getPlayersSize());
// 사다리 초기화
Ladder ladder = initializeLadder(players);
Copy link

@dooboocookie dooboocookie Nov 13, 2024

Choose a reason for hiding this comment

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

[Comment]

🎮 컨트롤러 나누기 2

이만큼이 한 동작(혹은 한 요청)에 해당 될 수 있을 것 같아요.

이제 어느정도 http api 도 익숙해지셨을테니,
Spring의 컨트롤러로 예시를 들겟습니다.

Suggested change
List<String> playerNames = InputView.splitString(InputView.inputNames());
List<String> kindOfResults = InputView.splitString(InputView.inputLadderResults());
// 플레이어 생성
Players players = new Players(playerNames);
// 결과 종류 생성
ResultTypes resultTypes = new ResultTypes(kindOfResults, players.getPlayersSize());
// 사다리 초기화
Ladder ladder = initializeLadder(players);
@PostMapping("/api/ladders")
public ResponseEntity<Void> createLadder(@RequestBody LadderSaveRequest request) {
// 요청
List<String> playerNames = request.playerNames;
List<String> kindOfResults = request.kindOfResults;
// 비즈니스 로직 호출
Players players = new Players(playerNames);
ResultTypes resultTypes = new ResultTypes(kindOfResults, players.getPlayersSize());
Ladder ladder = initializeLadder(players); // 메모리에 저장하거나, 영속화
// (추가) 응답 생성
return ResponseEntity.create( 레더를 찾아갈수있는 경로)
}

당연히 이게 정답이라고 공감하실 필요도 없고, 하셔도 안됩니다.

이렇게 사다리 생성(or게임 시작 or 게임방 생성 or 사다리 저장)이라고 정의할 수 있는 독립적인 동작이 완성됐네요.

Copy link
Author

Choose a reason for hiding this comment

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

넵 특히 이 예시를 통해 아직 분리가 덜 됐다는 것이 확연히 보이네요!


// 사다리 및 결과 출력
displayLadder(resultTypes, playerNames, ladder);

Choose a reason for hiding this comment

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

[Comment]

🎮 컨트롤러 나누기 3

Suggested change
displayLadder(resultTypes, playerNames, ladder);
@GetMapping("/api/ladders/{id}")
public ResponseEntity<Ladder> createLadder(@PathVariable Long id) {
// 비즈니스 로직, 찾는 작업... (아이디가 존재하지 않는 콘솔 어플리케이션이라해도)
Ladder ladder = 메모린지_디빈지_모를_저장소.findByIdOrNull(id);
// 응답 생성 -> 콘솔로 따지면 outputview가 출력할 수 있게 outputView로 전달
return ResponseEntity.�body(ladders)
}

playGameAndDisplayResults(resultTypes, players, ladder);

Choose a reason for hiding this comment

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

[Comment]

🎮 컨트롤러 나누기 4

이 영역도 2-3과 같이 해볼 수 있겠죠?

}

Choose a reason for hiding this comment

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

[Comment]

🎮 컨트롤러 나누기 1

이전 리뷰 질문에도 답변을 달았습니다.

Controller는 저희가 게임의 흐름을 관리하도록 만들어진 애는 아니라고 생각해요.
만약 run이라는 것이 1개의 동작이라고 생각한다면 생각을 존중하겠습니다만,
저희는 Controller를 어떤 동작을 할 때 View와 Model의 협업을 위해 연결다리로 둔 대상이죠.

제가 이 밑에 하나의 동작이라고 생각하는 영역들을 묶어볼거에요.


private Ladder initializeLadder(Players players) {
int width = players.getPlayersSize();
int height = InputView.inputHeight();

LadderGenerator ladderGenerator = new LadderGenerator();
Ladder ladder = ladderGenerator.createLadder(width, height);

return ladder;
}

private void displayLadder(ResultTypes resultTypes, List<String> playerNames, Ladder ladder) {
OutputView.printPlayers(playerNames);
OutputView.drawLadder(ladder);
OutputView.printResultTypes(resultTypes.getResultTypes());
}

private void playGameAndDisplayResults(ResultTypes resultTypes, Players players, Ladder ladder) {
players.moveAllPlayers(ladder);
OutputView.printResult(players, resultTypes.getResultTypes());
}
}
17 changes: 17 additions & 0 deletions src/main/java/domain/Ladder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
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);
}
}

Choose a reason for hiding this comment

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

[Comment]

getter 만 존재하는 일급컬렉션을 선언하신 이유가 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

List<Line>을 그냥 사용하는 것보다 일급 컬렉션으로 선언했을 때

  1. 관련된 로직을 직접 처리한다
  2. 각 컬렉션에 의미부여가 명확해진다

이 두 가지의 큰 장점이 생기는 것 같습니다.
근데 리뷰어님이 말씀하신대로 getter만 존재한다는 것은 장점1이 적용되지 않은 클래스라고 생각되네요! 다시 한 번 코드를 보면서 해당 클래스에서 처리할 메서드가 있다면 옮기도록 하겠습니다

Choose a reason for hiding this comment

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

[Comment]

여전히 허전하네요. 일급컬랙션으로 래핑할 이유가 없어보여요.

근데, 너무 중요한 도메인이죠? 이 미션이름이 **'사다리'**미션인데 어떻게 객체로 안만들어요.😭😭😭😭😭😭😭😭😭

그래도 아쉬운 마음에 조금 더 영업을 해보겠습니다.
저는 사다리 게임의 어떤 모양?을 상상하면
image
(출처: 네이버 사다리 게임)
image
(출처: 위키피디아)

사다리 꼭대기에 놓여진 players들이 너무 input 같지 않나요?🧐

Copy link
Author

Choose a reason for hiding this comment

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

저는 moveAllPlayers 메서드가 '플레이어들이 사다리 위를 움직인다'는 관점으로 봐서 1차 리뷰 수정 때 Players 클래스에 넣었는데 생각해보니까 '사다리' 게임인 만큼 리뷰어 님이 말해주신 저 관점으로 보면 Ladder 클래스에 넣는 게 더 적합할 수도 있겠네요

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

import java.util.ArrayList;
import java.util.List;

public class LadderGenerator {

private boolean randomTrueOrFalse() {
return Math.random() < 0.5; // 0.0(포함) - 1.0(미포함) 사이의 랜덤한 실수 반환
}

private Point createValue(List<Point> line, int lineIndex) {
if (lineIndex != 0 && line.get(lineIndex - 1).isEnabled()) { // 이전 값이 true 면 false 반환
return Point.DISABLED;
}

return Point.FROM_BOOLEAN.apply(randomTrueOrFalse());
}

public Line createLine(int width) {
List<Point> 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);
}

Choose a reason for hiding this comment

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

[Comment]

상희님은 왜 Ladder를 생성하는 로직을 Ladder 스스로에게 부여하지 않고 Factory로 빼셨을까요?

어떤 장단이 있어서 빼신것인지 생각을 들려주세요.

Factory로 빼는 것은 Ladder에 대한 생성 로직을 Ladder 클래스만 봐선 알기 힘들다는 단점이 있을 것 같은데요.

Copy link
Author

Choose a reason for hiding this comment

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

Boolean값이 모여 Line을 만들고 Line이 모여 Ladder클래스를 만들어 결국 사다리가 완성되는 것이기 때문에 가시성의 측면에서 이를 한 곳에 모음으로써 사다리 생성의 전체 과정을 쉽게 파악하고자 했습니다.

또한 사다리 생성 방식에 변화가 있을 경우에 대처하기가 쉽고 각 클래스의 생성 로직과 기능을 분리함으로써 책임 분리를 할 수 있다는 것이 장점이라고 생각합니다.

Choose a reason for hiding this comment

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

[Comment]

확실히 생성 로직이 복잡하고 그 변경사항이 다른 코드들과 협력이 많이 필요하거나 외부 인프라 등을 활용해야할 때 외부에 팩터리를 두는 것이 유리할때가 있죠.

하지만, 저는 여전히 해당 createLadder(29번-36번)은 그리 복잡해보이지 않은 로직이라, Ladder안에 정적팩터리매서드로 존재해도 좋다고 생각해요.

그렇게 한다면, 위에 Line과 Point를 만드는 로직 또한 각각의 클래스 내부로 넣어야겠죠.

사다리 생성 방식에 변화가 있을 경우에 대처하기가 쉽고

라는 의견이 틀렸다는 말은 아니지만, 저는 꼭 그렇진 않다고 생각해요. 만약 제가 상희님 후임자로 대신 이 프로그램을 유지보수한다고 생각했을 때, 사다리 생성 요구사항에 변경이 생기면

  1. 무조건 class Ladder 부터 찾아본다.
  2. Ladder 생성자가 쓰이는 곳을 찾아본다.
    이 순서로 생성 로직을 볼 것 같아요.

속는 셈치고 루카 훈수 따라보기

  • 현재 상황의 고민 점
    • 이렇게 팩터리 패턴을 유지한다 했을 때, 해당 클래스는 유틸 클래스여야하는가?
  • 훈수 들어보기
    • 각 Ladder, Line, Point의 생성 로직을 해당 클래스로 넣어보기
    • Point에 대해서는 해당 코드에서 좀 더 자세히 말씀드리겠습니다

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

import java.util.Collections;
import java.util.List;

public class Line {
private final List<Point> points;

public Line(List<Point> points) {
this.points = points;
}

public boolean canMoveRight(int ladderOrder) {
return (ladderOrder < points.size()) && (points.get(ladderOrder).isEnabled());
}

public boolean canMoveLeft(int ladderOrder) {
return (ladderOrder != 0) && (points.get(ladderOrder - 1).isEnabled());
}

Choose a reason for hiding this comment

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

[Comment]

|-----|        |-----|

Line 엔티티는 이러 형태의 객체인데요.
움직일 수 있냐도 좋은 메시지지만,
Line에 Players가 탄다고 생각했을 때,
Line이 좋은 결과물을 내뱉어주지 않을까요?
0번 위치 플레이어 -> 1번 위치
1번 위치 플레이어 -> 0번 위치
2번 위치 플레이어 -> 3번 위치
3번 위치 플레이어 -> 2번 위치

image


public List<Point> getLine() {
return Collections.unmodifiableList(points);
}

public int getSize() {
return points.size();
}
}
26 changes: 26 additions & 0 deletions src/main/java/domain/Player.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package domain;

import java.util.List;

public class Player {

private final PlayerName playerName;
private final Position position;

public Player(PlayerName name, Position position) {
this.playerName = name;
this.position = position;
}

public void moveAlongLadder(List<Line> ladder) {
ladder.forEach(this.position::decideWhereToGo);
}

public String getPlayerName() {
return playerName.getName();
}

public int getPosition() {
return position.getPosition();
}
}
41 changes: 41 additions & 0 deletions src/main/java/domain/PlayerName.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package domain;

public class PlayerName {

private final String name;
private final static int MAX_LENGTH = 5;
private final static String INVALID_NAME = "all";
Comment on lines +6 to +7

Choose a reason for hiding this comment

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

[Approve]

객체의 규칙을 한눈에 볼 수 있는 상수 정말 좋은데요? 👍


public PlayerName(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()) {
throw new IllegalArgumentException("이름은 공백일 수 없습니다.");
}
}

private void validateMaxLength(String name) {
if (name.length() > MAX_LENGTH) {
throw new IllegalArgumentException("이름은 " + MAX_LENGTH + "자를 초과할 수 없습니다.");
}
}

private void validateNotEqualAll(String name) {
if (name.equals(INVALID_NAME)) {
throw new IllegalArgumentException("이름은 " + INVALID_NAME + "일 수 없습니다.");
}
}

public String getName() {
return name;
}
}
51 changes: 51 additions & 0 deletions src/main/java/domain/Players.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
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 PlayerName(playerNames.get(i)), new Position(i)));
}

return players;
}

public void moveAllPlayers(Ladder ladder) {
players.forEach(player -> player.moveAlongLadder(ladder.getLadder()));
}

public Player findByName(String viewerName) {
return players.stream()
.filter(player -> player.getPlayerName().equals(viewerName))

Choose a reason for hiding this comment

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

[Request Chnage]

PlayerName을 VO로서 사용해보면 어떨까요? (실제로도 거의 그렇게 사용되고 있음)
그렇다면 Player는?

고민점

  1. 값 객체(VO)는 어떨때 같은 객체일까? 엔티티는 언제 같은 대상일까? 를 고민해보고 playerName과 player의 equals hascode를 재정의해주세요
  2. 이름이 같다고 같은 사람이라고 할 때 이 게임의 문제는 안생길까?

Copy link
Author

Choose a reason for hiding this comment

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

값 객체라는 키워드를 처음 접해봤는데요, 찾아본 결과
엔티티와 값 객체의 차이점은
불변성과 목적 측면에서 차이가 큰 것 같네요

PlayerName은 불변성의 측면에서 한 번 생성되면 내부 값이 변경되지 않는다는 특징이 있고 고유 식별자가 없어 객체 자체 보다는 내부 속성에 의미가 있기 때문에 VO로 사용하면 좋을 것 같습니다.

하지만 Player는 요구사항에 따라 내부 속성이 변경될 가능성이 크고 객체 자체가 도메인에서 의미가 있는 대상이니까 값 객체 보다는 엔티티의 역할을 하는 것이 맞는 것 같습니다.

  1. playerName에 재정의 추가했습니다
  2. 이름 중복 예외 처리 추가했습니다!

.findFirst()
.orElseThrow(() -> new NoSuchElementException("플레이어 이름 '" + viewerName + "' 이 존재하지 않습니다."));
}

public List<Player> getPlayers() {
return Collections.unmodifiableList(players);
}
Comment on lines +53 to +55

Choose a reason for hiding this comment

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

[Approve]

방어적 복사 👍👍


public int getPlayersSize() {
return players.size();
}
}
26 changes: 26 additions & 0 deletions src/main/java/domain/Point.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
package domain;

import java.util.function.Function;

public enum Point {
ENABLED(true),
DISABLED(false);

private final boolean enabled;

// 생성자를 통해 상태를 설정
Point(boolean enabled) {
this.enabled = enabled;
}

public boolean isEnabled() {
return enabled;
}

public static final Function<Boolean, Point> FROM_BOOLEAN = value -> {
if (value) {
return ENABLED;
}
return DISABLED;
};
}

Choose a reason for hiding this comment

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

[Comment]

만약 제 훈수를 따라오셨다면, 해당 enum에 생성로직을 만드셨을텐데요.

현재 이 enum은 true false 값을 대신한다는 느낌만 갖고있어요. 역하과 책임이 빈약하다는 생각이 듭니다.

  1. 생성로직을 가져보기 (DISABLED가 될수밖에 없는 건 누구 책임이야...?)
  2. 사다리 이동에 대한 책임을 가질 수 있지 않을까?

이 리뷰에서 말씀드린 학습테스트 해보셨나요? => Point 이넘의 객체 별 동작을 정의하거나 할때 많은 도움이 될 것입니다.

이름도 조금 더 의미있는 값으로 바꿔주세요

Copy link
Author

Choose a reason for hiding this comment

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

아 이거 잘 모르겠어서 고민을 꽤 했는데 인터페이스를 구현해서 익명 클래스로 만드는 방식을 말씀하신 건가요? (아니라면,, 말씀해주세요 ㅎ)
일단 이 방식으로 수정했습니다. 사실 익명 클래스랑 인터페이스 사용이 익숙하지 않았었는데 이번 기회를 통해 어떻게 사용하면 좋은지 알게 된 것 같아요 감사합니다 !

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

public class Position {

private int position;

public Position(int position) {
this.position = position;
}

public void decideWhereToGo(Line line) {
if (line.canMoveRight(position)) {
moveRight(line.getSize());
return;
}

if (line.canMoveLeft(position)) {
moveLeft();
}
}

public void moveLeft() {
if (position != 0) {
position--;
}
}

public void moveRight(int maxPosition) {
if (position != maxPosition) {
position++;
}
}

Choose a reason for hiding this comment

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

[Request Change]

이정도 대처로 충분하신가요?
만약 0번 인덱스에서 moveLeft를 호출해버렸다면, 0인채로 한칸 내려가겠네요.
그렇다면 그건 그 나름대로 심각한 버그 아닐까요?

그렇게 프로그램 내부에서 런타임에 예상도 확인도 불가능한 에러가 나게 두면 0인 인덱스가 동일한 층에서 여러개나올 수도 있겠네요...
어떻게 개발자가 이런 위험들을 감지하면서 서비스 운영을 할 수 있을까요?


public int getPosition() {
return position;
}
Comment on lines +25 to +27

Choose a reason for hiding this comment

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

[Comment]

position을 getter로 꺼내는 방식 보다 조금 더 이 값에 대해서 판단을 이 Position이란 객체에게 맡겨보는 것은 어떨까요?

위의 리뷰들을 참고해서 한번 변경해보면 좋을 것 같아요

}
34 changes: 34 additions & 0 deletions src/main/java/domain/ResultTypes.java
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

Choose a reason for hiding this comment

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

[�Comment]

이 List은 List 같이 의도가 분명한 Wrapper클래스로 감싸지 않은 이유가 있나요?

Copy link
Author

Choose a reason for hiding this comment

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

Result에 대한 속성이 단순히 문자형 결과이고 이에 따른 책임이 유효성 검증밖에 없다고 생각해서 굳이 나누지 않았습니다

Choose a reason for hiding this comment

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

[Comment]

사실 위에서 한 유효성 검증은 ResultType 하나의 요소에 대한 검증이지, List 자체에 대한 검증은 아닌 것 같기도 하네요.

그치만, 저도 상희님이 말씀하신 것 처럼 꼭 래핑할 이유는 없다고 생각해요 해당 클래스만으로도 충분히 의미전달이 된다고 생각합니다

Loading