Skip to content

[RIPL] 이준석 자동차 경주 미션 제출합니다.#201

Open
Joonseok-Lee wants to merge 54 commits into
next-step:joonseok-leefrom
Joonseok-Lee:joonseok-lee
Open

[RIPL] 이준석 자동차 경주 미션 제출합니다.#201
Joonseok-Lee wants to merge 54 commits into
next-step:joonseok-leefrom
Joonseok-Lee:joonseok-lee

Conversation

@Joonseok-Lee

Copy link
Copy Markdown

안녕하세요 대길님~ 반갑습니다! 만나뵙게 되어 영광이에요 ㅎ_ㅎ
이번에 스터디에 참여하게 된 이준석이라고 합니다!
초록 스터디를 진행하는 동안 잘 부탁드릴게요~~!!

Java를 배운 지는 1년이 조금 넘었고, Spring 프로젝트 경험은 얕게 5개 정도 진행해보았어요!
숙련되었다고는 할 수 없고, 이제 막 햇병아리 딱지를 뗀 느낌이네요.
테스트 코드를 작성하는 것은 이번이 처음입니다. 관련해서 어렵다고 느꼈던 부분부터 말씀드릴게요!

이번 미션에서 어려웠던 점

아무래도 순수 Java로 프로젝트를 진행하고, 테스트 코드를 적는 것은 제가 생각했던 것보다 더 깔끔하게 설계를 해야 할 것 같다고 느꼈어요.
테스트 코드를 작성할 때, 메소드 단위로 테스트를 진행하고자 생각했었는데, 테스트할 때 난수, 비동기적인 로직이 들어가면 안 된다고 알고 있어서,
해당 부분을 어떻게 분리할지 고민을 많이 했었어요. 특히 System.out과 System.in이 많이 어려웠습니다.

고려는 했지만 추가하지 않은 부분이 InputView 객체에서 차량 이름을 입력받는 것과, 몇 회 진행할 것인지 입력받는 부분을 예외처리를 할까 생각을 했었지만, 또다시 System.in, System.out에 의존하고 있기 때문에 테스트 코드를 적는 법을 잘 모르겠어서 하지 않았습니다. 이런 부분에 대해서는 어떻게 분리하면 좋을까요?

리뷰에서 중점적으로 봐주셨으면 하는 부분

현재 게임이 2칸 이동한 차량과 1칸 이동한 차량을 직접 설정한 후, 턴 수를 0턴으로 주어 2칸 이동한 차량이 항상 우승하는지 테스트하는 로직이 있습니다. 이를 위해 MovableCar에서 난수 생성하던 것을 중간부터 난수 생성 구현체와 해당 interface로 분리하고, 테스트 메소드에서는 익명 클래스, 함수로 전달하게끔 수정했었구요. 이런 패턴이 흔하게 적용되는 것인지 궁금합니다. 로직 상 2칸 이동한 것과 1칸 이동한 것을 비교하면 2칸 이동한 것이 우승하는 것은 자명하다고 생각하지만, 그럼에도 불구하고 테스트로 반드시 작성해야하는지 궁금해요!

메소드 별로 접근자를 나름대로 지정해보려고 노력했는데, 잘 지정한 것인지도 궁금합니다.

저 혼자만의 힘으로 과제 진행이 어려워서, 리팩토링 힌트에 있는 대로 psvm을 작성했는데, psvm은 최소한의 코드로 작성되어야 한다고 배웠었거든요. 그렇게 된다면 반드시 지금 psvm 안의 코드를 다른 클래스로 옮기고(예를 들자면, RacingGameApplication.class요!), 정적 메소드로 1줄만 작성해야 할까요? Spring Boot의 psvm처럼요!

…eam으로 순회하여 name의 List를 파싱하고, 그 값을 출력하도록 수정
…s 내에서 getHighestDistance를 호출하는 것으로 메소드 간 종속 제거
…부 난수 판단하는 메소드를 추상화. 실제 게임 실행 시에는 난수 생성으로 DI하며, 테스트할 땐 익명 클래스로 처리하기 위해 Movable intreface로 분리
… 호출할 때, 주입받은 movable.isMovable을 호출하도록 수정

@minzzun99 minzzun99 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

안녕하세요 준석님!
이번 자동차 경주 미션의 리뷰를 맡게 된 강민준입니다!

남겨주신 PR 본문과 코드를 읽어보고 리뷰를 남겼습니다.
시간 괜찮으실 때 확인 부탁드리겠습니다!

리뷰를 진행하면서 코드 곳곳에서 테스트와 책임 분리에 대해 많이 고민하신 것 같았습니다.
이번 리뷰에서는 제 생각을 정답처럼 전달하기보다, 준석님께서 하신 설계의 의도와 판단 근거를 여쭙고 제 의견을 함께 나누는 방식으로 진행하고자 합니다!

제 개인적으로 코드를 구현할 때 하나의 정답을 찾기보다는, 현재의 구조를 선택한 이유와 근거를 고민해 보는 과정이 중요하다고 생각합니다!

또한 저는 미션에서 주어진 요구사항을 빠짐없이 지키는 것도 중요하다고 생각합니다.
현재 구현에서 반영되지 않은 요구사항은 없는지 다시 한번 확인해 보시고, 리뷰를 반영하는 과정에서 함께 점검해 주시면 좋겠습니다.

아래는 PR 본문에 남겨주셨던 질문에 대한 내용입니다!

