Car 출발(1회) 구현완료 #1

wapago · 2023-04-29T15:15:47Z

No description provided.

dongKos

프로그램이 정상 동작 하지 않습니다 😢

dongKos · 2023-04-30T00:55:10Z

아직 구현중인건가..?

chs1234

구현하는데 고생하셨다는 말씀 먼저 드리며,
커밋 메시지, 주석 등 의미가 없는 코드들은 앞으로 작업하실 때 같이 올라가지 않도록
신경써주시면 더욱 줗아질 거 같습니다!

chs1234 · 2023-05-01T05:20:14Z

src/main/java/racing/model/User.java

+
+    public String[] inputCarNames() {
+        gameView.viewInputCarName();
+        String[] carNamesArray = gameView.inputCarName();


carNamesArray 변수를 inputCarNames() 메소드 내부에서 다시 사용하지 않을 거라면
바로 리턴해줘도 좋을 거 같아요!
return gameView.inputCarName()

chs1234 · 2023-05-01T05:20:36Z

src/main/java/racing/model/User.java


+        return playTime;


여기도 마찬가지!
return gameView.inputPlayTime();

chs1234 · 2023-05-01T05:21:20Z

src/main/java/racing/util/InputValidator.java

+
+        String[] carNamesArray = makeCarNamesArray(carNames);
+
+//        isContainSpecialChar(carNamesArray);


앞으로 커밋을 하실 때는 사용하지 않는 코드, 필요없는 주석은 지우고 올려주세요!

chs1234 · 2023-05-01T05:23:10Z

src/main/java/racing/util/InputValidator.java

+    }
+
+    public void validateInputPlayTimes(String inputPlayTime) {
+        if(!inputPlayTime.matches(NUMERIC)) {


정규식을 이용해서 패턴매칭 검사를 하셨네요!
좋습니다!

chs1234 · 2023-05-01T05:23:18Z

src/main/java/racing/util/InputValidator.java

+        return carNamesArray;
+    }
+
+//    public void isContainSpecialChar(String[] carNamesArray) {


지워주세요ㅠㅠ

chs1234

코멘트 추가합니다~!

chs1234 · 2023-05-01T05:33:51Z

src/main/java/racing/model/Result.java

-        this.isMove = isMove;
+    Car car;
+
+    public Result(Car car) {


작성하신 Result 객체는 사용하는 곳이 없는 거 같아요!

chs1234 · 2023-05-01T05:35:33Z

src/main/java/racing/util/InputValidator.java

+
+    public void isCarAtLeatTwo(String[] carNamesArray) {
+        if(!(carNamesArray.length >= 2)) {
+            throw new IllegalArgumentException("최소 2대의 자동차이름을 입력해야 합니다.");


메소드 내부에 매직넘버는 메소드 내부에서 직접 하드코딩 하지 않고
클래스 위쪽에 상수로 정의하시거나 따로 상수모음 클래스를 만들어서 관리하면 더 좋을 거 같습니다!

chs1234 · 2023-05-01T05:37:49Z

src/main/java/racing/model/Decider.java

+import java.util.*;
+
+public class Decider {
+    GetRandomNumber getRandomNumber = new GetRandomNumber();


클래스는 명사로 네이밍 해주시는 게 좋습니다.

chs1234 · 2023-05-01T05:40:31Z

src/main/java/racing/model/Decider.java

+
+        Integer max = Collections.max(positionList);
+
+        for(Map.Entry<String, Integer> entry : positionMap.entrySet()) {


key, value의 의미를 carName, position 변수명으로 바꿔주시면 좀 더 가독성이 좋아질 거 같습니다!
if (position == max) { winners.add(carName); }

chs1234

리팩토링 추천 제안 코멘트

chs1234 · 2023-05-01T06:08:22Z

src/main/java/racing/model/Decider.java

+
+        if(canMove) {
+            car.setMoveMarker(currentMoveMarker + "-");
+            car.setPosition(currentPosition + 1);


추후에 position 값을 가지고 marker로 변환하는 걸로 리팩토링 해보면 좋을 거 같아요!
그럼 Car 객체는 굳이 moveMarker라는 멤버를 갖고 있지 않아도 되서 로직이 좀 더 깔끔해질 거 같습니다!

chs1234 · 2023-05-01T06:14:15Z

src/main/java/racing/model/Decider.java

+        List<Integer> positionList = new ArrayList<>();
+        List<String> winners = new ArrayList<>();
+
+        for(int i=0; i<carList.size(); i++) {


향상된 for문으로 리팩토링 하시면 코드가 좀 더 깔끔해질 거 같아요!

chs1234 · 2023-05-01T06:15:13Z

src/main/java/racing/model/Decider.java

+            String key = entry.getKey();
+            Integer value = entry.getValue();
+
+            if(value == max) {


int 형 비교가 아닌 Wrapper 클래스 Integer는 null을 허용할 수 있기 때문에
좀 더 안전하게 equals()로 비교하시길 추천드립니다.
cf. 인텔리제이에서도 그렇게 추천함

dongKos

고생하셨습니다! 초장맛 어떠신가요? 😄

dongKos · 2023-05-01T06:02:26Z

src/main/java/racing/model/Car.java

@@ -3,20 +3,38 @@
 import racing.util.GetRandomNumber;

 public class Car {
-    private final String name;
+    private String carName;


변경되지 않는 property들은 final로 선언해주세요.

dongKos · 2023-05-01T06:03:33Z

src/main/java/racing/model/Cars.java

+
+public class Cars {
+
+    public List<Car> carList = new ArrayList<>();


객체 내의 필드들은 private 접근제어자로 캡슐화를 하는것이 기본적입니다. 특별한 이유가 없다면요

dongKos · 2023-05-01T06:04:01Z

src/main/java/racing/model/Cars.java

+    public List<Car> carList = new ArrayList<>();
+    int playTime;
+
+    Decider decider = new Decider();


Decider 라는 이름이 명확하지 않습니다. 무엇을 결정하는지? 네이밍에 포함시켜주세요

dongKos · 2023-05-01T06:04:32Z

src/main/java/racing/model/Cars.java

+            decider.decideMove(car);
+        }
+
+        System.out.println();


출력구문은 이 함수에 책임이 없습니다. 뷰 단으로 분리해보면 어떨까요?

dongKos · 2023-05-01T06:05:47Z

src/main/java/racing/model/Cars.java

+    }
+
+    public void move() {
+        for(Car car : carList) {


향상된 for문도 좋지만 java 8의 stream을 이용해 봅시다. forEach는 stream 안써도 걍 쓸 수 있어요. 함수형 프로그래밍에 익숙해지면 코드가 간결해지고 가독성이 올라갑니다

Suggested change

for(Car car : carList) {

carList.forEach(car -> decider.decideMove(car));

dongKos · 2023-05-01T06:14:53Z

src/main/java/racing/util/InputValidator.java

+//        }
+//    }
+
+    public void isCarAtLeatTwo(String[] carNamesArray) {


is~로 시작하는 함수는 보통 리턴형을 boolean으로 하는것이 관행입니다. 여기서는 validation check를 하기 때문에 명명을 조금 다르게 하는게 좋을것 같아요

Suggested change

public void isCarAtLeatTwo(String[] carNamesArray) {

public void validateIfCarsAtLeastTwo(String[] carNamesArray) {

Suggested change

public void isCarAtLeatTwo(String[] carNamesArray) {

public void throwIfCarsLessThanTwo(String[] carNamesArray) {

dongKos · 2023-05-01T06:15:06Z

src/main/java/racing/util/InputValidator.java

+        }
+    }
+
+    private void isDuplicateCarNames(String[] carNamesArray) {


위와 동 입니다.

dongKos · 2023-05-01T06:15:34Z

src/main/java/racing/view/GameView.java

+            }
+        }
+
+        String resultStr = stringBuilder.toString();


의미 없는 변수선언은 지양하고 바로 리턴!

dongKos · 2023-05-01T06:15:48Z

src/main/java/racing/view/GameView.java

+        for(int i=0; i<winners.size(); i++) {
+            stringBuilder.append(winners.get(i));
+
+            if(winners.size() != 1 && i != winners.size() - 1) {


2단으로 들어가기 금지 입니다 :) 수정해주세요

dongKos · 2023-05-01T06:16:08Z

src/main/java/racing/view/GameView.java

+import java.util.Scanner;
+
+public class GameView {
+    Scanner scanner = new Scanner(System.in);


외부에서 사용하지 않아야 하기 때문에 private final을 붙여주세요

Car출발 구현완료

5ae0cad

wapago requested review from chs1234 and dongKos April 29, 2023 15:16

입력횟수만큼 car 출발 구현완료

8a06649

dongKos requested changes Apr 30, 2023

View reviewed changes

wapago added 2 commits May 1, 2023 10:57

게임결과 출력구현완료

ecd8eea

inputCarNames,inputPlayTime 유효성 검증로직 구현완료

060f82d

chs1234 reviewed May 1, 2023

View reviewed changes

dongKos requested changes May 1, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Car 출발(1회) 구현완료 #1

Car 출발(1회) 구현완료 #1

wapago commented Apr 29, 2023

dongKos left a comment •

edited

Loading

dongKos commented Apr 30, 2023

chs1234 left a comment

chs1234 May 1, 2023

chs1234 May 1, 2023

chs1234 May 1, 2023

chs1234 May 1, 2023

chs1234 May 1, 2023

chs1234 left a comment

chs1234 May 1, 2023

chs1234 May 1, 2023

chs1234 May 1, 2023

chs1234 May 1, 2023

chs1234 left a comment

chs1234 May 1, 2023

chs1234 May 1, 2023

chs1234 May 1, 2023

dongKos left a comment

dongKos May 1, 2023

dongKos May 1, 2023

dongKos May 1, 2023

dongKos May 1, 2023

dongKos May 1, 2023

dongKos May 1, 2023

dongKos May 1, 2023

dongKos May 1, 2023

dongKos May 1, 2023

dongKos May 1, 2023


		String[] carNamesArray = makeCarNamesArray(carNames);

		// isContainSpecialChar(carNamesArray);


		Integer max = Collections.max(positionList);

		for(Map.Entry<String, Integer> entry : positionMap.entrySet()) {


		public class Cars {

		public List<Car> carList = new ArrayList<>();

	for(Car car : carList) {
	carList.forEach(car -> decider.decideMove(car));

	public void isCarAtLeatTwo(String[] carNamesArray) {
	public void validateIfCarsAtLeastTwo(String[] carNamesArray) {

	public void isCarAtLeatTwo(String[] carNamesArray) {
	public void throwIfCarsLessThanTwo(String[] carNamesArray) {

Car 출발(1회) 구현완료 #1

Are you sure you want to change the base?

Car 출발(1회) 구현완료 #1

Conversation

wapago commented Apr 29, 2023

dongKos left a comment • edited Loading

Choose a reason for hiding this comment

dongKos commented Apr 30, 2023

chs1234 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chs1234 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chs1234 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongKos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongKos left a comment •

edited

Loading