-
Notifications
You must be signed in to change notification settings - Fork 8
김정교 3번째 PR입니다. #12
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
base: 김정교
Are you sure you want to change the base?
김정교 3번째 PR입니다. #12
The head ref may contain hidden characters: "\uAE40\uC815\uAD50"
Conversation
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.
전반적으로 깔끔하게 잘 작성해주신 것 같아요 👍
간단한 피드백 몇 개만 코멘트로 남겨두었습니다.
java collection과 관련된 이야기는 아마도 오늘 다루는 주제들 중 하나일 것 같은데,
한번 고민해보시고 반영해주시면 좋을 것 같아요 :)
| public class Car { | ||
|
|
||
| private static final double GO_FLOOR = 0.4; | ||
| private static final double GO_THRESHOLD = 0.4; |
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.
👍
| winnerList.add("a"); | ||
| winnerList.add("b"); | ||
| winnerList.add("c"); | ||
| winnerList.equals(racingCarGamePlay.getWinner(racingCarGamePlay.makeCars(targetCarNames))); |
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.
동작을 검증하는 부분이 필요할 것 같아요!
src/test/java/RacingCarGameTest.java
Outdated
| String[] targetCarNames = {"a", "b", "c"}; | ||
| String testString = "a,b,c"; | ||
| String[] splitedCarNames = RacingCarGame.makeCarNames(testString); | ||
| assertEquals(true, Arrays.equals(splitedCarNames,targetCarNames)); |
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.
makeCarNames에서 배열 대신 collection을 리턴하면 이 부분도 조금 더 간결해질 수 있을거같아요.
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.
변경했습니다... 감사합니다..
| @Test | ||
| void testGetWinner() { | ||
| String[] targetCarNames = {"a", "b", "c"}; | ||
| RacingCarGamePlay racingCarGamePlay = new RacingCarGamePlay(targetCarNames, 0); |
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.
테스트에서도 하드코딩된 숫자를 추출하면 더 쉽게 읽을 수 있을것 같아요 :)
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.
테스트를 지워버렸어요!
|
re-request review 버튼이 안보여용 ㅠ |
TDD로 다시 짜는 것을 하지 못하고 단위테스트 몇 개를 붙이는데 멈추고 말았습니다...