Skip to content

[사다리 게임] 동건 -성혁 사다리 게임 구현 완료했습니다. 리뷰 부탁드립니다.#2

Open
sunghyuki wants to merge 59 commits intosproutt:kikatfrom
sunghyuki:kikat
Open

[사다리 게임] 동건 -성혁 사다리 게임 구현 완료했습니다. 리뷰 부탁드립니다.#2
sunghyuki wants to merge 59 commits intosproutt:kikatfrom
sunghyuki:kikat

Conversation

@sunghyuki
Copy link
Contributor

@sunghyuki sunghyuki commented Feb 21, 2022

사다리 게임

자세한 후기는 회고를 통하여 남기도록 하겠습니다.

View에서 예외를 처리 vs Controller에서 예외를 처리

  • 성혁

제 생각은 뷰가 바뀜에 따라 사용자로부터 입력받는 값의 포맷이 바뀔 수 있기 때문에, 컨트롤러가 거기에 종속적이어서는 안 된다고 생각합니다. 뷰에서 나가는 데이터는 직관적인 형태로 최소한의 정리가 되어야 한다고 생각하구요. 뷰의 역할이 어디까지인가에 대한 고민을 해보았는데 예를 들어 뷰로부터 자동차들의 이름을 받아서 새로운 Car 객체들을 생성한다면, 컨트롤러의 역할은 뷰로부터 자동차들의 이름을 받아서 새로운 Car 객체를 생성하는 것이라고 생각되기 때문에 위에서 이야기 했던 것처럼 뷰에서 나가는 데이터는 직관적인 형태로 최소한의 정리가 되어야 한다고 생각합니다

  • 동건

지금은 View가 Console이기 때문에 Java 코드로 작성되어 있지만, View가 html 코드로 바뀐다면 예외처리 코드를 다시 작성해줘야 하지 않을까라고 생각을 한다. View가 데이터를 어느 정도 정리해서 Controller에게 보내 주는 것은 맞지만, 항상 올바른 값만 들어올 수는 없다고 생각한다. 프론트와 백으로 생각을 해봤을 때, 프론트가 보내주는 데이터의 검증을 한번 했다고 백엔드에서 검증을 하지 않는 것은 프론트에게 너무 의존적인 것이 아닐까라는 생각이 든다.

  • 우리의 결론

View의 역할은 구분자를 분리하고, 공백을 제거해주는 등과 같은 데이터를 최소한의 정리된 형태로 만들어, Controller로 넘겨주는 것까지라고 생각된다. 그래서 예외처리는 Controller에서 해야된다.

1주차 사다리 게임 회고

성혁 - https://velog.io/@sung_hyuki/2%EC%A3%BC%EC%B0%A8-%ED%8E%98%EC%96%B4-%ED%94%84%EB%A1%9C%EA%B7%B8%EB%9E%98%EB%B0%8D-%EC%82%AC%EB%8B%A4%EB%A6%AC-%EA%B2%8C%EC%9E%84-%ED%9A%8C%EA%B3%A0
동건 -

2주차 사다리 게임 회고

성혁 -
상엽 - https://saint6839.github.io/sproutt-2nd/2022/03/02/Sproutt-Pair-Programming-With-Sunghyuk.html

