-
Notifications
You must be signed in to change notification settings - Fork 35
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
[보스 몬스터 잡기] 신우경 미션 제출합니다. #7
base: kung036
Are you sure you want to change the base?
Conversation
- 상태 출력 - 물리 공격 - 마법 공격 - 데미지로 인한 HP 감소
- 상태 출력 - 공격 - 데미지로 인한 HP 감소
안녕하세요 우경님! 바톤팀 에단입니다. 만나서 반값습니다! 리뷰는 내일까지 진행하겠습니다! |
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.
안녕하세요 우경님! 바톤팀의 에단입니다.😄
코드 확인했고, 정상적으로 동작하는 걸 확인했습니다. 코맨트를 남겨놨으니 확인해시면 감사하겠습니다!
추가로 전체적인 코멘트를 남기자면 객체에 책임에 대해서 좀 더 고민하시면 훨신 더 좋은 코드가 될 것 같아요.
이와 관련해서 객체지향의 사실과 오해라는 책을 추후에 학습해도 좋을 것 같습니다.
리뷰 반영해서 올려주시면 계속 리뷰 이어가겠습니다. 더욱 멋진 코드 기대할게요💪💪
package bossmonster.character; | ||
|
||
public class BossMoster { | ||
private int HP; // 100 ~ 300 |
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.
구글코드컨벤션에 따르면 자바에서 필드명은 lowerCamelCase로 작성해요😄 다른 부분도 수정하면 좋을것 같아요!
참고
return false; | ||
} | ||
|
||
} |
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.
POSIX 명세라는게 존재해요!
public class Main { | ||
public static void main(String[] args) { | ||
System.out.println("Hello world!"); | ||
Play play = new Play(); | ||
play.start(); |
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.
import java.util.Random; | ||
|
||
// 보스 몬스터 행동 | ||
public class BossMonsterAction { |
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.
현재 BossMonsterAction은 크게 보면 행동과 출력으로 나뉘는데요. 상태 출력하는 것이 BossMonsterAction가 가져야 할 책임일까요? 우경님의 생각이 궁금합니다. 출력부분이 BossMonsterAction에 있는 것과 없는 것 어떤 차이가 있을까요?
// 플레이어에게 공격 당함 | ||
public void damage(int damage) { | ||
bossMoster.setCurrentHP(bossMoster.getCurrentHP() - damage); | ||
if (bossMoster.getCurrentHP() < 0) bossMoster.setCurrentHP(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.
구글컨벤션에 따르면 비어있는 if문도 중괄호를 사용합니다!
|
||
// 플레이어에게 공격 당함 | ||
public void damage(int damage) { | ||
bossMoster.setCurrentHP(bossMoster.getCurrentHP() - damage); |
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.
setter를 사용하지 않고 구현해볼까요? setter를 사용한 경우와 사용하지 않은 경우 어떤 장단점이 있을가요?
try { | ||
throw new IllegalArgumentException(); | ||
} catch (IllegalArgumentException e) { | ||
Error.printError("보스 몬스터 초기 HP는 100 이상 300 이하입니다."); | ||
} |
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.
try-catch 문이 예외를 핸들링 하는게 아니라 if문 처럼 동작하는 것처럼 느껴져요.
어떻게 하면 사용자 입력이 잘못되었을 때, 예외를 던지면서 옳은 값을 입력받을 때 까지 반복할 수 있을까요?
public void start() { | ||
// 첫 시작 | ||
|
||
while (!bossMonsterAction.checkHP() && !playerAction.checkHP()) { |
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.
클린코드에서는 부정조건절을 지양해요.
import bossmonster.error.Error; | ||
|
||
// 게임 | ||
public class Play extends InitialCharacter { |
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.
InitialCharacter를 상속한 이유가 무엇인가요?
// 플레이어 공격 | ||
private boolean playerAttack(int attack) { | ||
boolean check = true; // 공격 성공한 경우 | ||
if (attack == 1) { // 물리공격 |
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.
1번이 물리공격이군요! 물리공격이라는 주석을 지운다면 어떻게 다른 개발자에게 물리공격이라고 알려줄 수 있을까요?
안녕하세요 우경님! 리뷰 반영을 해주셔야 추가적인 리뷰를 진행할 수 있습니다. |
고민 사항
처음으로 코드리뷰 받아봅니다!
항상 혼자서 코드를 작성했는데, 내 코드에 어떤 문제점이 있는지 항상 궁금했습니다.
좋은 코드를 작성하고 싶은데, 어떤 점을 고치면 좋을지 조언해주시면 감사하겠습니다.