해당 부분을 어떻게 분리할지 고민을 많이 했었어요. 특히 System.out과 System.in이 많이 어려웠습니다.

System.in, System.out에 의존하고 있어 테스트하기 어려웠다고 작성해 주셨는데요.
콘솔에서 값을 입력받는 행위와 입력받은 값이 유효한지 검증하는 행위는 같은 책임일까요?
예를 들어 자동차 이름을 콘솔에서 직접 입력받는 과정과, 전달된 자동차 이름의 개수나 길이, 중복 여부 등을 확인하는 과정은 구분해서 생각해 볼 수 있을 것 같습니다.
후자의 로직도 반드시 System.in, System.out에 의존해야 하는지 고민해 보면 좋겠습니다.

외부에서 값을 가져오는 로직과 전달받은 값을 검증하는 로직을 분리한다면, 어떤 부분까지 입출력 없이 테스트할 수 있을까요?
이를 바탕으로 코드에서 어떤 대상에 의존한다는 것이 무엇을 의미하는지도 함께 생각해 보면 좋을 것 같습니다!

저 혼자만의 힘으로 과제 진행이 어려워서, 리팩터링 힌트에 있는 대로 psvm을 작성했는데, psvm은 최소한의 코드로 작성되어야 한다고 배웠었거든요. 그렇다면 반드시 지금 psvm 안의 코드를 다른 클래스로 옮기고, 정적 메서드 한 줄만 작성해야 할까요? Spring Boot의 psvm처럼요!

main 메서드를 반드시 한 줄로 작성해야 하는 것은 아니라고 생각합니다.
코드의 길이보다, main 메서드가 애플리케이션의 진입점으로서 어디까지 책임질 것인지 결정하는 것이 더 중요하다고 생각합니다.
현재 main이 애플리케이션의 실행을 시작하는 역할뿐만 아니라 구체적인 실행 순서와 세부 로직까지 알고 있는데요.
이러한 책임이 진입점에 속하는지, 실행 흐름이 변경될 때 main이 어떤 영향을 받게 될지를 기준으로 현재 구조를 다시 한번 고민해 보면 어떨까요?


남겨드린 질문에 정해진 답이 있다고 생각하지는 않습니다.
준석님께서 코드를 작성하며 생각하셨던 의도와 근거를 편하게 공유해주시면 저도 함께 고민해 보겠습니다!

@@ -0,0 +1,86 @@
package domain;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

준석님께서 이번 미션을 구현하면서 어떤 대상을 핵심 도메인으로 생각하셨는지 궁금해요!
그렇게 판단하신 이유도 함께 들어보고 싶습니다.

this.movable = movable;
}

private void carSetInit(List<String> carNames) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

현재 사용되지 않는 메서드와 코드는 제거해도 좋을 것 같습니다.
불필요한 코드가 남아 있으면 실제로 필요한 로직을 파악하기 어렵고 유지보수에도 혼란을 줄 수 있다고 생각합니다.
혹시 남겨두신 이유가 있을까요?


public class RacingGame {

private Set<MovableCar> carSet = new HashSet<>();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

자동차 컬렉션으로 Set을 선택하신 이유와, 해당 필드에 final을 사용하지 않으신 이유가 궁금합니다!


import java.util.List;

public class ResultView {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

ResultView가 담당하도록 의도하신 역할과 책임의 범위가 궁금합니다.

Comment thread src/main/java/Main.java

import java.util.Set;

public class Main {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Main에서 전체 실행 순서를 직접 제어하도록 설계하신 이유가 궁금합니다.
Main은 실행을 담당할 객체를 생성하고 호출하는 역할만 맡도록 책임을 분리해 볼 수도 있을 것 같아요.


import java.util.Random;

public class MovableCar {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

MovableCar라는 네이밍이 이 객체의 역할과 책임을 충분히 표현하고 있을까요?
현재 로직에서 자동차는 매 턴마다 조건에 따라 이동 여부가 달라지는데, Movable이라는 표현을 사용하신 의도가 궁금합니다.

moveAllCar();
printTurnResult();
}
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

도메인 객체에서 출력까지 직접 수행하도록 설계하신 이유가 궁금합니다.
출력 방식이나 형식이 변경된다면 도메인 객체에는 어떤 영향이 생길까요?


// then
assertThat(game.getWinnerNames()).containsExactly("move_2");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

로직 상 2칸 이동한 것과 1칸 이동한 것을 비교하면 2칸 이동한 것이 우승하는 것은 자명하다고 생각하지만, 그럼에도 불구하고 테스트로 반드시 작성해야하는지 궁금해요!

말씀하신 것처럼 2칸 이동한 자동차가 1칸 이동한 자동차보다 우승한다는 도메인 규칙 자체는 당연하다고 생각합니다.
다만 규칙이 당연하다는 사실과 코드가 그 규칙을 올바르게 구현했다는 것은 별개이지 않을까요?
테스트를 작성하는 목적이 무엇인지 함께 생각해 보면 좋을 것 같아요.

@Override
public int isMovable() {
return new Random().nextInt(10);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

테스트할 때 난수, 비동기적인 로직이 들어가면 안 된다고 알고 있어서

“테스트에 난수나 비동기적인 로직이 들어가면 안 된다”고 작성해 주셨는데요. 그렇게 생각하신 이유가 궁금합니다!

.toList();
}

private List<MovableCar> getWinnerList(int highestDistance) {

Copy link
Copy Markdown

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.

2 participants