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

Step3 : 자동차 경주 #1512

Open
wants to merge 4 commits into
base: sendkite
Choose a base branch
from
Open

Step3 : 자동차 경주 #1512

wants to merge 4 commits into from

Conversation

sendkite
Copy link

@sendkite sendkite commented Nov 4, 2023

리뷰어님 안녕하세요.

3단계 미션 PR 드립니다.
잘 부탁드립니다!

임의의 Random 숫자 테스트를 위해 인터페이스를 활용하여 Mock 객체를 사용했습니다.

설계

  1. RacingGame : 게임을 시작하고, 결과를 print
  2. Car: 게임에 참가하는 자동차, position이 있고 조건에 따라 +1
  3. RandomNumberHolder: 자동차에 이동 조건인 0~9 사이 무작위 수
  4. InputView, OutputView: 게임 시작 값 input, 결과값 output

@sendkite sendkite changed the title Step3 Step3 : 자동차 경주 Nov 4, 2023
Copy link

@kyucumber kyucumber left a comment

Choose a reason for hiding this comment

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

안녕하세요

몇가지 코멘트 남겨두었으니 참고 부탁드립니다.

@@ -0,0 +1,10 @@
package racingcar.mock

Choose a reason for hiding this comment

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

테스트 더블에서 mock은 호출에 대한 기대를 명세하고 내용에 따라 동작하도록 프로그래밍 된 객체를 의미합니다.

현재는 호출에 대한 기대를 정의하지 않고(어떤 메서드 호출 시 어떤 값이 반환될 지 정의하는 것) 정해진 값을 리턴하고 있으므로 Mock보다는 Stub에 가깝지 않나 하는 생각이 드네요.

자세한 내용은 아래 문서 한번 참고 부탁드립니다.

https://tecoble.techcourse.co.kr/post/2020-09-19-what-is-test-double/

package racingcar

class Car(
var position: Int = 0,

Choose a reason for hiding this comment

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

지금과 같은 구조라면 move를 호출하지 않고도 외부에서 가변인 position의 값을 바꿀 수 있을 것 같네요. 이 부분에 대해 고민해보시면 좋겠습니다.

Comment on lines +5 to +8
var moveCondition: RandomNumberHolder
) {
fun move() {
if (moveCondition.getRandomNumber() >= 4) {

Choose a reason for hiding this comment

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

추상화의 범위를 어떻게 잡을 지 정답은 없으나 여기서는 랜덤 넘버를 반환하는 것 보다는 움직일지 말지 여부를 결정하는 부분을 추상화하는게 어땠을까 하는 생각이 드네요. 이 부분에 대해서도 고민해보시면 좋겠습니다.

정답은 없으므로 변경하진 않으셔도 됩니다.


data class InputView(
var gameCount: Int = 0,
var cars: List<Car> = listOf()

Choose a reason for hiding this comment

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

아래 프로퍼티로 정의할 필요가 없을 것 같다고 말씀드리긴 했는데요, 가변 프로퍼티라 추가로 코멘트 남깁니다.
가변성과 관련해서는 아래 내용 한번 참고해보시면 좋겠습니다.

이펙티브 코틀린 아이템 1 - 가변성을 제한하라

Comment on lines +4 to +5
var gameCount: Int = 0,
var cars: List<Car> = listOf()

Choose a reason for hiding this comment

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

둘 다 프로퍼티로 정의할 필요가 있을까요? 필요한 값을 바로 반환할 순 없을까요?

}

fun validate(input: Int) {
require(input > 0) { throw IllegalArgumentException("음수는 입력할 수 없습니다.") }

Choose a reason for hiding this comment

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

require은 조건에 따른 결과가 맞지 않을 경우 IllegalArgumentException을 반환하기 때문에 예외 메시지만 남겨주시면 될 것 같습니다.

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