-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[자동차 경주] 한별 미션 제출합니다. #2380
base: main
Are you sure you want to change the base?
[자동차 경주] 한별 미션 제출합니다. #2380
Conversation
리드미 파일에 필요한 메서드, 클래스, 데이터 등을 명시했습니다.
view/GuideMessage : guide message 추가 controller/UserInputController : inputCarNames() 메서드 추가 controller/Game : gameStart() 메서드 추가 racingcar/Application : Game class생성, gameStart()실행 gameStart()에서 racingCarNames는 test용으로 삭제할 예정
model/number : 자동차 이름 글자수 maxLength추가 model/racingGameModel : listUpRacingCar() 이름 글자수 조건에 따라 자동자 후보를 ArrayList에 저장하는 메서드 추가 model/utilityModel : splitByComma(), isValidName() 추가 controller/Game : 입력받은 자동차 이름을 컴마로 구분해 racingCarNames에 저장
model/number model/utilityModel view/guideMessage
controller/userInputController : inputCarNames()삭제-> userInput() 함수로 모든 사용자 입력에 재활용 model/utilityModel : stringToInt 추가 model/racingGameModel : racingCount 추가 controller/game : 사용자 입력을 받아 시도횟수를 racingCount에 저장
model/utilityModel: getRandomNum() 추가
before : racingGameModel인스턴스 생성 후 사용자 입력을 받아 각 멤버변수에 접근해 저장. after : 사용자 입력을 받아 racingGameModel 생성자의 파라미터로 전달해 인스턴스 생성과 함께 초기화
before : racingGameModel에서 ArrayList<Sting> racingCarNames에 경주할 자동차 이름 저장 after : racingCarInfo class선언해서 경주할 자동차의 이름, 움직인 횟수를 한번에 관리 stringArrayToStringArrayList()메서드 불필요해짐에 따라 삭제.
controller/game : startRacing() 추가 model/racingGameModel : updateRaceStatus()추가 view : raceStatus 추가 view/raceStatus : printRaceStatus(), printMoveStatus() 추가. attemptCount는 racingGameModel의 멤버일 필요가 없다고 판단되어 삭제.
System.out.println(EXECUTION_RESULT);
model/racingGameModel : findWinner(), findMaxCountOfCarInfos()추가. view : raceResult 추가. view/raceResult : showWinners()추가. controller/game : endRacing() 추가.
controller/userInputController : getRaceCarNames(), getAttemptCount() 추가 & userInput()삭제
findWinner()->getWinnerNames() & addWinnerIfMaxCount()추가.
Math.max 사용해 if문 제거.
model/racingGameModel : compareRandomToBase()메서드 추가.
isInValid()를 UserInput-getRaceCarNames로 이동. isInValid()코드 수정.
racingcar/model/GameModelTest
view -> controller
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.
리뷰 남겼습니다~!
UserInput userInput = new UserInput(); | ||
//경주할 자동차 이름 입력 | ||
String[] carNamesArray = userInput.getRaceCarNames(); | ||
//시도 횟수 입력 | ||
int attemptCount = userInput.getAttemptCount(); | ||
//gameModel class 인스턴스 초기화 | ||
GameModel gameModel = new GameModel(carNamesArray); | ||
//랜덤값에 따라 게임 현황 업데이트 및 출력 | ||
startRacing(attemptCount, gameModel); | ||
//게임 결과 판독 및 출력 | ||
endRacing(gameModel); |
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 startRacing(int attemptCount, GameModel gameModel) { | ||
RaceStatus raceStatus = new RaceStatus(); | ||
System.out.println("\n" + EXECUTION_RESULT); |
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.
mvc 모델을 적용하셨으니 실제로 System.out.println()과 같은 입/출력을 담당하는 코드는 view패키지의 클래스가 담당하도록 옮겨봐도 좋을 것 같아요! 다른 controller 패키지 내부에서도 입/출력 부분도 view로 옮기면 좋을 것 같습니다!
import static model.UtilityModel.isInValidNames; | ||
import static model.UtilityModel.splitByComma; | ||
import static model.UtilityModel.stringToInt; | ||
import static view.GuideMessage.PLEASE_INPUT_RACINGCAR_NAMES; | ||
import static view.GuideMessage.PLEASE_INPUT_RACING_COUNT; | ||
|
||
import camp.nextstep.edu.missionutils.Console; | ||
|
||
public class UserInput { |
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 startRacing(int attemptCount, GameModel gameModel) { | ||
RaceStatus raceStatus = new RaceStatus(); | ||
System.out.println("\n" + EXECUTION_RESULT); | ||
for (int i = 0; i < attemptCount; i++) { |
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.
i 도 조금은 사소할 수 있지만(?) 의미있게 이름을 지어줘봐도 좋을 것 같습니다!
public static int MIN_NUMBER = 0; | ||
public static int MAX_NUMBER = 9; | ||
public static int BASE_NUMBER = 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.
현재 클래스는 상태값만 가지고 있는데, enum을 한번 활용해보셔도 좋을 것 같습니다!
public static String PLEASE_INPUT_RACING_COUNT = "시도할 회수는 몇회인가요?"; | ||
public static String EXECUTION_RESULT = "실행 결과"; | ||
public static String FINAL_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.
클래스에서 별다른 동작을 수행하는 메서드가 없다면 마찬가지로 enum을 한번 고려해봐도 좋을 것 같습니다!
for (int count = 0; count < carInfo.moveCount; count++) { | ||
System.out.print("-"); | ||
} |
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.
안녕하세요! 리뷰 남겨볼게요!
단순 숫자나 문자보다 의미있는 이름을 가진 static final과 같은 상수를 사용해서 분리해주는 것도 좋을 것 같습니다. "매직넘버"를 사용하지 말아라 라는 말이 있는데 찾아보시면 좋을 것 같아요!
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.
2주차 미션 고생하셨습니다 !!
코드 짜시는데 생각이 느껴져서 좋았습니다. 3주차도 같이 고생해요 ㅎㅎ
public void gameStart() { | ||
UserInput userInput = new UserInput(); | ||
//경주할 자동차 이름 입력 | ||
String[] carNamesArray = userInput.getRaceCarNames(); |
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.
String배열보단 List 컬렉션을 사용하는게 어떨까요?
gameModel.updateRaceStatus(); | ||
raceStatus.printRaceStatus(gameModel); |
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.
game model이 race status를 업데이트하고, raceStatus는 또 gameModel을 받아서 출력하고 ..
뭔가 더 좋은 방법이 있을 것 같아요 ㅎㅎ
public String[] getRaceCarNames() { | ||
System.out.println(PLEASE_INPUT_RACINGCAR_NAMES); | ||
String userInput = Console.readLine(); | ||
String[] carNamesArray = splitByComma(userInput); |
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.
Array는 빼도 의미가 통할 것 같네용
import java.util.List; | ||
|
||
public class GameModel { | ||
public ArrayList<RacingCarInfo> racingCarInfos; |
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로 받는건 어떨까요?
|
||
public GameModel(String[] carNamesArray) { | ||
racingCarInfos = new ArrayList<RacingCarInfo>(); | ||
int idx = 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.
언뜻봐서는 RacingCarInfo에서 idx를 활용하는 부분은 없어보이는데 값을 넣어주신 이유가 있을까요??
package model; | ||
|
||
public class RacingCarInfo { | ||
public final 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.
public으로 설정하신 이유가 있을까요??
|
||
public static void isInValidNames(String[] names) { | ||
for (String name : names) { | ||
compareLengthToMaxLength(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.
메서드 추출하셔서 가독성이 좋아진것 같습니다 !
|
||
public int getAttemptCount() { | ||
System.out.println(PLEASE_INPUT_RACING_COUNT); | ||
String userInput = Console.readLine(); |
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.
시도횟수 입력에 대한 validate도 있다면 좋을 것 같습니다 ㅎㅎ
No description provided.