Skip to content

260616_10_박승빈#61

Open
pigyou0601 wants to merge 4 commits into
SurvivalCodingCampus:student/10_박승빈from
pigyou0601:master
Open

260616_10_박승빈#61
pigyou0601 wants to merge 4 commits into
SurvivalCodingCampus:student/10_박승빈from
pigyou0601:master

Conversation

@pigyou0601

Copy link
Copy Markdown

YYMMDD_01_홍길동

📝 과제 정보

상속

  • 교육 주제:
    부모클래스와 자식클래스를 올바르게 사용하는 방법

📋 체크리스트

  • [ v] 코드 컨벤션 준수 (Formatter 적용)
  • [ v] 모든 테스트 케이스 통과 (JUnit)

📷 실행 결과 (또는 테스트 결과)

image

❓질문 및 어려웠던 점

자식클래스가 부모클래스의 필드값이나 클래스를 사용할 수 있게 만드는 것
클래스의 필드값이 상수일 때 가져오게 하는 방법

🔄 자체 평가 & 회고

상속에서 잘못 이해했던 점에 대해 다시 한번 재대로 이해함
getter와 setter를 재대로 사용하는 방법에 대해 배

@coderabbitai

coderabbitai Bot commented Jun 16, 2026

Copy link
Copy Markdown

Review Change Stack

상속 과제 코드 리뷰

🎯 좋은 점들

1. 캡슐화 원칙의 적용

  • Hero 클래스에서 getHp()setHp(int hp) 메서드를 통해 hp 필드를 보호하고 있습니다. 특히 setHp()에서 Math.min(hp, MAX_HP)를 사용하여 최대 체력 초과를 방지하는 로직이 좋습니다.
  • Slime 클래스에서 suffixfinal로 선언하여 불변성을 보장한 점도 잘 되어 있습니다.

2. 상속의 올바른 활용

  • Wizard를 상속받은 GreatWizard가 생성자에서 this.mp, this.heal, this.mpCost를 통해 부모 클래스의 protected 필드에 접근하고 있습니다.
  • PoisonSlimeSlime을 상속받고 super.attack(hero)를 호출하여 기존 기능을 재사용한 후 추가 기능을 구현하는 방식이 다형성을 잘 보여줍니다.

3. 접근 제어자의 적절한 선택

  • Wizard 클래스에서 heal, mp, mpCostprotected로 설정하여 자식 클래스(GreatWizard)에서만 접근할 수 있도록 설계했습니다.
  • PoisonSlimepoisonCountprivate으로 설정하여 외부에서 직접 접근할 수 없게 보호하고 있습니다.

❓ 생각해볼 점들

1. 클래스 정의의 일관성
현재 코드에서 Main.java의 다음 라인을 보시면:

SuperHero superHero = new SuperHero("홍길동", 100);

SuperHero 클래스가 어디에 정의되어 있나요? 🤔

  • 만약 이 클래스가 필요하다면, 어떤 클래스를 상속받아야 할까요?
  • SuperHeroHero의 관계는 어떤 상속 관계여야 할까요?

2. 부모 클래스의 초기화
Wizard 클래스를 보면, namehp 필드가 정의되어 있지만 생성자에서 초기화되지 않고 있습니다.

  • 부모 클래스의 필드가 초기화되지 않으면 자식 클래스에서 어떤 문제가 발생할까요?
  • GreatWizard 생성자에서 부모 클래스의 생성자를 호출(super())할 필요가 있을까요?

3. Hero와 Wizard의 관계
현재 Hero 클래스와 Wizard 클래스가 독립적으로 존재합니다. 하지만 두 클래스를 살펴보면:

  • 둘 다 name, hp 필드를 가지고 있습니다
  • 둘 다 게임 캐릭터입니다

이렇게 공통된 부분이 있을 때, 상속의 관점에서 어떻게 설계할 수 있을까요? 💭

  • 공통 필드와 메서드를 모두 부모 클래스로 올려서 관리하면 어떤 이점이 있을까요?

