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

[5castle0] 백엔드 스프링 1주차 미션 제출합니다. #11

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

Conversation

5castle0
Copy link

No description provided.

@5castle0 5castle0 requested a review from cmj7271 as a code owner November 23, 2024 11:29
Copy link

@astrokan astrokan left a comment

Choose a reason for hiding this comment

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

미션 명세에 충실하면서도 코드가 간결하고 가독성이 좋아요.
주석 처리도 신경써주신 덕분에 읽기 너무 편했습니다😁😁

콘솔 창으로부터 받는 입력 예외 처리

1주차 미션의 명세에 포함되지는 않지만..

Application.javamain(), game.javarunGame()에서

콘솔창으로부터 원하지 않는 입력(ex.턴 수를 입력받을 때 정수형이 아닌 문자열을 입력받음)
을 받았을 때의 예외 처리를 해준다거나,

원하는 입력을 받을 때까지 무한루프를 돌린다거나 하는 식의 코드를 추가한다면
더 멋진 코드가 될 거 같아요!

아 물론 플레이어가 입력 양식을 잘 지키는 올바른(?) 사람이라고 가정한다면 수정할 필요 전혀 없습니다ㅎㅎㅎ

+)

자바 공통 네이밍 규칙에 따라 game.java->Game.java 로 네이밍 수정하시면 좋을 거 같아요..!!!
아마 오탈자라고 생각합니다^__^

Comment on lines 37 to 65
if (choiceA == 2 || choiceB == 2) { //둘 중 하나는 방어, 하나는 공격
if (choiceA == 2 && choiceB == 2) {
return;
} //둘다 방어일 경우 turn 넘김
else if (choiceA == 2) { //A가 방어인 경우
//코드를 함수화할 수 있을 것 같지만,,,, 난 바쁘다,,
if (isSkill(B, choiceB) == 0) return;
int[] actionList = {action.attack(), action.defense(), action.attack2(), action.attack3(), action.attackHard()}; //그때그때 랜덤값 생성을 위함
int damage = max(0, actionList[choiceB - 1] - actionList[1]); //damage는 음수일 수 없음
A.losehp(damage);
} else { //B가 방어인 경우
if (isSkill(A, choiceA) == 0) return;
int[] actionList = {action.attack(), action.defense(), action.attack2(), action.attack3(), action.attackHard()};
int damage = max(0, actionList[choiceA - 1] - actionList[1]);
B.losehp(damage);
}
} else {//둘다 공격인 경우
if (isSkill(A, choiceA) == 0) return; //마나 없음
else {
int[] actionListA = {action.attack(), action.defense(), action.attack2(), action.attack3(), action.attackHard()};
int damageA = actionListA[choiceA - 1]; //A가 먼저 공격
B.losehp(damageA);
}
if (B.getHp() <= 0) return; // B죽음 (A가 무조건 먼저 공격)
if (isSkill(B, choiceB) == 0) return; //마나 없음
int[] actionListB = {action.attack(), action.defense(), action.attack2(), action.attack3(), action.attackHard()};
int damageB = actionListB[choiceB - 1];
A.losehp(damageB);
}

Choose a reason for hiding this comment

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

[변경 제안] 케이스 분류 개선

if 조건문의 케이스 분류 개선 가능성이 있어 제안드립니다.

if (choiceA == 2 || choiceB == 2) { //둘 중 하나는 방어, 하나는 공격
            if (choiceA == 2 && choiceB == 2) {
                return;
            } //둘다 방어일 경우 turn 넘김

여기서 가능한 모든 케이스를 고려하시고 계신 점은 코드를 통해 충분히 인지하였습니다.

(choiceA == 2 || choiceB == 2): "A 방어 또는 B 방어일 때"

이 조건절과 대응하는 else는 당연히 "A와 B 모두 공격"인 경우를 다루게 될 겁니다.

하지만 //둘 중 하나는 방어, 하나는 공격 코멘트를 보아서는

if에서 처리하고자 하는 case는 방어가 유효해지는 case야...!!!

와 같은 의도로 받아들여지는데요,

만약 코멘트의 의도가 있으시다면 if절 내에서 둘 다 방어인, 즉 둘의 방어가 무효한 case를 처리하는 것이 조금은 어색하게 느껴집니다.

하단의 수도 코드와 같은 케이스 분류는 어떠할까요..?

if (둘 다 공격) {
...
}
else if (둘 중 하나 방어) {
...
}
else { // 둘 다 방어
...
}

Copy link
Author

Choose a reason for hiding this comment

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

다시보니 제가봐도 분류한 if문이 어색하게 느껴지네요! 좋은 의견 감사드립니다~~~

Comment on lines 34 to 35
Action a = new Action();
Action b = new Action();

Choose a reason for hiding this comment

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

[질문] 사용되지 않은 객체가 있어요.

함수 내에서 선언된 Action 타입 객체 a, b 가 한 번도 사용되지 않았습니다.

혹시 수정 중이신 부분인가요..?
아니라면 지워도 무방할 것 같습니다!!

Copy link
Author

Choose a reason for hiding this comment

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

명세에 맞게 수정하느라 해당 객체는 삭제했습니다! 감사합니당

@5castle0
Copy link
Author

@astrokan 너무너무너무 꼼꼼하고 유용한 피드백 감사드립니다! 열심히 봐주신 정성이 여기까지 느껴집니다 ㅠㅠ 해당 리뷰들 반영하고 수정해서 다시 올리도록 하겠습니다!

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