Skip to content

김진호 - 자동차 경주 게임#5

Closed
lomayd wants to merge 7 commits intoDcom-KHU:lomaydfrom
lomayd:lomayd
Closed

김진호 - 자동차 경주 게임#5
lomayd wants to merge 7 commits intoDcom-KHU:lomaydfrom
lomayd:lomayd

Conversation

@lomayd
Copy link
Copy Markdown

@lomayd lomayd commented Mar 17, 2023

2023-spring-codeReview-study 1주차 과제 제출

getCarList - String input을 List carList로 바꾸는 함수
fiveValidate - 각각의 Car Name이 다섯 글자가 넘어가는 지 검사하는 함수
getGameCount - String input을 Integer count로 바꾸는 함수, 이때 input이 숫자인지 검사
playGame - count만큼의 게임을 진행하는 함수
playOneGame - 한 게임을 진행하는 함수
getWinner - 승자를 결정하는 함수
printWinner - 승자를 출력하는 함수

Copy link
Copy Markdown
Collaborator

@rohsik2 rohsik2 left a comment

Choose a reason for hiding this comment

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

하나의 클래스에서 모든 기능을 다 만드셨는데, 클래스의 역할을 다른 클래스에 나누어 주시는건 어떠신가요?


public static Integer getGameCount(String input) throws IllegalArgumentException {
Integer intInput;
try {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

대부분의 게임 로직을 Car 안에서 Static method로 구현하셨는데, 다른 class를 하나 만들어서 위임을 해주시는건 어떠신가요?

Comment on lines +39 to +42
public static void fiveValidate(String s) {
if (s.length() > 5)
throw new IllegalArgumentException("[Error] Car Name is over 5 letters");
}
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

뭔가 더 좋은 method name... 이 있을것같아요 fiveValidate 강렬해서 좋습니다만.. 음... 네...

return intInput;
}

public static void playGame(List<Car> carList, Integer count) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

parameter로 wrapper class 사용하셨는데 혹시 이유가 있으신가요?

@lomayd lomayd changed the title Lomayd 김진호 - 자동차 경주 게임 구현 Mar 18, 2023
@lomayd lomayd changed the title 김진호 - 자동차 경주 게임 구현 김진호 - 자동차 경주 게임 Mar 18, 2023
Copy link
Copy Markdown

@yuseunggeun yuseunggeun left a comment

Choose a reason for hiding this comment

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

LGTM!

Comment on lines +31 to +36
for (String s : inputList) {
fiveValidate(s);
Car car = new Car(s);
carList.add(car);
}
return carList;
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

오류 발생 시 새로운 입력을 다시 받는 기능이 추가되면 좋을 것 같습니다!

Comment on lines +64 to +66
if (Randoms.pickNumberInRange(0, 9) > 4) {
c.position = c.position + 1;
}
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

position을 수정하는 부분을 별도의 메소드로 구현하는 것은 어떨까요?


public static List<Car> getCarList(String input) {
List<Car> carList = new ArrayList<>();
List<String> inputList = new ArrayList(Arrays.asList(input.split(",")));
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

간결하게 한 줄로 표현하신게 좋아 보입니다!

Copy link
Copy Markdown

@choi5798 choi5798 left a comment

Choose a reason for hiding this comment

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

고생하셨습니다~

if (Randoms.pickNumberInRange(0, 9) > 4) {
c.position = c.position + 1;
}
for (int j = 0; j < c.position; j++) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

자동차가 전진하는 부분과 결과를 출력하는 부분을 나누는 것도 좋아보여요

}
}

return print.substring(0,print.length()-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.

뒤에 ", "을 없애기 위해 substring 을 사용한 것도 좋지만 ", " 을 구분자로 join 메소드를 사용해 보는 것은 어떨까요?

return position;
}

public static List<Car> getCarList(String input) {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

자동차 이름이 5자 이하인 경우 이외에 다른 예외도 처리해주면 좋을 것 같아요

@lomayd lomayd closed this by deleting the head repository Jun 19, 2023
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.

4 participants