-
Notifications
You must be signed in to change notification settings - Fork 84
[LBP] 김태우 자동차 미션 1-3단계 제출합니다. #92
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: twootwoo
Are you sure you want to change the base?
Changes from all commits
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 |
---|---|---|
@@ -0,0 +1,46 @@ | ||
public class Car { | ||
int position = 0; | ||
public int carNumber; | ||
public String carName; | ||
|
||
public Car() { | ||
this.position = 0; | ||
} | ||
|
||
public Car(int carNumber, String carName) { | ||
this.carNumber = carNumber; | ||
this.carName = carName; | ||
} | ||
Comment on lines
+6
to
+13
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. 이미 Car 클래스의 position 필드는 0으로 초기화 되어 있는 것 같아요. 기본 생성자에서는 0으로 다시 할당하고, 아래의 생성자에서는 할당하지 않은 특별한 이유가 있나요? 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. 상단의 접근 제어자를 지정하지 않은 것과 비슷하게, 코드 작성 시 코드의 실행 가능 여부에 집중했습니다. 그러다 보니 우선 코드가 작동만 되도록 작성한 뒤 미처 확인하지 못하고 커밋을 진행했습니다. |
||
|
||
public int getRandom() { | ||
double randomValue = Math.random(); | ||
return (int) (randomValue * 9); | ||
} | ||
Comment on lines
+15
to
+18
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.
|
||
|
||
public boolean getStatus() { | ||
int randomValue = getRandom(); | ||
boolean result = true; | ||
|
||
if (randomValue <= 3) { | ||
result = false; | ||
} | ||
|
||
return result; | ||
} | ||
Comment on lines
+20
to
+29
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.
// 형태 예시
if (value > 4) {
return true;
}
return false; 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.
|
||
|
||
public void tryMove() { | ||
if(getStatus()) { | ||
this.position++; | ||
} | ||
} | ||
Comment on lines
+31
to
+35
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.
// 책임 분리 예시
// 자동차 외부 어딘가에서
if (isMoveable()) { // 움직일 수 있나? (랜덤 또는 새로운 움직임 규칙이 생겼을 때 대체 가능)
car.moveForword(); // 차량 앞으로 전진, (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.
|
||
|
||
public int getPosition() { | ||
return position; | ||
} | ||
|
||
|
||
public String getCarName() { | ||
return carName; | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,77 @@ | ||
import java.util.*; | ||
import java.util.stream.Collectors; | ||
|
||
public class CarRace { | ||
private final List<Car> cars = new ArrayList<>(); | ||
List<String> carNames; | ||
int raceAttemptCount; | ||
List<String> winnerCarNames; | ||
Comment on lines
+4
to
+8
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. default 접근 제한자로 지정됨으로써, 제가 의도하지 않은 곳에서 변수의 값이 수정될 수 있는 문제가 발생할 수 있다고 생각합니다. 이는 프로그램의 안정성에 큰 악영향을 미친다고 생각합니다. 변수 선언시 접근 제한자를 더 꼼꼼히 확인하도록 하겠습니다!! |
||
|
||
public void receiveCarNamesInput() { | ||
Scanner scanner = new Scanner(System.in); | ||
|
||
System.out.println("경주할 자동차 이름을 입력하세요(이름은 쉼표(,)를 기준으로 구분)."); | ||
String value = scanner.nextLine(); | ||
if(value.isEmpty()) { | ||
throw new IllegalArgumentException("경주할 자동차의 이름을 반드시 입력해주세요."); | ||
} | ||
carNames = Arrays.asList(value.split(",")); | ||
|
||
if(carNames.stream().anyMatch(cn -> cn.length()>5)) { | ||
throw new IllegalArgumentException("자동차 이름은 5글자 이하로 작성해주세요."); | ||
} | ||
|
||
} | ||
Comment on lines
+10
to
+24
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.
|
||
|
||
public void recieveCarRaceAttemptCountInput() { | ||
Scanner scanner = new Scanner(System.in); | ||
|
||
System.out.println("시도할 회수는 몇회인가요?"); | ||
raceAttemptCount = scanner.nextInt(); | ||
} | ||
|
||
public void makeCarObject(int sizeOfList) { | ||
for(int i = 0; i < sizeOfList; i++) { | ||
cars.add(new Car(i, carNames.get(i))); | ||
} | ||
} | ||
Comment on lines
+33
to
+37
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. 자동차 객체를 만드는 것과 자동차 목록에 추가하는 역할을 분리할까 고민해 보았습니다. 하지만 메소드명을 makeCarList 로 바꾸는 것이 차라리 나을 것 같아 수정했습니다! 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.
|
||
|
||
public void moveCars() { | ||
for (Car car : cars) { | ||
car.tryMove(); | ||
} | ||
} | ||
|
||
public void doRace(int count) { | ||
System.out.println("실행 결과"); | ||
for (int i = 0; i < count; i++) { | ||
moveCars(); | ||
printRaceResult(); | ||
} | ||
} | ||
|
||
public void printRaceResult() { | ||
for (Car car : cars) { | ||
System.out.println(car.getCarName() + " : " + "-".repeat(car.getPosition())); | ||
} | ||
System.out.println(); | ||
} | ||
Comment on lines
+39
to
+58
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. 메서드 선언 순서도 태우만의 규칙이 있으면 더 읽기 좋을 것 같아요. moveCars()는 어디서 쓰지? 하다가 바로 아래인 moveCars()에서 사용하는 것을 알게 되었어요. 반대로 printRaceResult는 위와 반대로 아래에 존재하네요. 다른 사람이 코드를 읽기 쉽게하려면 이렇게 사소한 것까지 규칙을 지켜서 기대하는 대로 보여지는 것이 중요할 것 같아요. 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. 남겨주신 코맨트를 읽고 메소드 선언 순서가 일정하지 않아 코드의 가독성에 악영향을 미쳤다는 것을 알게 되었습니다. 이런 사소하지만 중요한 부분도 꼼꼼히 지적해주셔서 정말 감사드립니다! 앞으로 가독성 향상을 위해 디테일만 부분에서 더 신경써 보도록 하겠습니다! |
||
|
||
public void setWinnerCarNames() { | ||
int maxPosition = cars.stream() | ||
.max(Comparator.comparingInt(Car::getPosition)) | ||
.orElseThrow(NoSuchElementException::new) | ||
.getPosition(); | ||
|
||
winnerCarNames = cars.stream() | ||
.filter(c -> c.getPosition() == maxPosition) | ||
.map(Car::getCarName) | ||
.collect(Collectors.toList()); | ||
} | ||
Comment on lines
+60
to
+70
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. 이건 바로 수정하지 않고 스스로 생각해 보면 좋을 것 같아요. (Stream을 쓰는 것에 초점을 맞춘 것이 아닙니다!) 현재 우승한 자동차의 이름(들)을 위해 filter 부분에서 자동차의 포지션을 getter로 얻은 후 비교하고 있어요. getter로 빼서 비교하거나 수정, 추가 등의 작업을 한다면, 해당 객체는 무슨 책임을 가지고 있는 것일까요? 단순히 데이터(상태)를 저장하는 것만이 중요할까요? |
||
|
||
public void printWinnerCarNames() { | ||
System.out.print(String.join(", ", winnerCarNames)); | ||
System.out.println("가 최종 우승했습니다."); | ||
} | ||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
public class Main { | ||
public static void main(String[] args) { | ||
CarRace cr = new CarRace(); | ||
|
||
cr.receiveCarNamesInput(); | ||
cr.recieveCarRaceAttemptCountInput(); | ||
cr.makeCarObject(cr.carNames.size()); | ||
cr.doRace(cr.raceAttemptCount); | ||
cr.setWinnerCarNames(); | ||
cr.printWinnerCarNames(); | ||
} | ||
Comment on lines
+2
to
+11
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. cr로 축약하신 특별한 이유가 있나요? 오히려 저는 보기 어려웠던 것 같아요. 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. CarRace 즉, 자동차 경주 객체가 사용자에게 입력을 받는 것을 책임져야 하나요? 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.
|
||
|
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Nested; | ||
import org.junit.jupiter.api.Test; | ||
import java.util.ArrayList; | ||
import java.util.Arrays; | ||
import static org.assertj.core.api.AssertionsForClassTypes.assertThatCode; | ||
|
||
public class CarRaceTest { | ||
|
||
private final CarRace cr = new CarRace(); | ||
private final Car car = new Car(); | ||
|
||
@Nested | ||
class testRace { | ||
|
||
@Test | ||
@DisplayName("랜덤 값에 따라 true, false 반환하고 doesNotThrowAnyException 예외 발생 여부를 확인한다.") | ||
void shouldReturnStatusOfCarRaceTest() { | ||
System.out.println("3이하 false, 4이상 true : " + car.getStatus()); | ||
assertThatCode(car::getStatus).doesNotThrowAnyException(); | ||
} | ||
Comment on lines
+16
to
+21
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. 상단의 코맨트를 읽어 내려온 뒤 제가 짠 테스트 코드를 보니 정말 처참하기 그지없네요.. |
||
|
||
@Test | ||
@DisplayName("자동차 경주 결과를 반환하고 doesNotThrowAnyException 예외 발생 여부를 확인한다.") | ||
void shouldReturnResultOfCarRaceTest() { | ||
cr.carNames = new ArrayList<>(Arrays.asList("neo", "brie", "brown")); | ||
cr.raceAttemptCount = 5; | ||
cr.makeCarObject(cr.carNames.size()); | ||
cr.doRace(cr.raceAttemptCount); | ||
assertThatCode(cr::setWinnerCarNames).doesNotThrowAnyException(); | ||
} | ||
} | ||
Comment on lines
+23
to
+32
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. 코맨트를 읽고 다시 생각해 보니, 규모가 큰 프로그램을 짜보지 못해서 생긴 문제라는 생각이 들었습니다. DisplayName에 적는 설명은 어느정도의 추상화가 필요한 것 같네요. 앞으로 DisplayName 작성 시 참고하도록 하겠습니다. 감사합니다! |
||
|
||
} |
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.
position 필드의 접근 제어자를 지정하지 않은 이유가 있나요?
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.
처음 선언할 때 코드의 안정성보다는 로직의 구현에 집중하다 보니 그냥
int position = 0;
으로 선언한 뒤 깜빡하고 말았습니다. 부끄러운 실수이지만, 이번 기회에 제 안좋은 습관을 확실히 알게 되었습니다. 앞으로는 필드를 선언할 때 접근 지정자를 꼼꼼히 확인하도록 하겠습니다! 👀