- Point들이 모여서 한 라인이 생성된다.
- Line들이 모여서 사다리가 생성된다.
- 첫 번째 Point의 left 상태는 false인 메서드 추가
- 이전 Point의 right 상태에 따라 현재 Point의 right 상태를 결정하는 메서드 추가
- 마지막 Point의 right 상태는 false인 메서드 추가
- boolean 값 랜덤 생성 메서드 추가
- OutputView에 가로선 연결 여부값만 보내주기 위함
- limit을 사용한 이유는 맨오른쪽 줄 right 상태값이 포함된 것을 제거하기 위한 범위 설정
- 참여할 인원 수 입력받기 메서드 추가
- 사다리 높이 입력받기 메서드 추가
- 사다리 한 층 생성 메서드 추가
- 사다리 생성 메서드 추가
- 사다리 출력
- 사다리 한 층 출력 메서드 추가
- 사다리 연결 여부에 따른 공백 또는 연결 판단 메서드 추가
- 참여할 사람의 수를 입력받아 라인을 만들어주는 메서드 추가
- 첫 번째 Point 반환 메서드 추가
- 중간 Point 반환 메서드 추가
- 마지막 Point 반환 메서드 추가
- 참여할 사람의 수가 1명일 경우와 2명 이상일 경우 Line을 만드는 메서드 분리
- 참여할 사람의 수와 사다리 높이를 입력받아 사다리를 만들어주는 메서드 추가
- 이름 값 포장을 위한 래퍼 클래스
- 이름 값 생성 규칙 예외 검증 메서드 추가
- 참여한 사람과 실행 결과 위치를 나타내는 객체
- 참여한 사람 객체 생성을 위한 클래스
- Point 객체가 가지고 있는 Direction에 따라 이동 메서드 추가
- 현재 위치 값을 기준으로 이동하는 메서드 추가
- Direction Enum 클래스 추가
- player 객체들의 이름을 반환해주는 메서드 추가
- 모든 플레이어들을 완주하게 하는 메서드 추가
- 각 Point의 좌우 라인 존재 유무 검사 후 Direction을 초기화
- 실행 결과 이름 원시값 포장 -> GameResultName
- gameResultName들을 반환하는 Dto 메서드 추가
- View의 역할은 구분자를 분리하고, 공백을 제거해주는 등과 같은 데이터를 최소한의 정리된 형태로 만들어, Controller로 넘겨주는 것까지라고 생각된다. 그래서 예외 처리는 Controller에서 해야된다고 생각한다.
- 참여할 사람 이름 입력받기 메서드 추가
- 사다리 높이 입력받기 메서드 추가
- 실행 결과 이름 입력받기 메서드 추가
- 문자열 처리 메서드 추가
- 결과를 보고 싶은 사람 입력받기 메서드 추가
- 참여한 사람 이름 출력 메서드 추가
- 실행 결과 이름 출력 메서드 추가
- 입력한 사람의 결과 보여주는 메서드 추가
sunghyuki and others added 29 commits February 21, 2022 21:17
- Line의 의미는 Point들이 모인 가로 한 라인을 의미하는데 이전 메서드 네임은 세로 한 줄이라는 의미를 가지고 있어서 혼란을 주기 때문에 세로 한줄이라는 의미를 지니는 Vertical을 추가
- 한 명의 참가자일 경우 LineGenerator 클래스의 generateFirstPoint 메서드를 호출해서 사다리를 만들 경우에 Point의 오른쪽 값이 false or true로 초기화됨으로 오른쪽 값이 false로 초기화되도록 generateOnlyFirstExist 메서드 추가
- Point 클래스의 오른쪽, 왼쪽값을 모두 false로 초기화시키는 onlyFirstExist 메서드 추가
- Point의 방향 초기화의 역할을 Line과 Ladder가 가지고 있을 역할은 아니라고 생각함으로 Point 생성자에서 Direction 초기화
- 이름 생성 규칙 예외 처리 클래스 생성
- 사다리 높이 규칙 예외 처리 클래스 생성
- Game 컨트롤러에서 예외를 잡을 수 있도록 입력값을 래퍼 클래스로 포장
- Code Formatting
- Players 객체에 결과를 보고 싶은 사람 이름이 존재하는지 판단하는 메서드 추가
- 결과를 보고 싶은 사람 한 명 이름 입력 시 결과를 출력하고 반복해서 결과를 보고 싶은 사람을 입력받는다.
- 모든 결과를 보고 싶을 때 결과를 출력하고 프로그램을 종료한다.
- player는 position값 변경에 대한 책임만을 갖기 때문에 사다리 한 층을 움직이고 모든 층을 움직이는 역할은 players에서 담당해야 한다고 생각한다.
- refactor : 참여자 이동과 관련된 메서드 수정
  - player는 position값 변경에 대한 책임만을 갖기 때문에 사다리 한 층을 움직이고 모든 층을 움직이는 역할은 players에서 담당해야 한다고 생각한다.
