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

feat : 우승자가 있는 자동차 경주 #48

Open
wants to merge 3 commits into
base: 6161990
Choose a base branch
from

Conversation

6161990
Copy link

@6161990 6161990 commented Aug 23, 2023

No description provided.

@6161990 6161990 requested a review from moon9ua August 23, 2023 12:09
@6161990 6161990 changed the title feat : 게임 결과에 자동차 이름을 출력한다. feat : 우승자가 있는 자동차 경주 Aug 23, 2023
@@ -4,36 +4,71 @@ import com.yoon.racingCar.domain.f1.TrackResult

object GameGuide {

fun entry(): Pair<ParticipateCount, RaceCount> {
fun entry(): Pair<Participate, RaceCount> {
Copy link

Choose a reason for hiding this comment

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

Pair도 원시 타입이라고 생각 되어서 Dto로 묶어서 표현 했는데 다른분들 생각은 어떠신지

Copy link

Choose a reason for hiding this comment

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

저도 제코드 짜면서 궁금했던 점인데.. 역시 dto로 묶는게 권장되는 방법이겠죠?? (모름)

Choose a reason for hiding this comment

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

타입으로 의미를 명확하게 알 수 있으면 Pair<> 정도는 괜찮다고 생각하긴해요.

근데 여러가지를 합쳐서 반환하는 메서드라면 별도의 class를 만드는걸 저는 선호하긴 합니다. 😀
class로 분리를 안하게 되면 나중에 필드를 추가하거나 수정할 때 변경 범위가 넓어지더라구요.


private fun readTeamListInput(prompt: String): List<String> {
var value: String
var teamList: List<String> = mutableListOf()
do {
Copy link

Choose a reason for hiding this comment

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

do while문 오랜만에 보네요 ㅎㅎ 👍

Copy link

@moon9ua moon9ua left a comment

Choose a reason for hiding this comment

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

저는 우승자 기능을 아예 따로 추가하려고 했는데, 기존 코드에서 수정하셨군요 👀
기능(메서드)를 추가하는 방식으로 구현해보는 것도 공부에 도움이 될 것 같습니다!
approve 드립니당 고생 많으셨습니다!!

fun factory(participationCount: ParticipateCount): List<RacingCar> {
require(participationCount.value > 0) { throw IllegalArgumentException("ParticipationCount more then 0") }
fun factory(participate: Participate): List<RacingCar> {
require(participate.participates.isNotEmpty()) { throw IllegalArgumentException("ParticipationCount more then 0") }
Copy link

Choose a reason for hiding this comment

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

require(participate.participates.isNotEmpty()) { "ParticipationCount more then 0" } 요론 식도 괜찮을 것 같습니다!

@@ -1,6 +1,7 @@
package com.yoon.racingCar.domain.car

data class RacingCar(
val carName: String,
Copy link

Choose a reason for hiding this comment

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

VO 관련은 아직 잘 모르겠지만.. 요런 값도 포장(VO)하면 좋은게 아닐까(?) 싶습니다!

Copy link

Choose a reason for hiding this comment

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

앗 근데 RacingCar 자체가 data class였군요 👀 그러면 이런 경우에는 보통 carName을 따로 포장하지는 않는 것일까요..??? 이 부분을 잘 몰라서 공부하고 추가 댓글 적어보겠습니다 ㅠ

Copy link
Member

Choose a reason for hiding this comment

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

비즈니스적으로 유의미한 값이라면 VO 로 포장해서 사용하는게 좋다고 생각해요 👍
저는 유의미한지 판단을 "나중에 자동차 이름 정책을 수정할 때 자동차의 다른 정보들과 함께 수정할까? 아니면 자동차 이름 정책만 수정할까?" 로 기준삼아 하는 편이에요.

@@ -10,7 +10,7 @@ class CarFactoryTest{
@Test
@DisplayName("참가자 수만큼 Racing Car 가 생성된다.")
fun name() {
val actual = CarFactory.factory(ParticipateCount(3))
val actual = CarFactory.factory(Participate(listOf("페라리", "메르세대스", "레드불")))
Copy link

Choose a reason for hiding this comment

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

😆

@@ -23,7 +22,7 @@ class F1Test{
@DisplayName("F1 경기가 끝나고 나면 RaceCount 만큼 경기 결과가 반환된다")
fun name2() {
sut = F1(threshold(3), RaceCount(3))
val racingCar = RacingCar(RacingConstructor(anyMin, anyMax))
val racingCar = RacingCar("any", RacingConstructor(anyMin, anyMax))
Copy link

Choose a reason for hiding this comment

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

val carName: String = "any", 같은 방식으로 carName에 대해 기본값을 넣어줘도 괜찮을 것 같습니다

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.

안녕하세요 윤지님, 구화님 리뷰에 간단한 코멘트 덧붙였습니다.

Comment on lines 44 to +54
private fun readValidInput(prompt: String): Int {
var value: Int
while (true) {
println(prompt)
val input = readlnOrNull()?.toIntOrNull()
if (input != null && input > 0) {
return input
} else {
retryMessage()
}
}
}
Copy link
Member

Choose a reason for hiding this comment

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

아하~ 재입력을 요청하는 것 까지 view 의 로직이죠 👍

@@ -1,6 +1,7 @@
package com.yoon.racingCar.domain.car

data class RacingCar(
val carName: String,
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 +61 to +68
value = readlnOrNull().toString()
if(value.isEmpty()){
retryMessage()
}else{
teamList = value.split(",").map { it.trim() }
}
} while (value < 1)
return value
} while (value.isEmpty())
return teamList
Copy link
Member

Choose a reason for hiding this comment

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

else 예약어를 쓰지 않는다.

판별 로직을 메서드로 분리하면서 이름을 부여하면 어떨까요?

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.

5 participants