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

[202201912 이서인] 1주차 미션을 제출합니다. #1

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

Conversation

sslynn22
Copy link

No description provided.

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.

안녕하세요 서인님~
지금 코드를 보니 컴파일이 제대로 이루어지지 않았네요~
카톡 방에 올라온 공지를 보고 rebase는 완료된 것은 확인했으나,
camp.nextstep.edu.missionutils 대신 mallang.missionutils 패키지의 Console과 Random을 사용하도록
코드를 바꿔주시면 감사하겠습니다~

제가 임의로 바꾼 뒤 테스트를 진행해보니 테스트가 성공되지 않고 있어요!
스크린샷 2024-02-17 오후 5 49 08

테스트가 성공하는것을 확인한 뒤 제출해주시면 감사하겠습니다.
(System.exit()을 사용하면 테스트가 제대로 진행되지 않으니 다른방법으로 게임을 종료시켜주세요 :))

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.

테스트는 아직 통과하지 않지만, 작성된 코드를 바탕으로 간단히 리뷰를 남겨드렸어요!
코드를 보니 메서드 분리를 너무 잘 해주신 것 같아 코드를 이해하기 쉬웠습니다 :)
이번 미션에서는 이정도만 작성해도 충분히 잘 작성한 코드지만, 잘 작성해 주셨으니 조금만 더 귀찮게 해보겠습니다..ㅎㅎ

지금은 하나의 Application 코드에 모든 로직을 작성해 주셨어요!
이번 미션은 간단하기 떄문에 이렇게 작성해도 문제는 없지만,
구현해야하는 기능이 복잡해 질 수록 이렇게 하나의 클래스에 모든 기능을 구현하게 되면 유지보수에 어려움을 겪을 수 있습니다~
클래스를 조금 나누어 책임을 분리하여, 가독성과 유지보수성을 올릴 수 있게 코드를 변경한 뒤 저에게 말씀해주시면 추가 피드백 드릴게요!

추가로 커밋 단위를 좀 더 분리하는 연습을 해보시면 좋을 것 같아요~

잘 모르겠다면 다른 분들의 코드를 참고해보는 것도 좋을 것 같습니다 :)

Comment on lines 13 to 15
private static final int MIN = 1;
private static final int MAX = 9;
private static final int BALL_SIZE = 3;
Copy link
Contributor

@shin-mallang shin-mallang Feb 17, 2024

Choose a reason for hiding this comment

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

매직 넘버 대신 상수로 정의해서 사용해 주셨네요 💯

변수의 이름을 조금 더 명확하게 지어주는 것은 어떨까요?

MIN이 정확히 어떤 것에 대한 최소인지,
MAX가 정확히 어떤 것에 대한 최대인지 알기 어려워 보입니다!

Copy link
Author

Choose a reason for hiding this comment

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

넵 수정하겠습니다!

Comment on lines 18 to 20
while (true) {
playGame();
}
Copy link
Contributor

Choose a reason for hiding this comment

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

System.exit를 사용하지 않고 게임을 종료하려면 이 부분에 대한 수정이 필요할 것 같네요 :)

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.

아하 그렇군요! 꼭 필요한 부분에만 주석 작성하도록 하겠습니다 :>

Comment on lines 25 to 27
// 랜덤 3자리 숫자 생성
int computerBall[] = generateRandomBall();
System.out.println("랜덤 숫자 : " + Arrays.toString(computerBall));
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.

넵! 메서드로 코드 작성해보겠습니당

Comment on lines 34 to 36
// 볼, 스트라이크, 낫싱 체크
int ballScore = ballCheck(computerBall, userBall);
int strikeScore = strikeCheck(computerBall, userBall);
Copy link
Contributor

Choose a reason for hiding this comment

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

ballCheck, strikeCheck 만으로도 이미 무엇을 하는지 의미가 명확하다고 생각해요! 💯
주석은 불필요해 보여요!

Copy link
Author

Choose a reason for hiding this comment

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

넵! 주석 삭제해놓겠습니다!

Comment on lines 38 to 51
// 힌트 출력
if (strikeScore != 0 && ballScore == 0) {
System.out.println(strikeScore + "스트라이크");
} else if (strikeScore == 0 && ballScore != 0) {
System.out.println(ballScore + "볼");
} else if (strikeScore == 3) {
System.out.println(strikeScore + "스트라이크");
System.out.println("3개의 숫자를 모두 맞히셨습니다! 게임 종료");
playAgain();
} else if (strikeScore == 0 && ballScore == 0) {
System.out.println("낫싱");
} else {
System.out.println(ballScore + "볼" + strikeScore + "스트라이크");
}
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.

넵! 수정하겠습니당

}

/* 게임 반복 메소드 */
private static void playAgain() {
Copy link
Contributor

Choose a reason for hiding this comment

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

제공드린 코드 컨벤션 문서를 참고하여 메서드 순서를 올바르게 변경해주세요 :)

if (inputNum.length() != BALL_SIZE) {
throw new IllegalArgumentException("3자리 숫자를 입력해주세요");
}
int[] userBall = new int[BALL_SIZE];
Copy link
Contributor

Choose a reason for hiding this comment

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

배열 대신 List를 사용해봐요!

Copy link
Author

Choose a reason for hiding this comment

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

넵! 리스트로 변경하겠습니당

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