4. setHp() 메서드의 범용성
Wizard 클래스의 heal() 메서드에서 hero.setHp(hero.getHp() + heal)을 사용하고 있습니다.

  • 현재 구현으로 의도한 동작이 제대로 이루어질까요?
  • setHp()는 최대값을 초과하지 않도록 조정하는데, 이때 음수 값이 전달되면 어떻게 될까요? 혹시 추가 검증이 필요하지 않을까요?

5. 메서드 오버라이딩의 의도
GreatWizardheal() 메서드를 보면:

public void heal(Hero hero) {
    super.heal(hero);
}

현재는 부모 메서드를 그대로 호출하고 있습니다.

  • 이 메서드가 정말 필요한가요? 제거해도 동작에 문제가 없을까요?
  • 만약 GreatWizard만의 특별한 heal() 로직이 필요하다면 어떻게 구현해야 할까요?

📝 학습을 위한 제안

상속 설계 재검토:
현재 클래스 구조를 다시 한 번 살펴보세요. HeroWizard, 그리고 SuperHero가 어떤 관계를 가져야 하는지 생각해보세요. 게임 캐릭터들이 공통으로 가진 속성과 행동이 무엇인지 파악하면, 더 효율적인 상속 구조를 설계할 수 있을 것입니다.

접근 제어자의 역할:
protected, private, default, public의 차이를 다시 한 번 정리해보세요. 각각 언제 어떤 목적으로 사용하는지 명확히 이해하면, 더 안전하고 유지보수하기 좋은 코드를 작성할 수 있습니다.

수고하셨습니다! 상속의 기본 개념을 잘 이해하고 구현하고 있으세요. 위의 질문들을 통해 더욱 견고한 설계를 만들어보세요. 🚀

Walkthrough

Hero 클래스에 상태 필드·접근자·행동 메서드가 확장되고, SlimePoisonSlime, WizardGreatWizard 상속 계층이 신규 추가된다. Main이 이 클래스들을 조합한 실행 흐름으로 교체되며, 11장 상속 연습문제 문서(Exercise.md)가 함께 추가된다.

Changes

상속 기반 게임 클래스 계층 구현

