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

[1단계 사다리 생성] 다즐(최우창) 미션 제출합니다. #70

Merged
merged 36 commits into from
Feb 20, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
374f493
docs: 기능 목록 작성
woo-chang Feb 14, 2023
00e2f5a
feat: 사람 이름 영문자 검증 기능 구현
woo-chang Feb 14, 2023
cdfb0e0
feat: 사람 이름 길이 검증 기능 구현
woo-chang Feb 14, 2023
816fdc4
docs: 요구사항 명세서 수정
woo-chang Feb 14, 2023
9176b1e
feat: 플레이어 이름 앞, 뒤 공백 제거 기능 구현
woo-chang Feb 14, 2023
3b0a1bc
feat: 플레이어 수 검증 기능 구현
woo-chang Feb 15, 2023
0961c8f
feat: 플레이어 이름 중복 검증 기능 구현
woo-chang Feb 15, 2023
599438a
chore: TDD 이후 클래스 위치 이동
woo-chang Feb 15, 2023
495c9a5
feat: 특정 이동값을 가지는 방향 구현
woo-chang Feb 15, 2023
7298261
feat: 발판을 위한 숫자 생성 기능 구현
woo-chang Feb 15, 2023
d8a9c35
feat: 값에 알맞은 방향 반환 기능 구현
woo-chang Feb 15, 2023
af94afb
feat: 방향 생성 전략 기능 구현
woo-chang Feb 15, 2023
20ae067
feat: 방향들의 집합인 선 구현
woo-chang Feb 15, 2023
c143e85
feat: 선 생성 기능 구현
woo-chang Feb 15, 2023
b8f3a18
feat: 사다리 생성 기능 구현
woo-chang Feb 15, 2023
147d7cb
feat: 플레이어들의 이름 입력 기능 구현
woo-chang Feb 15, 2023
094df09
feat: 최대 사다리 높이 입력 기능 구현
woo-chang Feb 15, 2023
87efb38
feat: 사다리 생성 결과 출력 기능 구현
woo-chang Feb 15, 2023
84fa42a
feat: 높이 검증 기능 구현
woo-chang Feb 16, 2023
2f27677
refactor: 플레이어 이름 역할 분리
woo-chang Feb 16, 2023
172394f
feat: 사다리 애플리케이션 기능 구현
woo-chang Feb 16, 2023
0d89856
refactor: 상수 추가 및 네이밍 수정
woo-chang Feb 16, 2023
3cd6d4b
refactor: 사다리 높이 역할 분리에 따른 리팩토링
woo-chang Feb 16, 2023
c49a5fa
refactor: 네이밍 수정 및 불필요한 파라미터와 메서드 삭제
woo-chang Feb 16, 2023
90fcb7a
refactor: 메서드 결합도와 복잡성을 높이는 메서드 전달자 제거
woo-chang Feb 18, 2023
6d26352
refactor: 의미가 모호한 메서드명, 시그니처 수정
woo-chang Feb 18, 2023
3a521e5
refactor: 사다리 생성 역할 분리
woo-chang Feb 18, 2023
72cb897
refactor: for -> while 사용하도록 수정
woo-chang Feb 18, 2023
5f1752f
refactor: 한번만 사용되는 예외 메시지인 경우 상수화 하지 않는 것으로 수정
woo-chang Feb 18, 2023
9cba752
refactor: 공백이 포함되면 유효하지 않은 이름으로 처리하도록 수정
woo-chang Feb 18, 2023
3af9db2
refactor: format 비용을 고려하여 string 연산을 사용하도록 수정
woo-chang Feb 18, 2023
3baf790
refactor: 의미있는 추상화를 위한 메서드명 수정
woo-chang Feb 18, 2023
a1fc51b
refactor: 생성자에서 의존성 주입이 아닌 메서드에서 의존성 주입하도록 수정
woo-chang Feb 18, 2023
8531b23
refactor: 라인 생성 메소드 재귀 -> while 방법으로 수정
woo-chang Feb 18, 2023
79a8148
refactor: 발판에 대한 책임은 Direction이 가지도록 수정
woo-chang Feb 18, 2023
bb70ff7
refactor: 출력 요구사항에 맞게 개행 추가
woo-chang Feb 18, 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: 46 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,3 +5,49 @@
## 우아한테크코스 코드리뷰