- 포인트가 오직 하나 존재할 경우를 포인트의 양쪽 연결 다리의 존재 여부는 거짓을 반환한다.
- 포인트가 하나 이상일 경우 첫 번째 포인트의 왼쪽 연결 다리는 거짓을 반환한다.
- 이전 포인트의 오른쪽 연결 다리가 존재하면 현재 포인트의 오른쪽 연결 다리는 거짓을 반환한다.
- 이전 포인트의 오른쪽 연결 다리가 존재하지 않으면 현재 포인트의 왼쪽 연결 다리는 거짓을 반환한다.
- 마지막 포인트의 오른쪽 연결 다리는 존재하지 않는다.
- 한 포인트의 왼쪽과 오른쪽 연결 다리가 존재하면 예외를 처리한다.
- 위치를 왼쪽으로 이동할 때 포인트의 x 좌표가 왼쪽으로 이동하는지 테스트한다.
- 위치를 오른쪽으로 이동할 때 포인트의 x 좌표가 오른쪽으로 이동하는지 테스트한다.
- 위치를 아래쪽으로 이동할 때 포인트의 y 좌표가 아래로 이동하는지 테스트한다.
- x 좌표 또는 y 좌표가 음수일 때 예외를 처리한다.
- 한 포인트의 왼쪽 연결 다리가 존재하고 오른쪽 연결 다리가 존재하지 않을 경우 방향을 왼쪽으로 반환한다.
- 한 포인트의 오른쪽 연결 다리가 존재하고 왼쪽 연결 다리가 존재하지 않을 경우 방향을 오른쪽으로 반환한다.
- 한 포인트의 양쪽 다리가 모두 존재하지 않을 경우 방향을 아래로 반환한다.
- 이름이 없을 경우 예외를 처리한다.
- 이름의 길이가 5글자를 초과할 경우 예외를 처리한다.
- 사다리의 높이가 음수일 경우 예외를 처리한다.
- 포인트의 방향이 왼쪽일 때 플레이어의 위치를 x좌표는 -1만큼 y좌표는 +1만큼 이동한다.
- 포인트의 방향이 오른쪽일 때 플레이어의 위치를 x좌표는 +1만큼 y좌표는 +1만큼 이동한다.
- 포인트의 방향이 아래일 때 플레이어의 위치를 y좌표는 +1만큼 이동한다.
- 결과를 보고 싶은 사람이 참여한 사람들에 포함되어 있다면 true를 반환한다.
- 결과를 보고 싶은 사람이 참여한 사람들에 포함되어 있지 않다면 false를 반환한다.
- 참여한 사람들의 수만큼 Player 객체를 생성한다.
- 이름이 없을 경우 예외를 처리한다.
- 참여한 사람들의 수만큼 Player 객체를 생성한다.
Copy link

@hyukjin-lee hyukjin-lee left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~
테스트도 최대한 작성하려고 노력하셨군요
질문 겸 피드백 몇개 남겼으니 확인 부탁드려요~

import java.util.Map;

public class PlayerGameResult {
public static Map<String, String> match(Players players, GameResults gameResults) {

Choose a reason for hiding this comment

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

이 클래스가 꼭 필요할까요? common 패키지에 위치한 이유와 각 책임을 다른 클래스들로 위임할 수 없을지 고민해보면 좋을 것 같습니다.

public static Map<String, String> match(Players players, GameResults gameResults) {
Map<String, String> result = new HashMap<>();
for (Player player : players.getPlayers()) {
String gameResultName = gameResults.getGameResults().stream()

Choose a reason for hiding this comment

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

command + ,
java code style
method chaining multi line
부분에 체크하고 정렬하시면 가독성 좋게 정렬됩니다 ~ 추천드려요

Map<String, String> result = new HashMap<>();
for (Player player : players.getPlayers()) {
String gameResultName = gameResults.getGameResults().stream()
.filter(gameResult -> gameResult.getPosition().equals(player.getPosition()))

Choose a reason for hiding this comment

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

get 으로 가져와서 구현하는 것보다 Tell don't ask 원칙을 적용해보면 어떨까요?


public class Game {
private InputView inputView;
private OutputView outputView;

Choose a reason for hiding this comment

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

상태값이 존재하지 않도록 구현해서 static 하게 사용해보면 어떨까요

}

public void play() {
Players players = PlayersFactory.from(inputPlayer());

Choose a reason for hiding this comment

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

Players 가 아닌 Factory 클래스를 도입했을 때 어떤 장점이 있을까요?

}
}

public Direction checkConnection() {

Choose a reason for hiding this comment

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

check 라는 메서드는 Direction 을 반환하기에 적절하지 않은 메서드명인 것 같습니다.
getConnectedDirection() 은 어떨까요


import java.util.Random;

public class RandomPointGenerator {

Choose a reason for hiding this comment

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

역할은 Random 을 감싼 wrapper 클래스인데 Point 라는 제한을 이름으로 주게 되는 것 같습니다.
RandomGenerator util 로 보면 어떨까요


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

class LineTest {

Choose a reason for hiding this comment

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

good 테스트 잘 작성하셨네요

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

class PlayerTest {
@Test

Choose a reason for hiding this comment

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

비슷한 케이스만 다양해지는 경우 @ParameterizedTest
를 활용해볼 수 있을지 검토해보면 좋을 것 같습니다~

@DisplayName("참여한 사람들의 수만큼 Player 객체를 생성한다.")
void testgeneratedPlayersSize() {
// given
Name 김성혁 = new Name("김성혁");

Choose a reason for hiding this comment

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

프로젝트 인코딩 문제가 발생할 수 있으니 웬만하면 변수명에 한글사용은 자제하는게 좋을 것 같습니다~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants