-
Notifications
You must be signed in to change notification settings - Fork 8
Refactoring, test 구현 #14
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?
The head ref may contain hidden characters: "\uC774\uC131\uD6C8"
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.
코드 간결하게 잘 작성해주신 것 같아요 👍
객체지향적으로 클래스를 구성하려고 하신 흔적이 많이 보여요 ㅎㅎ
고민해보시면 좋을 것 같은 부분들에 코멘트 남겨두었습니다, 확인 부탁드려요 :)
그리고 자바 컨벤션중에 익숙하지 않으신 것들도 조금 있으실 것 같은데요,
다소 귀찮으시겠지만, 아래 링크 한 번 쭉 읽어보시고 반영해보시면 코드가 조금 더 깔끔해질 수 있을거같아요 :)
https://myeonguni.tistory.com/1596
| this.distance = distance; | ||
| } | ||
|
|
||
| public RacingCar(String name) { |
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.
메서드 순서도 convention! 사소한 것이긴 한데, constructor는 보통 getter 보다 위에 위치하는 경우가 많아요.
src/main/java/RacingGame.java
Outdated
| } | ||
|
|
||
| public static void whoWannaJoin(){ | ||
| private void initRacingCar(){ |
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.
메서드 순서가 위에서 아래로 읽히도록 되어있으면 더 읽기에 편할 것 같아요 :)
| } | ||
|
|
||
| private void showWinner() { | ||
| int winnerDist = 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.
길이가 길어지더라도 축약하지 않고 의미를 드러낼 수 있는 단어로 이름지어주면 어떨까요?
|
|
||
| private void showWinner() { | ||
| int winnerDist = 0; | ||
| List<String> winnerGroup = new ArrayList(); |
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/main/java/RacingGame.java
Outdated
| for(RacingCar racingCar : racingCars.getRacingCars()){ | ||
| if(runDice()) | ||
| { | ||
| racingCar.setDistance(racingCar.getDistance() + 1); |
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.
setter를 사용하지 않고 구현해보면 어떨까요?
객체가 스스로 동작하도록요!
| winnerGroup.add(entry.getKey()); | ||
| private boolean runDice(){ | ||
| Random rand = new Random(); | ||
| if(rand.nextInt(10) >= 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.
하드코딩된 숫자를 상수 변수로 선언하면 어떨까요?
|
|
||
| import static org.junit.jupiter.api.Assertions.*; | ||
|
|
||
| public class TestRacingCars { |
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를 붙이는 경우가 많습니다.
RacginCarsTest처럼요!
단축키를 활용하면 자동으로 네이밍되어 생성될거에요 :)
| showWinner(); | ||
| } | ||
|
|
||
| private void showWinner() { |
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.
지금은 RacingCars가 단순히 자바 collection을 네이밍만 바꿔서 한 번 래핑하여 collection의 일부 기능만 사용할 수 있도록 열어두었는데요, 일급 콜렉션은 약간 다른 개념일 것 같아요. (https://jojoldu.tistory.com/412 해당 링크 참고)
winner를 구하는 로직이 racingCars 안으로 들어가면 조금 더 일급콜렉션의 역할에 맞는 객체가 되지 않을까요?
| public class TestRacingGame { | ||
|
|
||
| @Test | ||
| public void 생성자테스트(){ |
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.
생성자 테스트에서는 어떤 것들을 테스트할 수 있을까요?
지금은 생성자를 테스트하기 어려운 상태인데, 테스트 가능하게 하려면 어떻게 해야 할까요?
위와 같은 지점들이 테스트하기 어려운 부분 중 하나일 것 같아요 :)
지금은 짧게만 한 번 생각해보시고, 주말에 같이 고민해 봐요 ㅎㅎ
테스트 구현하는게 꽤 어렵군요...ㅜ