- [온라인 코드 리뷰 과정](https://github.com/woowacourse/woowacourse-docs/blob/master/maincourse/README.md)

## 개요

해당 저장소는 사다리 생성을 구현한 저장소입니다. `n`명의 사람 이름과 높이 `m`을 입력하면 사다리가 생성됩니다.

## 세부사항

플레이어

- 플레이어는 여러 명 존재할 수 있다.
- [x] [제한사항] 플레이어는 2명 이상, 10명 이하만 가능하다.
- [x] [제한사항] 플레이어의 이름은 중복될 수 없다.
- 플레이어는 이름을 가집니다.
- [x] [제한사항] 이름은 영문자만 가능하다.
- [x] [제한사항] 이름은 최대 5글자까지 가능하다.

방향

- [x] 왼쪽, 오른쪽, 정지를 가진다.
- [x] 값에 알맞은 방향을 반환한다.


- [x] 방향들의 집합이다.

사다리

- [x] 선들의 집합이다.
Comment on lines +33 to +35

Choose a reason for hiding this comment

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

오오.. 유비쿼터스 언어를 잘 정의해주셨네요

Copy link
Member Author

Choose a reason for hiding this comment

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

해당 명세서를 개발자만 읽는 것이 아닌, 모두가 읽을 수 있게 만들기 위해 고민했던 것 같습니다 😀

- [x] 높이는 2이상, 10이하만 가능합니다.

방향 생성

- [x] 랜덤 방향을 생성한다.

선 생성

- [x] 방향 생성 전략에 따라 선을 생성한다.

입력

- [x] 참여할 플레이어들의 이름을 입력한다.
- [x] 최대 사다리 높이를 입력한다.

출력

- [x] 사다리 생성 결과를 출력한다.
19 changes: 19 additions & 0 deletions src/main/java/ladder/LadderApplication.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
package ladder;

import java.util.Scanner;
import ladder.controller.LadderController;
import ladder.domain.generator.LadderGenerator;
import ladder.view.InputView;
import ladder.view.OutputView;

public class LadderApplication {

public static void main(String[] args) {
final var inputView = new InputView(new Scanner(System.in));
final var outputView = new OutputView();
final var ladderGenerator = new LadderGenerator();

LadderController ladderController = new LadderController(inputView, outputView, ladderGenerator);
ladderController.run();
}
}
41 changes: 41 additions & 0 deletions src/main/java/ladder/controller/LadderController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
package ladder.controller;

import java.util.function.Function;
import java.util.function.Supplier;
import ladder.domain.Height;
import ladder.domain.Ladder;
import ladder.domain.Players;
import ladder.domain.generator.LadderGenerator;
import ladder.domain.generator.LineGenerator;
import ladder.view.InputView;
import ladder.view.OutputView;

public class LadderController {

private final InputView inputView;
private final OutputView outputView;
private final LadderGenerator ladderGenerator;

public LadderController(final InputView inputView, final OutputView outputView,
final LadderGenerator ladderGenerator) {
this.inputView = inputView;
this.outputView = outputView;
this.ladderGenerator = ladderGenerator;
}

public void run() {
final Players players = generate(inputView::readPlayerNames, Players::new);
final Height height = generate(inputView::readHeight, Height::new);
final Ladder ladder = ladderGenerator.generate(new LineGenerator(), players, height);
outputView.printLadderResult(players, ladder);
}

private <T, R> R generate(final Supplier<T> supplier, final Function<T, R> function) {
try {
return function.apply(supplier.get());
} catch (IllegalArgumentException e) {
outputView.printErrorMessage(e.getMessage());
return generate(supplier, function);
}
}
}
34 changes: 34 additions & 0 deletions src/main/java/ladder/domain/Direction.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package ladder.domain;

import java.util.Arrays;

public enum Direction {

LEFT(-1, "-----"),
STAY(0, " "),
RIGHT(1, " "),
;

private final int move;
private final String foothold;

Direction(final int move, final String foothold) {
this.move = move;
this.foothold = foothold;
}

public static Direction from(final int value) {
return Arrays.stream(Direction.values())
.filter(direction -> direction.getMove() == value)
.findFirst()
.orElseThrow(() -> new IllegalArgumentException("유효하지 않은 값입니다."));
}
Comment on lines +20 to +25

Choose a reason for hiding this comment

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

매번 stream을 열어줘야 하는걸까요 ?
(너무 깊게 생각하진 말아주세요 :) )

Copy link
Member Author

Choose a reason for hiding this comment

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

조건문을 사용하는 방법도 생각해 보았는데, 조건문을 사용하게 되면 Direction에 새로운 값이 추가될 때마다 from 메서드의 수정이 발생하게 되었습니다. 그렇기에 매번 stream을 열어주는 리스크가 있더라도 위의 방법을 사용하게 되었습니다.

메서드를 수정 비용보다 매번 stream을 열어주는 비용이 더 큰걸까요 .. ?

Choose a reason for hiding this comment

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

해당 로직을 static화 해보는게 어떨지 말씀드린거였어요 :)
크게 중요하진 않은 부분이라 깊게 생각하지 않으셔도 됩니다 :)