Layer / File(s) Summary
Hero 클래스 상태·접근자·행동 메서드 확장
game/src/main/java/com/survivalcoding/Hero.java
MAX_HP 상수, money, Sword 필드 추가. 다중 생성자, public getHp()/setHp(int)(상한 적용), setRandomMoney() 신규 추가. run(), sit(int), slip(), sleep() 행동 메서드가 새로 포함됨.
Slime / PoisonSlime 적 계층
game/src/main/java/com/survivalcoding/Slime.java, game/src/main/java/com/survivalcoding/PoisonSlime.java
Slimesuffix·hp 관리와 고정 10 데미지 attack(Hero)을 제공. PoisonSlimeSlime 상속 후 poisonCount(초기값 5) 소진까지 hero.getHp()/5의 독 데미지를 추가 적용.
Wizard / GreatWizard 힐러 계층
game/src/main/java/com/survivalcoding/Wizard.java, game/src/main/java/com/survivalcoding/GreatWizard.java
Wizardprotected 마나 필드와 조건 분기 heal(Hero) 제공. GreatWizardWizard 상속 후 superHeal(Hero)(mp≥50 시 HP를 MAX_HP로 완전 회복)를 추가.
Main 실행 흐름 교체 및 연습문제 문서 추가
game/src/main/java/com/survivalcoding/Main.java, game/src/main/Exercise.md
Main.mainHero·SuperHero·PoisonSlime·GreatWizard 인스턴스를 조합한 흐름으로 교체되고 기존 코드는 주석 처리됨. Exercise.md에 11장 상속 연습문제와 정답이 새로 작성됨.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning PR 제목 '260616_10_박승빈'은 상속 과제 내용과 무관하며, 날짜와 학생명만 포함되어 있어 변경사항의 핵심을 명확히 전달하지 못합니다. 상속 관련 기능 구현 내용을 반영한 제목으로 변경하세요 (예: '상속을 활용한 Hero, Wizard, Slime 클래스 구현').
✅ Passed checks (4 passed)
Check name Status Explanation
Description check ✅ Passed PR 설명이 거의 모든 필수 섹션을 포함하고 있으나, '자체 평가 & 회고' 섹션이 불완전하게 작성되어 있습니다.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@game/src/main/Exercise.md`:
- Line 3: The markdown headings are missing required spaces after the hash
symbol, violating the MD018 markdownlint rule. In the file
game/src/main/Exercise.md, add a space between the hash symbol and the heading
text at line 3 (change `#문제 1` to `# 문제 1`) and at line 13 (change `#문제` to `#
문제`) to comply with proper markdown syntax for ATX headings.

In `@game/src/main/java/com/survivalcoding/Hero.java`:
- Around line 54-60: The sit method directly modifies this.hp instead of using
the setHp setter method, which bypasses the boundary value contract enforcement.
Replace the direct assignment this.hp += sec with a call to setHp(this.hp + sec)
in the sit method. Additionally, apply the same fix to the other location at
lines 68-70 where this.hp is directly modified, ensuring all HP modifications
throughout the Hero class use the setHp method to maintain consistency and
enforce upper/lower bounds.
- Around line 32-34: The setHp method only constrains the upper limit of HP to
MAX_HP using Math.min, but does not enforce a lower bound, allowing negative HP
values to be stored. Modify the setHp method to also apply a lower bound
constraint of 0, ensuring that HP never goes below zero. This can be
accomplished by applying both Math.min for the upper limit and Math.max for the
lower limit (0) to the hp parameter.

In `@game/src/main/java/com/survivalcoding/Wizard.java`:
- Around line 13-16: The mana condition in the heal method of the Wizard class
is hardcoded to check if this.mp >= 10, but then deducts this.mp -= mpCost,
which may be a different value (e.g., 5 in GreatWizard subclass). Change the
condition from this.mp >= 10 to this.mp >= mpCost so that both the validation
check and the mana deduction use the same field, ensuring that subclass
overrides of mpCost are properly respected and the logic remains consistent.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 77c8db58-2c75-4f08-950d-b3620360b068

📥 Commits

Reviewing files that changed from the base of the PR and between 677773e and 65aa665.

📒 Files selected for processing (7)
  • game/src/main/Exercise.md
  • game/src/main/java/com/survivalcoding/GreatWizard.java
  • game/src/main/java/com/survivalcoding/Hero.java
  • game/src/main/java/com/survivalcoding/Main.java
  • game/src/main/java/com/survivalcoding/PoisonSlime.java
  • game/src/main/java/com/survivalcoding/Slime.java
  • game/src/main/java/com/survivalcoding/Wizard.java

Comment thread game/src/main/Exercise.md
@@ -0,0 +1,22 @@
11장 상속 연습문제

#문제 1

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

ATX 헤딩 뒤 공백 누락(MD018) 수정이 필요합니다.

Line 3, Line 13의 #문제 형식은 markdownlint 경고 대상입니다. # 문제로 공백을 넣어 주세요.

수정 제안
-#문제 1
+# 문제 1
...
-#문제 2
+# 문제 2

Also applies to: 13-13

🧰 Tools
🪛 markdownlint-cli2 (0.22.1)

[warning] 3-3: No space after hash on atx style heading

(MD018, no-missing-space-atx)

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@game/src/main/Exercise.md` at line 3, The markdown headings are missing
required spaces after the hash symbol, violating the MD018 markdownlint rule. In
the file game/src/main/Exercise.md, add a space between the hash symbol and the
heading text at line 3 (change `#문제 1` to `# 문제 1`) and at line 13 (change `#문제`
to `# 문제`) to comply with proper markdown syntax for ATX headings.

Source: Linters/SAST tools

