-
Notifications
You must be signed in to change notification settings - Fork 452
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
[2단계 - 자동차 경주 구현] 유콩(김유빈) 미션 제출합니다. #391
Changes from all commits
6f84862
823ba76
2766eaf
7d1fac0
76d2675
0921832
a7b6a2c
d766f78
ff3031d
3336910
d25e076
c31485e
b2e7753
8a04ed9
9b61954
e9fd026
c2d5cef
cdb6e31
19f23e9
58a2197
c75be9b
6e15a7a
ee098a8
81803ce
04800df
2909795
03bd0fe
8f08141
7030762
39f2a98
2610e18
50a31a2
be7e5ac
3d7f62f
b45edd0
9501d26
5abeaa3
81d254f
10557cf
32b0a84
2f02b83
8de85d1
bb98fd9
d5d875a
7593a3f
f93c576
2a4418d
dfe5502
a02d824
ea1e12a
0bedfd4
7297c7a
ad6a6ca
3c6c3ec
bd5f929
a26dda5
6e59f11
195dbab
7350989
203af30
d4ef0a5
51a48f4
054c5e4
8fe382d
ee23ac8
2b376c2
d2cdbc3
b18a5ca
c2be287
6d79eba
4203c14
63556a8
8af5cec
3847bc1
ec7b11c
18d359b
d183a03
802b5a0
fcc2b4f
9d1b043
57c11ce
8a98f49
daae066
f6f715d
eb20782
178d770
64b3c4d
4100c4d
412e0c2
c05f462
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,3 +30,6 @@ out/ | |
|
||
### VS Code ### | ||
.vscode/ | ||
|
||
### mac ### | ||
.DS_Store |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
package calculator.view; | ||
|
||
public class Output { | ||
private static void info(final String message) { | ||
System.out.println("[INFO] " + message); | ||
} | ||
|
||
public static void emptyValueOfSum() { | ||
info("빈 값은 0으로 바뀌어 계산됩니다."); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,61 @@ | ||
package racingcar.controller; | ||
|
||
import racingcar.domain.Attempt; | ||
import racingcar.domain.Cars; | ||
import racingcar.domain.RoundResult; | ||
import racingcar.view.Input; | ||
import racingcar.view.Output; | ||
|
||
import java.util.List; | ||
|
||
public class RacingCarGame { | ||
private Cars cars; | ||
private Attempt attempt; | ||
private final RoundResult roundResult = new RoundResult(); | ||
|
||
private final Input input; | ||
|
||
public RacingCarGame() { | ||
input = new Input(); | ||
} | ||
|
||
public void run() { | ||
init(); | ||
round(); | ||
win(); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 메소드 위치의 절대적으로 옳은 위치는 없지만, toString이 단순히 값을 편히 보기위한 편의기능이라면 아래의 private메소드들이 이 객체의 역할을 더 드러내는 것으로 보여 프라이빗과 투스트링의 위치를 수정하는건 어떨까요? |
||
return "- cars\n" + cars + "- " + attempt; | ||
} | ||
|
||
private void init() { | ||
cars = createCars(input.carName()); | ||
attempt = createAttempt(input.attempt()); | ||
} | ||
|
||
private Cars createCars(final String carName) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 요 부분은 취향차이긴 하지만, 저는 new Cars(carName) 자체만으로도 createCars라는 의미를 충분히 전달하는 것 같습니다. |
||
return new Cars(carName); | ||
} | ||
|
||
private Attempt createAttempt(final String attempt) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto |
||
return new Attempt(attempt); | ||
} | ||
|
||
private void round() { | ||
Output.resultTitle(); | ||
|
||
int nowAttempt = 0; | ||
while (!attempt.isSame(nowAttempt++)) { | ||
cars.play(roundResult); | ||
roundResult.roundEnd(); | ||
} | ||
Output.result(roundResult); | ||
} | ||
|
||
private void win() { | ||
List<String> winners = cars.findWinners(); | ||
Output.showResult(winners); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,21 +6,17 @@ public class Car implements Comparable<Car> { | |
private final String name; | ||
private int position = 0; | ||
|
||
public Car(String name) { | ||
checkValid(name); | ||
public Car(final String name) { | ||
checkValidName(name); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 생성자 오버로딩을 사용할 때는 부생성자들이 항상 마지막엔 주 생성자를 호출하는 형태가 일반적인데요. 그 이유에 대해 고민해보면 좋을 것 같아요. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 중복되는 코드를 줄일 수 있고 객체 정보가 수정될 경우 모든 생성자를 전부 수정해야 하기 때문에 생성자를 타고 들어가는 형태를 사용하는 것 같습니다. 수정했습니다! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 잘 수정해주신 것 같습니다. 조금 더 이어서, 두 생성자 중 현재는 2번째 생성자가 1번 생성자를 호출하고 있는데, 이 방향이 맞을까요? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 호출하는 방향을 바꾸었으나 왜 방향을 바꾸는 것이 좋은지는 모르겠습니다.🥲 파라미터의 개수가 많은 생성자가 더 적은 생성자를 호출 후 이후에 나머지 값을 대입하는 방식은 중요 변수를 대입하는 과정을 놓칠 수 있기 때문인가요?? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 제가 말씀드린 이유는 각 파라미터에 대한 일관된 검증을 하기 위함인데요. String name만 가지고 생성하는 생성자의 경우 Position 에 대한 Validation 을 수행하지 않기에 발생할 수 있는 이슈들을 제거하기 위해 호출의 방향을 변경하는 것이 좋을 것 같다고 생각했습니다. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. name 만으로 생성자를 호출할 경우 검증이 필요없는 0이 position 에 대입되므로 검증이 필요 없을 거라 생각했습니다. 혹시나 다른 개발자가 초깃값을 수정할 가능성이 있기에 잘못된 객체 생성을 막기 위해서는 검증이 포함되는 구조로 바꾸는 것이 좋겠군요. 테스트를 위한 생성자를 삭제하면서 해당 생성자는 삭제했지만 다음 미션때는 그 부분을 고려하도록 하겠습니다. 감사합니다! |
||
this.name = name; | ||
} | ||
|
||
public void drive(boolean directing) { | ||
if (directing) { | ||
public void drive(final Movable movable) { | ||
if (movable.isMoving()) { | ||
move(); | ||
} | ||
} | ||
|
||
public boolean isSamePosition(Car other) { | ||
return this.position == other.position; | ||
} | ||
|
||
public String getName() { | ||
return name; | ||
} | ||
|
@@ -29,17 +25,26 @@ public int getPosition() { | |
return position; | ||
} | ||
|
||
public boolean isSamePosition(final Car other) { | ||
return this.position == other.position; | ||
} | ||
|
||
@Override | ||
public int compareTo(Car car) { | ||
public int compareTo(final Car car) { | ||
return this.position - car.position; | ||
} | ||
|
||
private void checkValid(String names) { | ||
@Override | ||
public String toString() { | ||
return "name : " + name + ", position : " + position; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 2단계 요구사항에서 이 요구사항이 의미하는 바를 조금 더 깊게 고민해보면 좋을 것 같아요. 단순히 패키지 의존만을 의미하는 것인지, 아니면 도메인 패키지가 뷰의 로직을 알면 안된다인지 생각해보면 좋을 것 같아요. |
||
} | ||
|
||
private void checkValidName(final String names) { | ||
checkBlank(names); | ||
checkNameLength(names); | ||
} | ||
|
||
private void checkBlank(String name) { | ||
private void checkBlank(final String name) { | ||
String text = "자동차 이름은 %s일 수 없습니다."; | ||
if (name == null) { | ||
throw new NullPointerException(String.format(text, "null")); | ||
|
@@ -49,7 +54,7 @@ private void checkBlank(String name) { | |
} | ||
} | ||
|
||
private void checkNameLength(String name) { | ||
private void checkNameLength(final String name) { | ||
if (!(name.trim().length() <= CAR_LENGTH_LIMIT)) { | ||
throw new IllegalArgumentException("자동차의 이름은 " + CAR_LENGTH_LIMIT + "글자를 초과할 수 없습니다."); | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
정답은 없지만, 내부에서 선언하여 같은 값을 사용한다면 위의 RoundResult처럼 같이 선언과 동시에 초기화하거나 객체 생성자에서 초기화하는 일관된 형태를 띄면 좋을 것 같습니다.