public int getMove() {
return move;
}

public String getFoothold() {
return foothold;
}
}
33 changes: 33 additions & 0 deletions src/main/java/ladder/domain/Height.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package ladder.domain;

public class Height {

private static final int MINIMUM_HEIGHT = 2;
private static final int MAXIMUM_HEIGHT = 10;

private final int value;

public Height(final int value) {
validate(value);
this.value = value;
}

private void validate(final int value) {
if (hasShort(value) || hasLong(value)) {
throw new IllegalArgumentException("높이는 " + MINIMUM_HEIGHT + "이상, " + MAXIMUM_HEIGHT + "이하여야 합니다."
+ " 현재 높이는 " + value + "입니다.");
}
}

private boolean hasShort(final int value) {
return value < MINIMUM_HEIGHT;
}

private boolean hasLong(final int value) {
return MAXIMUM_HEIGHT < value;
}

public int getValue() {
return value;
}
}
18 changes: 18 additions & 0 deletions src/main/java/ladder/domain/Ladder.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
package ladder.domain;

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

public class Ladder {

private final List<Line> lines;

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

public List<Line> getLines() {
return Collections.unmodifiableList(lines);
}
}
Comment on lines +14 to +17

Choose a reason for hiding this comment

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

객체내의 상태를 그대로 반환한다면, 상태는 숨기고 행동을 노출하는 캡슐화의 장점을 포기한다는 생각이 들어요.

Copy link
Member Author

Choose a reason for hiding this comment

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

저도 상태를 그대로 반환하는 getter 사용 지양에 적극 공감합니다! 하지만 출력을 위해서는 getter 사용이 따라오게 되었는데 이 부분은 어떻게 해결할 수 있을까요 😂

Choose a reason for hiding this comment

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

VO를 DTO로 쓴다! 라고 한다면 지금과 같이 작성되어도 문제없을 것 같아요 :)
하지만, VO가 아니게 된다면, 출력에 필요한 인자들을 만들어 보내는게 조금더 결합을 낮출 수 있을 것 같습니다.


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

import java.util.List;

public class Line {

private final List<Direction> directions;

public Line(final List<Direction> directions) {
this.directions = directions;
}

public List<Direction> getDirections() {
return directions;
}
}
Comment on lines +5 to +16

Choose a reason for hiding this comment

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

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.

Direction이라는 Enum을 정의하였기에 검증은 필요없다고 생각하였고 이번 미션에서는 사다리 생성 후 출력만 진행하고 있기에 필요한 책임이 없었던 것 같습니다 .. 상태 노출도 위와 동일하게 출력을 위한 getter 였기에 따라오게 되었습니다 😂

추가적으로 가져야할 책임이 어떤게 있을지 조언을 듣고 싶습니다!

Choose a reason for hiding this comment

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

Line이 foothold를 가지는건 어떨까요? Direction에 UI를 위한 String이 포함되는게 좋아보이진 않아서요 :)

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

import java.util.Objects;
import java.util.regex.Pattern;

public class Name {

private static final Pattern NAME_FORMAT = Pattern.compile("[a-zA-Z]+");
private static final int NAME_MAX_LENGTH = 5;

private final String value;

public Name(final String value) {
validate(value);
this.value = value;
}

private void validate(final String value) {
validateFormat(value);
validateLength(value);
}
Comment on lines +18 to +21

Choose a reason for hiding this comment

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

Suggested change
private void validate(final String value) {
validateFormat(value);
validateLength(value);
}
private void validate(final String value) {
if (isNotEnglish(value)) {
throw new IllegalArgumentException("사람 이름은 영문자만 가능합니다. 현재 입력은" + value + " 입니다.");
}
if (hasExceedLength(value)) {
throw new IllegalArgumentException("사람 이름은 " + NAME_MAX_LENGTH + "글자까지 가능합니다. 현재 입력은+ " + value + "입니다.");
}
}

argument가 1개일 경우에는 string 연산을 사용해주세요.
String 재할당 비용보다 비용이 큽니다.

Copy link
Member Author

Choose a reason for hiding this comment

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

format에 드는 비용은 생각지도 못하고 있었는데 성능적으로도 생각해볼 수 있었습니다! 감사합니다 👍


private void validateFormat(final String value) {
if (isNotEnglish(value)) {
throw new IllegalArgumentException("사람 이름은 영문자만 가능합니다. 현재 입력은 " + value + " 입니다.");
}
}

private boolean isNotEnglish(final String value) {
return !NAME_FORMAT.matcher(value).matches();
}

private void validateLength(final String value) {
if (hasExceedLength(value)) {
throw new IllegalArgumentException("사람 이름은 " + NAME_MAX_LENGTH + "글자까지 가능합니다. 현재 입력은 " + value + " 입니다.");
}
}

private boolean hasExceedLength(final String value) {
return NAME_MAX_LENGTH < value.length();
}

public String getValue() {
return value;
}

@Override
public boolean equals(final Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
final Name name = (Name) o;
return Objects.equals(getValue(), name.getValue());
}

@Override
public int hashCode() {
return Objects.hash(getValue());
}
}
33 changes: 33 additions & 0 deletions src/main/java/ladder/domain/Player.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
package ladder.domain;

