Skip to content
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

[201800288 채다영] 1주차 미션을 제출합니다. #4

Open
wants to merge 22 commits into
base: main
Choose a base branch
from

Conversation

CHAEGODA
Copy link

No description provided.

구현할 기능 목록 수정
구현할 기능 목록 수정
구현할 기능 목록 수정1
1 ~ 9 서로 다른 임의의 수 생성 후 배열에 저장
임의의 수 생성 후 배열에 저장
사용자로부터 숫자 입력받아 배열저장
입력받은 String Int로 파싱해주기
정답 확인 함수
스트라이크 개수
결과 출력
게임 재시작
게임 시작
Copy link
Contributor

@shin-mallang shin-mallang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

안녕하세요 다영님~
아직 완성은 안 되신 것 같지만, 그래도 1단계 미션 너무 잘 해주셨어요~ 💯
특히 메서드 순서 컨벤션 맞게 한번에 작성해주신분은 처음인 것 같네요... (감동)

간단하게 리뷰 남겨드렸고, 전반적으로 드리고 싶은 말은 여기에 작성할게요~

먼저 커밋을 잘 나누어 주셨어요~
그런데 커밋 메세지만 보고 해당 커밋에서 어떤 변화가 발생했는지 알기 어려운 것 같아요~

커멋 메세지만 보고도 어떤 작업이 수행되었는지 알 수 있도록 작성해주시면 좋을 것 같아요~

추가로 코드에 주석이 너무 많은 것 같습니다~
주석을 무조건 사용하지 말라는 것은 아니지만, 주석 역시도 유지보수의 대상이므로
주석을 많이 사용하게 되면 그만큼 코드 변경 시 신경써야할 점들이 많아지는 것을 의미해요~

주석이 없더라도 코드를 쉽게 이해할 수 있게 작성하도록 노력해봐요 :)

리뷰에 대해서, 또는 미션 진행하면서 궁금한 점이 있으시다면 언제든 편하게 질문 주세요~

@@ -1,5 +1,24 @@
# 미션 - 숫자 야구 게임

## 구현할 기능 목록
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

기능 목록 잘 작성해주셨네요 💯

}
// 게임 시작
public static void startGame() {
int[] answer = answerNum();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

배열 대신 List를 사용해볼까요?

int[] result = checkNum(answer, guess);
printResult(result);

if (result[2] == 3) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result[2] == 3이 정확히 뭘 의미하는지 알기 어려워요!
어떻게 하면 알기 쉽게 바꿀 수 있을까요?

Comment on lines 27 to 28
}
// 1부터 9까지의 서로 다른 임의의 수를 3개 생성해서 배열에 저장 (숫자가 겹치지 않도록 수정)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주석을 사용하면 주석 위에도 공백 한 칸 부탁드려요!

Comment on lines 27 to 35
}
// 1부터 9까지의 서로 다른 임의의 수를 3개 생성해서 배열에 저장 (숫자가 겹치지 않도록 수정)
public static int[] answerNum() {
int[] answer = new int[3];
for(int i = 0; i < answer.length; i++) {
answer[i] = Randoms.pickNumberInRange(1,9);
}
return answer;
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

행위를 보니 answerNumber를 생성하는 코드 같아요~
메서드 이름을 좀 더 명확하게 바꿔볼까요?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수명만 변경하면 될까? 아니면 이 부분도 배열을 리스트로 바꾸는게 좋나요?
배열 대신 리스트를 사용하는 것이 요소의 삽입/삭제가 용이하기때문일까요?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

함수명 변경 해주시고,
배열은 모두 다 리스트로 바꿔주세요~

요소의 삽입 삭제가 용이하다보다는 여러 가지 이유가 있는데,
배열보다는 리스트를 사용하라는 키워드로 검색해보시면 블로그 글들이 엄청 쏟아질거에요~
한번 찾아보시면 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

감사합니다!

public static int[] userNum() {
System.out.println("숫자를 입력해주세요 : ");
String input = Console.readLine();
int[] guess = parseInput(input);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guess 변수명 너무 좋은 것 같아요 💯
그런데 이 부분도 배열 대신 List로 바꿔볼까요?

추가로 왜 배열을 List로 바꾸라 할까요?

배열은 어떤 단점이 있을까요?

public static int[] parseInput(String input) {
int[] guess = new int[3];
for(int i = 0; i < guess.length; i++) {
guess[i] = Character.getNumericValue(input.charAt(i));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Integer.parseInt도 있습니다~

Comment on lines 52 to 53
// 정답 확인 함수
public static int[] checkNum(int[] answer, int[] guess) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checkNum 대신 정답 확인 함수라는 것을 명확히 드러내는 이름을 사용해볼까요?

int[] result = new int[3];
countBalls(answer, guess, result);
countStrikes(answer, guess, result);
result[2] = 3 - result[0] - result[1]; // 낫싱 개수
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

주석이 없으면 이해하기 힘들겠죠?

주석을 지우고, 주석 없이도 이해하기 쉽게 코드를 바꿔볼 수 있을까요?

int choice = Integer.parseInt(Console.readLine());
return choice == 1;
}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

공백 제거 부탁드려용!

Copy link
Contributor

@shin-mallang shin-mallang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

아고 실수..

 answerNum() 함수명 createNum()으로 변경 /
 배열을 리스트로 변경
숫자를 중복해서 뽑는 것을 방지하기 위한 alreadyNum 함수 생성
parseInput 함수 수정
게임 재시작 함수 continueGame() 수정
@CHAEGODA
Copy link
Author

오류가 발생했는데 해결하지 못했습니다ㅠㅠ
시간이 되어 제출했는데 다시 해볼게요!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants