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

0단계 - JUnit 5 학습 #775

Merged
merged 1 commit into from
Jan 27, 2025
Merged

0단계 - JUnit 5 학습 #775

merged 1 commit into from
Jan 27, 2025

Conversation

jaeykweon
Copy link

@jaeykweon jaeykweon commented Jan 26, 2025

안녕하세요 리뷰어님

0단계 리뷰 요청드립니다.

회사 업무로 늦게 미션을 시작하여, 시간 관계상 커밋까지 분리하면서 하지는 않았습니다.

그래도 가능한 변경 사항 등을 고려하여 코드 작성하였습니다.

주석과 코멘트로 안내 및 질문사항 남겨놓았습니다.

감사합니다

Comment on lines +12 to +16
public Car(String name, int initPosition, MoveStrategy moveStrategy) {
this.name = new CarName(name);
this.position = new CarPosition(initPosition);
this.moveStrategy = moveStrategy;
}
Copy link
Author

Choose a reason for hiding this comment

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

생성자에서 이처럼 원시값을 받는 것이 좋을까요? 아니면 값 객체를 받는 것이 좋을까요?

Copy link
Member

Choose a reason for hiding this comment

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

생성자는 편의에 따라서 만들어주면 될 것 같은데요~
저는 생성자가 "객체가 다룰 수 있는 정보를 나타내주는 장치" 역할도 수행한다고 생각해서
VO 를 파라미터로 전달받는 생성자와, 개발 편의를 위한 원시값 생성자를 모두 만드는 편입니다 😄

Copy link
Author

Choose a reason for hiding this comment

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

그렇군요 답변 감사합니다

Comment on lines +18 to +24
public void move(int input) {
this.position = moveStrategy.getNextPosition(position, input);
}

public int getPosition() {
return position.getCurrentPosition();
}
Copy link
Author

Choose a reason for hiding this comment

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

위와 이어지는 질문인데, 값 객체를 반환하는 것이 좋을까요 원시값을 반환하는 것이 좋을까요?

Copy link
Member

Choose a reason for hiding this comment

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

getter 역시 비즈니스 로직을 수행하는 메서드 아닐까요?
재용님께서 생각하시기에 어떤 형태가 자동차 경주 비즈니스에 더 적합할까요?

Copy link
Author

Choose a reason for hiding this comment

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

현재로써는 비지니스로직이 별게 없어 원시값으로도 충분해보이지만,
추후 위치를 가지고 모종의 로직을 실행하게된다면 값객체를 넘기는것도 괜찮아보입니다
현구막님께서는 실무에서 어떻게 사용하고 계신지 알 수 있을까요?