Comment on lines +32 to +34
public void setHp(int hp) {
this.hp = Math.min(hp, MAX_HP);
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

setHp가 음수 HP를 허용하고 있습니다.

Line 32-34에서 상한만 제한해 음수값이 그대로 저장됩니다. 전투/독 데미지 누적 시 HP가 음수로 내려가 UI/로직이 깨질 수 있습니다.

수정 제안
 public void setHp(int hp) {
-    this.hp = Math.min(hp, MAX_HP);
+    this.hp = Math.max(0, Math.min(hp, MAX_HP));
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
public void setHp(int hp) {
this.hp = Math.min(hp, MAX_HP);
}
public void setHp(int hp) {
this.hp = Math.max(0, Math.min(hp, MAX_HP));
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@game/src/main/java/com/survivalcoding/Hero.java` around lines 32 - 34, The
setHp method only constrains the upper limit of HP to MAX_HP using Math.min, but
does not enforce a lower bound, allowing negative HP values to be stored. Modify
the setHp method to also apply a lower bound constraint of 0, ensuring that HP
never goes below zero. This can be accomplished by applying both Math.min for
the upper limit and Math.max for the lower limit (0) to the hp parameter.

Comment on lines +54 to +60
void sit(int sec) {
//앉은 초 수만큼 hp 증가
this.hp += sec;

System.out.println(this.name + "는" + sec + "초 앉았다");
System.out.println("HP가" + sec + "포인트 회복되었다");
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

HP 변경 로직이 setter를 우회해 상한/하한 계약이 깨집니다.

Line 54-60, 68-70에서 this.hp를 직접 변경/대입합니다. 이미 setHp로 경계값 계약을 관리하고 있으므로, 내부 행동 메서드도 동일 경로를 사용해야 일관성이 유지됩니다.

수정 제안
 void sit(int sec) {
-    this.hp += sec;
+    setHp(this.hp + sec);

     System.out.println(this.name + "는" + sec + "초 앉았다");
     System.out.println("HP가" + sec + "포인트 회복되었다");
 }

 void sleep() {
-    this.hp = 100;
+    setHp(MAX_HP);
     System.out.println(this.name + "는 잠을 자고 회복했다!");
 }

Also applies to: 68-70

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@game/src/main/java/com/survivalcoding/Hero.java` around lines 54 - 60, The
sit method directly modifies this.hp instead of using the setHp setter method,
which bypasses the boundary value contract enforcement. Replace the direct
assignment this.hp += sec with a call to setHp(this.hp + sec) in the sit method.
Additionally, apply the same fix to the other location at lines 68-70 where
this.hp is directly modified, ensuring all HP modifications throughout the Hero
class use the setHp method to maintain consistency and enforce upper/lower
bounds.

Comment on lines +13 to +16
if (this.mp >= 10) {
hero.setHp(hero.getHp() + heal);
this.mp -= mpCost;
System.out.println("힐을 시전했습니다. " + hero.name + "HP : " + hero.getHp());

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

마나 조건이 mpCost를 무시하고 하드코딩되어 있습니다.

Line 13에서 this.mp >= 10을 사용해 mpCost 변경(예: GreatWizard의 5)이 반영되지 않습니다. 조건과 차감 기준을 동일 필드(mpCost)로 맞춰야 합니다.

수정 제안
-        if (this.mp >= 10) {
+        if (this.mp >= this.mpCost) {
             hero.setHp(hero.getHp() + heal);
             this.mp -= mpCost;
             System.out.println("힐을 시전했습니다. " + hero.name + "HP : " + hero.getHp());
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if (this.mp >= 10) {
hero.setHp(hero.getHp() + heal);
this.mp -= mpCost;
System.out.println("힐을 시전했습니다. " + hero.name + "HP : " + hero.getHp());
if (this.mp >= this.mpCost) {
hero.setHp(hero.getHp() + heal);
this.mp -= mpCost;
System.out.println("힐을 시전했습니다. " + hero.name + "HP : " + hero.getHp());
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@game/src/main/java/com/survivalcoding/Wizard.java` around lines 13 - 16, The
mana condition in the heal method of the Wizard class is hardcoded to check if
this.mp >= 10, but then deducts this.mp -= mpCost, which may be a different
value (e.g., 5 in GreatWizard subclass). Change the condition from this.mp >= 10
to this.mp >= mpCost so that both the validation check and the mana deduction
use the same field, ensuring that subclass overrides of mpCost are properly
respected and the logic remains consistent.

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