-
Notifications
You must be signed in to change notification settings - Fork 92
[2단계 - 자동차경주] 이지현 미션 제출합니다. #121
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: izzy80
Are you sure you want to change the base?
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.
이번 미션을 칼같이 제출해주셨네요 👏👏👏 고생하셨습니다!
빠르게 제출해주신만큼 물음표를 많이 던져봤습니다 ㅎㅎ
아래는 질문주신 내용에 대한 저의 의견입니다!
test할 때도 이 요구사항(indent, depth) 을 지키는 지 궁금합니다.
네! 테스트코드에서도 요구사항을 한번 지켜보시죠! (오히려 테스트코드에서 depth가 생기는 경우가 얼마나 있나? 생각이 들긴 하네요 🤔
private void moveAllCars(int testRounds){
for (Car car : cars) {
for (int i = 0; i < testRounds; i++) {
car.move(RandomUtil.randomGenerator());
}
}
}
private void saveWinnerName(){
for (Car car : cars) {
if (car.getPosition() == winnerPosition) {
winnerNames.add(car.getName());
}
}
}
이 코드는 요구사항을 만족하지 못하고 있으니 1depth로 줄여보시죠!
(hint: 일급 컬렉션으로 활용하면 어떤 변화가 있을지도??)
객체 초기화하는 부분에서 궁금해졌습니다. 필드와 생성자를 통해 구현하는게 있는데, 외부에서 주입을 받지 않는 경우 어디에서 초기화하는 것을 추천하나요?
필드, 생성자 어디서 초기화해도 상관은 없어보입니다 :)
다만 생성사 오버로딩을 고려해본다면 생성자에서 초기화하는것에 한표를 던져볼 수 있을 것 같아요!
물론 초기값이 절대 변하지 않는 값이라면 필드에서 초기화해도 좋아보이구요
이번 PR을 먼저 올리고 나서 확인해보니, this 키워드가 누락된 부분이 있었습니다.
구글 Java 스타일 가이드에서는 필드와 지역 변수의 이름이 겹칠 때만 this 사용을 권장하고, 그 외에는 생략해도 된다고 되어 있더라고요. 지금처럼 이름 충돌이 없는 경우 생략은 허용 된다고는 하지만, 역시 일관성 측면에서 this를 모두 붙이는 게 나을까요?
저는 구글 스타일가이드에 나와있듯이 이름 충돌이 없는 경우 생략하는 편이에요
굳이 안붙여도 될 상황에서까지 this를 붙이고싶지는 않다는 개인적인 의견입니다 ㅎㅎ
import java.util.ArrayList; | ||
|
||
public class CarWinner { | ||
private Car[] cars; //참가한 자동차들 |
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 사용이 권장되곤 하는데요? collection은 어떤 장점이 있을까요?
키워드 : jcf
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.
처음에 JCF가 어떤 약자인지 몰라서 헷갈렸는데, Java Collections Framework였군요!
List는 크기가 가변적인 데이터에 좋고, 인터페이스 기반의 설계가 되어있어 코드를 유연하게 작성할 수 있습니다!
배열보다는 List를 사용해야겠네요!
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.
추가적으로 list에서 제공해주는 기능이 편리하다는 부분도 하나의 큰 장점으로 다가오기도 할 것 같아요 ㅎㅎ
+) List로 반영이 되지 않은 것 같은데 확인 부탁드려요~!
private Car[] cars; //참가한 자동차들 | ||
private int winnerPosition; //우승 자동차의 위치(최대값) | ||
private int winnerCnt; //우승 자동차의 수 | ||
private ArrayList<String> winnerNames; //우승 자동차의 이름 |
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.
List로 타입을 선언하는것과 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.
//1
private ArrayList<String> arrayList = new ArrayList<>();
//2
private List<String> list = new ArrayList<>();
List는 인터페이스이고 ArrayList는 그 구현 클래스입니다.
아무래도 유연성과 유지보수 측면에서는 2번 방식이 더 권장된다고 하네요.
코딩테스트 연습할 때 ArrayList로 바로 선언하는 게 손에 익어서 자연스럽게 쓰게 되었네요 😅😅
단점으로는 List로 선언했을 때 ArrayList의 고유 메서드(ensureCapacity() 등)를 직접 사용할 수 없다는 점이 있지만,
필요할 경우 캐스팅으로 해결할 수 있으니, 인터페이스 타입을 사용하는 것이 더 바람직한 선택 같네요!
public class CarWinner { | ||
private Car[] cars; //참가한 자동차들 | ||
private int winnerPosition; //우승 자동차의 위치(최대값) | ||
private int winnerCnt; //우승 자동차의 수 |
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.
cnt 와 같이 축약된 변수명은 지양해보는게 어떨까요~?
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.
+) 이 부분 반영이 안된거같아요~!
public void whichWinner(int testRounds){ | ||
//1. 모든 자동차를 주어진 횟수만큼 이동 | ||
moveAllCars(testRounds); | ||
//2. 우승자의 수와 자동차의 위치(최대값) 계산 | ||
countWinners(); | ||
//3. 우승자 이름 저장 | ||
saveWinnerName(); | ||
} |
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.
whichWinner 라는 메소드명은 어떤 의미를 나타내나요?
현재 메소드는 어떤 역할을 수행하고있나요?
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.
whichWinner는 어떤 자동차가 우승하는지를 구하는 메소드명인데 너무 함축해서 메소드명을 쓴 것 같네요.
해당 메소드가 자동차들 이동 -> 우승자 결정 -> 우승자 정보 저장의 과정이 있어서 좀 더 포괄하는 메소드를 만들어야겠네요
메소드명이 항상 고민입니다ㅠ
src/test/java/CarTest.java
Outdated
@Test | ||
@DisplayName("주어진 횟수 동안 n대의 자동차 중에 우승자가 1명 이상 나온다") | ||
void whichCarIsWin() { | ||
Car carA = new Car("A",moveCondition); | ||
Car carB = new Car("B",moveCondition); | ||
Car carC = new Car("C",moveCondition); | ||
|
||
Car[] cars = {carA, carB, carC}; | ||
|
||
int testRounds = 1; | ||
CarWinner winner = new CarWinner(cars); | ||
winner.whichWinner(testRounds); | ||
|
||
//우승자는 1명 이상 나온다 | ||
assertThat(winner.getWinnerCnt()).isGreaterThanOrEqualTo(1); | ||
//우승자의 수와 이름의 수가 같아야 한다 | ||
assertThat(winner.getWinnerCnt()).isEqualTo(winner.getWinnerName().size()); | ||
|
||
} |
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.
우승자가 1명, 2명, 3명인 각각의 경우를 테스트하고싶다면 어떻게 테스트할 수 있을까요?
테스트하기 쉬운 구조로 개선해볼까요?
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.
@ParameterizedTest
@ValueSource(ints = {1, 2, 3})
사용하면 될 것 같습니다!
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.
현재 테스트코드상으로는 무조건 1명 이상이 될 것 같은데요?
정확히 1명, 2명, 3명이 각각 나오는 경우를 테스트하고싶다면 어떻게 개선할 수 있을까요?
// 이렇게 우승자가 1, 2, 3명이 되도록 제어하고싶으면 어떻게 테스트를 작성하면 좋을까요?
assertThat(winner.getWinnerCnt()).isEqualTo(1 or 2 or 3);
public class CarWinner { | ||
private Car[] cars; //참가한 자동차들 |
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.
컬렉션을 Collection을 감싸면서 그 외 다른 멤버 변수가 없는 상태를 일급 컬렉션이라고 합니다!
Cars를 일급컬렉션으로 한번 만들어보세요! 이 경우 어떤 식으로 테스트가 작성될 수 있는지도 같이 말씀해주세요 :)
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.
Cars
- 자동차 이동 (moveAll)
- 우승자 자동차 목록 (getWinners)
- 우승자 이름 목록 (getWinnerNames)
- 우승자 수 (getWinnerCount)
로 일급컬렉션을 구현했습니다.
테스트에서는 Cars를 주입한 뒤, moveAll()과 getWinnerCount() / getWinnerNames()를 호출하는 방식으로
각각의 테스트 조건(n명의 우승자가 나오는지 여부)을 파라미터 기반으로 검증할 수 있게 구성했습니다.
private Car[] cars; //참가한 자동차들 | ||
private int winnerPosition; //우승 자동차의 위치(최대값) | ||
private int winnerCnt; //우승 자동차의 수 | ||
private ArrayList<String> winnerNames; //우승 자동차의 이름 |
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.
우승 자동차의 위치를 알고싶다면 어떻게 할 수 있을까요?
winner는 꼭 객체의 상태로 존재해야하는 값일까요?
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://f-lab.kr/insight/understanding-and-applying-first-class-collections
이 글을 보고 궁금한 점이 생겼습니다.
winner는 꼭 객체의 상태로 존재해야하는 값일까요?
이 질문에 지금 요구사항에서는 결과를 저장하지 않아서 Winner 객체가 굳이 필요없는 걸까요?
그렇다면 추후 라운드별 결과를 저장하라는 요구사항이 생긴다면 Winner 객체가 생겨도 될 것 같다고 봤습니다.
그런데 여기서 약간 고민이 되는 건,
그 winner 정보를 Cars 안에 두는 게 맞는지,
아니면 따로 Winner 또는 GameResult 같은 객체를 만들어 분리해야 하는지입니다.
약간 두 객체의 정보가 비슷한 것 같다고 생각이 되어서...
그리고 뭔가 수정한 1급 객체인 cars가 책임이 아직도 섞여있다는 생각이 들고
Cars가 경기 전의 자동차들을 담는 '초기 상태' 개체인데,
이 객체에서 경기를 실행하고, 결과(우승자)까지 직접 계산해서 내보내는 구조가
입력/실행/결과 저장의 과정이 cars에 있는게 맞나? 라는 생각이 들고 그러면 또 winner를 만들어야 할 것 같은데....그럼 질문이 도돌이표로 돌아오고....
일급컬렉션에 대한 감이 확실히 안 오다 보니 머가 맞는 지 모르겠네요
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.
정말 공감되는 고민거리네요 ㅎㅎㅎㅎ
저도 이렇게 누구의 책임이지..? 하는 고민이 자주 드는 것 같아요
이럴때는 현실세계를 기준으로 시뮬레이션을 한번 돌려보곤 합니다
이번 자동차 미션의 경우 F1 경기장에서 자동차 경주를 보고있다는 생각을 해보게 되네요 (저는 F1에 대해서 하나도 모르지만요!)
저는 이 경기를 보는 관중의 입장에서 자동차 경주가 어떻게 진행되고, 우승자는 누가 결정해주지? 와 같은 고민을 하게 될 것 같아요!
그 사이에서 객체의 행위를 중심으로 책임을 가르게 되는것 같아요 😄
지현님은 어떻게 생각하시나요?
+) 살짝 부끄럽지만 약 3년전에 제가 자판기라는 미션을 했을 때 생각했던 사고의 흐름을 참고차 남겨봅니다
juno의 자판기 회고 글
고민했던 부분
세 명의 자동차가 모두 움직이지 않아 position == 0인 상황을 예외 케이스로 가정해보았습니다.
하지만 요구사항에 “우승자는 한 명 이상 존재해야 한다”고 명시되어 있어, 이 경우 모두가 우승자가 되는 것으로 판단하고 로직을 구현했습니다. 이 해석이 맞을까요?
궁금한 점
test할 때도 이 요구사항을 지키는 지 궁금합니다.
이 부분도 그럼 요구사항을 충족시키지 못 한 건가요? 그런데 이 부분을 메소드로 따로 빼낼 필요가 있는지에 대한 의문이 들어서 헷갈리네요😱
구글 Java 스타일 가이드에서는 필드와 지역 변수의 이름이 겹칠 때만 this 사용을 권장하고, 그 외에는 생략해도 된다고 되어 있더라고요. 지금처럼 이름 충돌이 없는 경우 생략은 허용 된다고는 하지만, 역시 일관성 측면에서 this를 모두 붙이는 게 나을까요?