import java.util.Objects;

public class Player {

private final Name name;

Choose a reason for hiding this comment

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

위에서 인스턴스 변수를 value로 설정하셨는데, 여기선 name로 설정되어있네요.

전체 소프트웨에서 통일시켜주세요 :)

"값 객체의 경우 값 그자체가 의미를 나타내기에 속성은 value로 통일한다." 라는 등의 다즐만의 기준이 있으면 좋겠어요.

Choose a reason for hiding this comment

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

다른 값 객체를 속성으로 포장하고 있어도 동일합니다.
(현재 속성이 1개뿐이여서요.)

Copy link
Member Author

Choose a reason for hiding this comment

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

Height, Name 안에서는 클래스명으로 의미를 담고 있기에 value라는 변수를 설정하였는데, Player에서는 이름이라는 것을 조금 더 직관적으로 표현하기 위해 name으로 설정하였습니다! 이런 경우에도 통일하는게 좋을까요?

Choose a reason for hiding this comment

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

네, 속성이 1개인 경우에는, Height, Name와 같이, 클래스가 해당 의미를 담고 있다고 생각해도 무방할 것 같습니다.

다른 2곳에서 속성값이 1개인 경우에 value로 설정했으니, Player도 value로 설정해도, 무방할 것 같고, 일관성도 지켜지는 것 같아서요.


public Player(final String value) {
this.name = new Name(value);
}

public String getName() {
return name.getValue();
}

@Override
public boolean equals(final Object o) {
if (this == o) {
return true;
}
if (o == null || getClass() != o.getClass()) {
return false;
}
final Player player = (Player) o;
return Objects.equals(name, player.name);
}

@Override
public int hashCode() {
return Objects.hash(name);
}
}
69 changes: 69 additions & 0 deletions src/main/java/ladder/domain/Players.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
package ladder.domain;

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

public class Players {

private static final int MINIMUM_PLAYER_SIZE = 2;
private static final int MAXIMUM_PLAYER_SIZE = 10;

private final List<Player> players;

public Players(final List<String> names) {
final List<Player> players = generatePlayers(names);
validate(players);
this.players = players;
}

private List<Player> generatePlayers(final List<String> names) {
return names.stream()
.map(Player::new)
.collect(Collectors.toList());
}

private void validate(final List<Player> players) {
validatePlayerSize(players);
validateDuplicatePlayer(players);
}

private void validatePlayerSize(final List<Player> players) {
if (hasSmallSize(players) || hasLargeSize(players)) {
throw new IllegalArgumentException(
"플레이어는 " + MINIMUM_PLAYER_SIZE + "명 이상, " + MAXIMUM_PLAYER_SIZE + "명 이하만 가능합니다."
+ " 현재 입력한 플레이어 수는 " + players.size() + "명 입니다.");
}
}

private boolean hasSmallSize(final List<Player> players) {
return players.size() < MINIMUM_PLAYER_SIZE;
}

private boolean hasLargeSize(final List<Player> players) {
return MAXIMUM_PLAYER_SIZE < players.size();
}

private void validateDuplicatePlayer(final List<Player> players) {
final Set<Player> uniquePlayers = new HashSet<>(players);

if (isDuplicate(players, uniquePlayers)) {
throw new IllegalArgumentException("플레이어 이름은 중복되면 안됩니다.");
}
}

private boolean isDuplicate(final List<Player> players, final Set<Player> uniquePlayers) {
return players.size() != uniquePlayers.size();
}

public List<String> getPlayerNames() {
return players.stream()
.map(Player::getName)
.collect(Collectors.toUnmodifiableList());
}

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

import ladder.domain.Direction;

@FunctionalInterface
public interface DirectionGenerator {

Direction generate();
}
Loading