260616_07_문재원#54
Open
Clair93 wants to merge 6 commits into
Open
Conversation
Pull Request 리뷰 - 상속과 메서드 오버라이딩🎯 전체 평가상속과 다형성의 개념을 잘 이해하고 구현했습니다! 💡 개선 제안 (오류는 아니지만 고민해볼 점들)1. 캡슐화 관점: Hero의 HP 제약생각해볼 질문:
2. 상속과 메서드 오버라이딩: PoisonSlime.attack() 로직생각해볼 질문:
3. 캡슐화: Hero 클래스의 검증 부재생각해볼 질문:
4. 클래스 책임: Wizard와 GreatWizard의 중복 코드생각해볼 질문:
5. 접근 제어자: 패키지 설계생각해볼 질문:
✅ 잘 구현된 부분
📚 학습 포인트이번 PR을 통해 상속, 메서드 오버라이딩, 캡슐화의 실제 관계를 경험하셨습니다. 특히 다음을 고민해보세요:
Walkthrough
ChangesGame 상속 구조 구현
TIL 학습 문서 업데이트
Sequence Diagram(s)sequenceDiagram
participant Caller
participant PoisonSlime
participant Slime
participant Hero
rect rgba(255, 100, 100, 0.5)
note over PoisonSlime,Hero: poisonCount > 0 구간
Caller->>PoisonSlime: attack(hero)
PoisonSlime->>Slime: super.attack(hero)
Slime->>Hero: setHp(hp - 10)
PoisonSlime->>Hero: setHp(hp - hp/5) 독 데미지
PoisonSlime->>PoisonSlime: poisonCount--
end
rect rgba(100, 100, 255, 0.5)
note over PoisonSlime,Hero: poisonCount <= 0 구간
Caller->>PoisonSlime: attack(hero)
PoisonSlime->>Slime: super.attack(hero)
Slime->>Hero: setHp(hp - 10)
end
sequenceDiagram
participant Caller
participant GreatWizard
participant Wizard
participant Hero
rect rgba(100, 200, 100, 0.5)
note over Wizard,Hero: Wizard.heal (mp >= 10)
Caller->>Wizard: heal(hero)
Wizard->>Wizard: mp -= 10
Wizard->>Hero: setHp(hp + 20)
end
rect rgba(200, 150, 50, 0.5)
note over GreatWizard,Hero: GreatWizard.heal (mp >= 5)
Caller->>GreatWizard: heal(hero)
GreatWizard->>GreatWizard: mp -= 5
GreatWizard->>Hero: setHp(hp + 25)
end
rect rgba(150, 50, 200, 0.5)
note over GreatWizard,Hero: GreatWizard.superHeal (mp >= 50)
Caller->>GreatWizard: superHeal(hero)
GreatWizard->>GreatWizard: mp -= 50
GreatWizard->>Hero: setHp(getMaxHp())
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/game.puml`:
- Around line 6-7: Fix the typo in the GreatWizard constructor notation on line
7 where the super call contains `np` which should be `mp` to match the parameter
defined on line 6. Additionally, reformat the constructor signature for the
GreatWizard class to clearly and consistently represent the full constructor
declaration including its parameters, parent class invocation with the corrected
parameter name, and method calls, ensuring the PlantUML diagram matches the
actual code signature.
In `@game/src/main/java/com/survivalcoding/Hero.java`:
- Around line 25-27: The setMaxHp method in the Hero class does not validate its
input or maintain the Hero state invariant that hp should never exceed maxHp.
Add validation to ensure the new maxHp is non-negative (maxHp >= 0), and if the
current hp exceeds the new maxHp after assignment, reduce hp to equal the new
maxHp value. This prevents violations of the Hero state invariant that could
cause issues in downstream code like GreatWizard.superHeal().
In `@game/src/main/java/com/survivalcoding/PoisonSlime.java`:
- Around line 16-26: The issue is that when poisonCount equals 1, both the first
if block (poisonCount > 0) and the second if block (poisonCount <= 0) execute in
the same turn, causing super.attack(hero) to be called twice. The first block
decrements poisonCount to 0, which then satisfies the second condition. Fix this
by changing the second if statement to else if, so that only one attack path is
executed per method call. Specifically, replace the second if (poisonCount <= 0)
condition with else if (poisonCount <= 0) to ensure the basic attack only
executes when there is no poison attack in that turn.
In `@game/src/test/java/com/survivalcoding/PoisonSlimeTest.java`:
- Around line 30-35: The Count test method only verifies that
poisonSlime.getPoisonCount() reaches zero after five attacks, but it does not
validate that the hero's HP actually decreased from those attacks. This means
the attack logic could be broken and the test would still pass. Capture the
hero's initial HP before the attack loop, then after the loop add an
assertEquals assertion to verify that hero.getHp() has been reduced by the
expected poison damage amount, ensuring both the poison count depletion and the
actual damage dealt are properly validated.
In `@TIL/sample/2026-06-16-상속.md`:
- Around line 60-68: The TIL documentation has method signatures that differ
from the actual project implementation. At TIL/sample/2026-06-16-상속.md lines
60-68, update the heal() method signature to accept a Hero parameter as
heal(Hero hero), and change the method body to call hero.setHp(hero.getHp() +
25) instead of setHp(getHp() + 25). Similarly, at TIL/sample/2026-06-16-상속.md
lines 70-79, update the superHeal() method signature to accept a Hero parameter
as superHeal(Hero hero), and change the method body to call
hero.setHp(hero.getMaxHp()) to restore the Hero's HP to maximum. These changes
align the TIL documentation with the actual implementation in
game/src/main/java/com/survivalcoding/GreatWizard.java and its corresponding
test cases.
- Line 33: The code example for the attack() method at line 33 is incomplete and
malformed with an improper comment notation "<<" that breaks the code syntax.
Replace this incomplete line with a complete, well-formed code example that
demonstrates method overriding with a proper `@Override` annotation, a complete
method signature including parameters if needed, a call to the parent class
method using super.attack(), and proper closing braces. Ensure the example is
syntactically correct and clearly illustrates the inheritance concept being
documented.
🪄 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: 8894ce07-97bf-420d-b014-15f9a7685bf9
⛔ Files ignored due to path filters (1)
game/game.pngis excluded by!**/*.png
📒 Files selected for processing (11)
TIL/sample/2026-06-15-캡슐화-컬렉션.mdTIL/sample/2026-06-16-상속.mdgame/game.pumlgame/src/main/java/com/survivalcoding/GreatWizard.javagame/src/main/java/com/survivalcoding/Hero.javagame/src/main/java/com/survivalcoding/PoisonSlime.javagame/src/main/java/com/survivalcoding/Slime.javagame/src/main/java/com/survivalcoding/Wizard.javagame/src/test/java/com/survivalcoding/GreatWizardTest.javagame/src/test/java/com/survivalcoding/PoisonSlimeTest.javagame/src/test/java/com/survivalcoding/WizardTest.java
Comment on lines
+6
to
+7
| +GreatWizard(String:name, int:hp, int:mp | ||
| <<super(name,hp,np)>>, setMp(150)) |
There was a problem hiding this comment.
GreatWizard 생성자 표기 오타(np)와 시그니처 표현이 코드와 불일치합니다.
Line 7의 super(name,hp,np)는 mp 오타이고, 현재 줄바꿈 형태도 생성자 시그니처를 잘못 읽히게 만듭니다. 다이어그램을 코드 시그니처와 동일하게 정리해 주세요.
🔧 제안 수정안
- +GreatWizard(String:name, int:hp, int:mp
- <<super(name,hp,np)>>, setMp(150))
+ +GreatWizard(String:name, int:hp, int:mp, <<super(name,hp,mp)>>, setMp(150))📝 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
| +GreatWizard(String:name, int:hp, int:mp | |
| <<super(name,hp,np)>>, setMp(150)) | |
| GreatWizard(String:name, int:hp, int:mp, <<super(name,hp,mp)>>, setMp(150)) |
🤖 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/game.puml` around lines 6 - 7, Fix the typo in the GreatWizard
constructor notation on line 7 where the super call contains `np` which should
be `mp` to match the parameter defined on line 6. Additionally, reformat the
constructor signature for the GreatWizard class to clearly and consistently
represent the full constructor declaration including its parameters, parent
class invocation with the corrected parameter name, and method calls, ensuring
the PlantUML diagram matches the actual code signature.
Comment on lines
+25
to
+27
| public void setMaxHp(int maxHp) { | ||
| this.maxHp = maxHp; | ||
| } |
There was a problem hiding this comment.
setMaxHp가 Hero 상태 불변식을 깨뜨릴 수 있습니다.
Line 25-27에서 maxHp를 그대로 대입하면 음수 값이나 현재 HP보다 작은 값이 들어갈 수 있고, 이후 GreatWizard.superHeal(...)(game/src/main/java/com/survivalcoding/GreatWizard.java Line 27) 경로에서 예외/비정상 상태로 이어질 수 있습니다. maxHp >= 0 검증과 hp <= maxHp 보정을 함께 넣는 편이 안전합니다.
🔧 제안 수정안
public void setMaxHp(int maxHp) {
- this.maxHp = maxHp;
+ if (maxHp < 0) {
+ throw new IllegalArgumentException("최대 HP는 0 이상이어야 합니다.");
+ }
+ this.maxHp = maxHp;
+ if (this.hp > maxHp) {
+ this.hp = maxHp;
+ }
} public void setHp(int hp) {
if (hp < 0) {
throw new IllegalArgumentException("HP는 0 이상이어야 합니다.");
}
- this.hp = hp;
+ this.hp = Math.min(hp, maxHp);
}🤖 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 25 - 27, The
setMaxHp method in the Hero class does not validate its input or maintain the
Hero state invariant that hp should never exceed maxHp. Add validation to ensure
the new maxHp is non-negative (maxHp >= 0), and if the current hp exceeds the
new maxHp after assignment, reduce hp to equal the new maxHp value. This
prevents violations of the Hero state invariant that could cause issues in
downstream code like GreatWizard.superHeal().
Comment on lines
+16
to
+26
| if (poisonCount > 0) { | ||
| super.attack(hero); | ||
| System.out.println("추가로, 독 포자를 살포했다!"); | ||
| int poisonDamage = hero.getHp() / 5; | ||
| hero.setHp(hero.getHp() - poisonDamage); | ||
| System.out.println(poisonDamage + "포인트 데미지"); | ||
| poisonCount--; | ||
| } | ||
| if (poisonCount <= 0) { | ||
| super.attack(hero); | ||
| } |
There was a problem hiding this comment.
독 카운트가 1일 때 기본 공격이 한 번 더 적용됩니다.
Line 16-26은 poisonCount == 1인 호출에서 첫 번째 블록 실행 후 poisonCount--로 0이 되고, 바로 아래 조건도 참이 되어 super.attack(hero)가 같은 턴에 2회 실행됩니다.
🔧 제안 수정안
`@Override`
void attack(Hero hero) {
if (poisonCount > 0) {
super.attack(hero);
System.out.println("추가로, 독 포자를 살포했다!");
int poisonDamage = hero.getHp() / 5;
hero.setHp(hero.getHp() - poisonDamage);
System.out.println(poisonDamage + "포인트 데미지");
poisonCount--;
- }
- if (poisonCount <= 0) {
+ return;
+ }
+ if (poisonCount <= 0) {
super.attack(hero);
}
}🤖 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/PoisonSlime.java` around lines 16 - 26,
The issue is that when poisonCount equals 1, both the first if block
(poisonCount > 0) and the second if block (poisonCount <= 0) execute in the same
turn, causing super.attack(hero) to be called twice. The first block decrements
poisonCount to 0, which then satisfies the second condition. Fix this by
changing the second if statement to else if, so that only one attack path is
executed per method call. Specifically, replace the second if (poisonCount <= 0)
condition with else if (poisonCount <= 0) to ensure the basic attack only
executes when there is no poison attack in that turn.
Comment on lines
+30
to
+35
| void Count() { | ||
| for (int i = 0; i < 5; i++) { | ||
| poisonSlime.attack(hero); | ||
| } | ||
|
|
||
| assertEquals(0, poisonSlime.getPoisonCount()); |
There was a problem hiding this comment.
Count 테스트가 HP 회귀를 검출하지 못합니다.
현재는 poisonCount만 검증해서, 공격 로직이 잘못되어도 테스트가 통과할 수 있습니다. hero.getHp()도 함께 단정해 독 소진 전/후 피해 계약을 고정해 주세요.
🔧 제안 패치
`@Test`
`@DisplayName`("poisonCount가 0이 되어야 한다.")
void Count() {
for (int i = 0; i < 5; i++) {
poisonSlime.attack(hero);
}
assertEquals(0, poisonSlime.getPoisonCount());
+ assertEquals(40, hero.getHp()); // 5회 공격 후 기대 HP(독 5회 적용)
+
+ poisonSlime.attack(hero);
+ assertEquals(30, hero.getHp()); // 독 소진 이후에는 기본 공격(10)만 적용
}🤖 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/test/java/com/survivalcoding/PoisonSlimeTest.java` around lines 30 -
35, The Count test method only verifies that poisonSlime.getPoisonCount()
reaches zero after five attacks, but it does not validate that the hero's HP
actually decreased from those attacks. This means the attack logic could be
broken and the test would still pass. Capture the hero's initial HP before the
attack loop, then after the loop add an assertEquals assertion to verify that
hero.getHp() has been reduced by the expected poison damage amount, ensuring
both the poison count depletion and the actual damage dealt are properly
validated.
|
|
||
| - 메서드도 부모 클래스에 final 이 있다면 상속을 막을 수 있다. | ||
| - public final void slip() | ||
| - public void attack() { super.attack(); << 상속 받은 것. |
There was a problem hiding this comment.
불완전한 코드 예제를 수정하세요.
라인 33의 코드 조각이 미완성 상태입니다. 주석 표기 "<<" 가 있고 메서드 본문이 제대로 작성되지 않았습니다. 명확한 예제 코드로 교체하거나 설명 텍스트로 변경하는 것이 좋습니다.
💡 개선 제안
라인 33을 다음과 같이 개선할 수 있습니다:
- - public void attack() { super.attack(); << 상속 받은 것.
+ - 예: `super.attack();` 로 부모 클래스의 메서드를 호출할 수 있습니다.또는 완전한 예제:
`@Override`
void attack(Hero hero) {
super.attack(hero); // 부모 클래스의 attack 호출
// 추가 로직...
}🤖 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 `@TIL/sample/2026-06-16-상속.md` at line 33, The code example for the attack()
method at line 33 is incomplete and malformed with an improper comment notation
"<<" that breaks the code syntax. Replace this incomplete line with a complete,
well-formed code example that demonstrates method overriding with a proper
`@Override` annotation, a complete method signature including parameters if
needed, a call to the parent class method using super.attack(), and proper
closing braces. Ensure the example is syntactically correct and clearly
illustrates the inheritance concept being documented.
Comment on lines
+60
to
+68
| void heal() { | ||
| if (getMp() < 5) { | ||
| System.out.println("마나가 부족합니다."); | ||
| } else { | ||
| setMp(getMp() - 5); | ||
| setHp(getHp() + 25); | ||
| System.out.println("힐을 시전했습니다. " + getName() + " HP : " + getHp()); | ||
| } | ||
| } |
There was a problem hiding this comment.
TIL 실습 코드의 메서드 시그니처가 실제 구현과 불일치합니다. TIL 문서의 GreatWizard 예제는 Hero 파라미터 없이 작성되었으나, 실제 game/src/main/java/com/survivalcoding/GreatWizard.java와 테스트 케이스(game/src/test/java/com/survivalcoding/GreatWizardTest.java)에서는 모두 Hero 파라미터를 요구합니다. 이러한 불일치는 학습자가 작성한 코드와 실제 프로젝트 코드 사이에 큰 혼동을 초래할 수 있습니다.
TIL/sample/2026-06-16-상속.md#L60-L68:heal()메서드 시그니처를heal(Hero hero)로 수정하고, 메서드 본문에서hero.setHp(hero.getHp() + 25)로 변경하여 실제 구현을 반영합니다.TIL/sample/2026-06-16-상속.md#L70-L79:superHeal()메서드 시그니처를superHeal(Hero hero)로 수정하고,hero.setHp(hero.getMaxHp())로 수정하여 Hero의 HP를 최대값으로 복구하도록 합니다.
📍 Affects 1 file
TIL/sample/2026-06-16-상속.md#L60-L68(this comment)TIL/sample/2026-06-16-상속.md#L70-L79
🤖 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 `@TIL/sample/2026-06-16-상속.md` around lines 60 - 68, The TIL documentation has
method signatures that differ from the actual project implementation. At
TIL/sample/2026-06-16-상속.md lines 60-68, update the heal() method signature to
accept a Hero parameter as heal(Hero hero), and change the method body to call
hero.setHp(hero.getHp() + 25) instead of setHp(getHp() + 25). Similarly, at
TIL/sample/2026-06-16-상속.md lines 70-79, update the superHeal() method signature
to accept a Hero parameter as superHeal(Hero hero), and change the method body
to call hero.setHp(hero.getMaxHp()) to restore the Hero's HP to maximum. These
changes align the TIL documentation with the actual implementation in
game/src/main/java/com/survivalcoding/GreatWizard.java and its corresponding
test cases.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
260616_07_문재원
📝 과제 정보
📋 체크리스트
📷 실행 결과 (또는 테스트 결과)
❓질문 및 어려웠던 점
🔄 자체 평가 & 회고