@Override
public CarPosition getNextPosition(CarPosition position, int input) {
if (input >= threshold) {
return position.move(1);
Copy link
Author

Choose a reason for hiding this comment

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

매직넘버긴 하지만, 이 한 곳만 사용하므로 읽기 쉽게하려고 이렇게 하였습니다
사용하는 곳이 2곳 이상이 되면 따로 빼려합니다

Copy link
Member

Choose a reason for hiding this comment

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

그렇게 고민해주신 덕분에 더욱 직관적이네요 😉 👍

@DisplayName("임계값_4의_threshold_전략을_가진_차는_입력값이_4이상일_때_한칸_이동한다")
@ParameterizedTest
@ValueSource(ints = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9})
void threshold_전략을_가진_차(final int input) {
Copy link
Author

Choose a reason for hiding this comment

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

이미 값 객체에서 다 테스트를 해버려서 이것 말고는 딱히 생각나는 테스트가 없는데,
따로 또 검증해볼 만한 것이 있을까요?

Copy link
Member

Choose a reason for hiding this comment

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

값 객체를 통해 단위 테스트가 모두 완료되었다면 굳이 불필요한 테스트를 작성할 필요는 없죠 😉
테스트가 적으면 적을 수록, 존재하는 테스트들의 중요도가 더욱 부각될테구요!

Copy link
Author

Choose a reason for hiding this comment

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

감사합니다

Copy link
Member

@Hyeon9mak Hyeon9mak left a comment

Choose a reason for hiding this comment

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

안녕하세요 재용님~ 첫번째 미션 리뷰를 맡게된 최현구입니다. 🙇‍♂️
0단계 잘 진행해주셨습니다! 간단한 코멘트 및 답변 드렸으니 확인해주세요.
다음 단계에서 뵙겠습니다~!

Comment on lines +12 to +16
public Car(String name, int initPosition, MoveStrategy moveStrategy) {
this.name = new CarName(name);
this.position = new CarPosition(initPosition);
this.moveStrategy = moveStrategy;
}
Copy link
Member

Choose a reason for hiding this comment

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

생성자는 편의에 따라서 만들어주면 될 것 같은데요~
저는 생성자가 "객체가 다룰 수 있는 정보를 나타내주는 장치" 역할도 수행한다고 생각해서
VO 를 파라미터로 전달받는 생성자와, 개발 편의를 위한 원시값 생성자를 모두 만드는 편입니다 😄

Comment on lines +18 to +24
public void move(int input) {
this.position = moveStrategy.getNextPosition(position, input);
}

public int getPosition() {
return position.getCurrentPosition();
}
Copy link
Member

Choose a reason for hiding this comment

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

getter 역시 비즈니스 로직을 수행하는 메서드 아닐까요?
재용님께서 생각하시기에 어떤 형태가 자동차 경주 비즈니스에 더 적합할까요?

Comment on lines +7 to +9
public record CarName(String name) {

static final int MAX_CAR_NAME = 5;
Copy link
Member

Choose a reason for hiding this comment

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

private 으로 숨겨줄 수 있겠네요 😉

Copy link
Author

Choose a reason for hiding this comment

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

수정하겠습니다

Comment on lines +3 to +7
/**
* 현재 위치를 기반으로 차의 이동 값이 결정되는 요구사항을 대비하여 현재 위치도 입력으로 받음
*/
@FunctionalInterface
public interface MoveStrategy {
Copy link
Member

Choose a reason for hiding this comment

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

함수형 인터페이스 선언 👍

Comment on lines +8 to +9
CarPosition getNextPosition(CarPosition position, int input);
}
Copy link
Member

Choose a reason for hiding this comment

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

자동차 이동전략 입장에서 input 이라는 표현을 알 필요가 있을까요? 🤔

Copy link
Author

Choose a reason for hiding this comment

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

입력 값이라는 의미에서 input이라고 사용하였습니다!

@Override
public CarPosition getNextPosition(CarPosition position, int input) {
if (input >= threshold) {
return position.move(1);
Copy link
Member

Choose a reason for hiding this comment

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

그렇게 고민해주신 덕분에 더욱 직관적이네요 😉 👍

Comment on lines +23 to +39
@ParameterizedTest
@DisplayName("차는 1~5 글자의 이름을 가질 수 있다")
@ValueSource(strings = {"레", "레진", "레진코", "레진코믹", "레진코믹스"})
void 차_이름_생성_성공(final String name) {

assertThatNoException()
.isThrownBy(
() -> new CarName(name)
);
}

@ParameterizedTest
@DisplayName("차 이름 생성 실패시 IllegalArgumentException 발생한다")
@ValueSource(strings = {"", "레진코믹스고"})
void 차_이름_생성_실패(final String name) {

assertThatThrownBy(() -> new CarName(name))
Copy link
Member

Choose a reason for hiding this comment

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

경계값을 이용한 테스트 👍

assertThatThrownBy(() -> new CarName(name))
.isInstanceOf(IllegalArgumentException.class);
}
}
Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Author

Choose a reason for hiding this comment

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

수정하겠습니다

@DisplayName("임계값_4의_threshold_전략을_가진_차는_입력값이_4이상일_때_한칸_이동한다")
@ParameterizedTest
@ValueSource(ints = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9})
void threshold_전략을_가진_차(final int input) {
Copy link
Member

Choose a reason for hiding this comment

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

값 객체를 통해 단위 테스트가 모두 완료되었다면 굳이 불필요한 테스트를 작성할 필요는 없죠 😉
테스트가 적으면 적을 수록, 존재하는 테스트들의 중요도가 더욱 부각될테구요!

@Hyeon9mak Hyeon9mak merged commit 06a0961 into next-step:jaeykweon Jan 27, 2025
Copy link
Author

@jaeykweon jaeykweon left a comment

Choose a reason for hiding this comment

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

자세한 리뷰 감사합니다
수정할 내용은 다음 step1에서 수정하도록 하겠습니다

Comment on lines +12 to +16
public Car(String name, int initPosition, MoveStrategy moveStrategy) {
this.name = new CarName(name);
this.position = new CarPosition(initPosition);
this.moveStrategy = moveStrategy;
}
Copy link
Author

Choose a reason for hiding this comment

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

그렇군요 답변 감사합니다

Comment on lines +18 to +24
public void move(int input) {
this.position = moveStrategy.getNextPosition(position, input);
}

public int getPosition() {
return position.getCurrentPosition();
}
Copy link
Author

Choose a reason for hiding this comment

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

현재로써는 비지니스로직이 별게 없어 원시값으로도 충분해보이지만,
추후 위치를 가지고 모종의 로직을 실행하게된다면 값객체를 넘기는것도 괜찮아보입니다
현구막님께서는 실무에서 어떻게 사용하고 계신지 알 수 있을까요?

Comment on lines +7 to +9
public record CarName(String name) {

static final int MAX_CAR_NAME = 5;
Copy link
Author

Choose a reason for hiding this comment

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

수정하겠습니다

Comment on lines +8 to +9
CarPosition getNextPosition(CarPosition position, int input);
}
Copy link
Author

Choose a reason for hiding this comment

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

입력 값이라는 의미에서 input이라고 사용하였습니다!

assertThatThrownBy(() -> new CarName(name))
.isInstanceOf(IllegalArgumentException.class);
}
}
Copy link
Author

Choose a reason for hiding this comment

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

수정하겠습니다

@DisplayName("임계값_4의_threshold_전략을_가진_차는_입력값이_4이상일_때_한칸_이동한다")
@ParameterizedTest
@ValueSource(ints = {0, 1, 2, 3, 4, 5, 6, 7, 8, 9})
void threshold_전략을_가진_차(final int input) {
Copy link
Author

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