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

[보스 몬스터 잡기] 최원준 미션 제출합니다 #3

Open
wants to merge 20 commits into
base: jhon3242
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
468ccbd
docs : 구현 목록 작성
jhon3242 Sep 21, 2023
1bbb293
feat(게임 초기화) : 보스 몬스터 HP 를 입력 받는 기능 구현
jhon3242 Sep 21, 2023
5544ab5
feat(게임 초기화) : 보스 몬스터 HP 입력 예외 사항 처리
jhon3242 Sep 21, 2023
bc8056a
feat(게임 초기화) : 플레이어 정보 입력 기능 구현
jhon3242 Sep 21, 2023
99e363c
feat(게임 초기화) : 플레이어 정보 예외 처리 기능 구현
jhon3242 Sep 21, 2023
40974a0
feat(게임 진행) : 레이드 시작 문구 출력
jhon3242 Sep 21, 2023
3937310
feat(게임 진행) : 보스 상태 정보 및 이미지 출력 기능 구현
jhon3242 Sep 21, 2023
d79e14f
feat(게임 진행) : 플레이어 정보를 출력한다.
jhon3242 Sep 21, 2023
2cd384a
feat(게임 진행) : 공격 방식 입력 기능 구현
jhon3242 Sep 21, 2023
8ff50db
feat(게임 진행) : 플레이어의 공격 기능 구현
jhon3242 Sep 21, 2023
d4b8a09
feat(게임 진행) : 보스의 공격 기능 구현
jhon3242 Sep 21, 2023
c249ed5
feat(게임 진행) : 공격 받으면 보스 이미지 변경 기능 구현
jhon3242 Sep 21, 2023
405f7a6
feat(게임 종료) : 레이드 종료 결과 출력 구현
jhon3242 Sep 21, 2023
147e2d5
refactor(게임 진행) : 마나 부족시 마법 공격을 안하기 때문에 MP 감소되지 않도록 수정
jhon3242 Sep 21, 2023
31a02c0
refactor(게임 진행) : Player 객체의 책임 분할을 위해 일급 컬렉션으로 리펙터링
jhon3242 Sep 21, 2023
436dab1
refactor(게임 진행) : 일급 컬렉션 리펙터링 결과 출력 확인
jhon3242 Sep 22, 2023
cd1b29e
refactor(게임 진행) : 책임에 따른 메서드이 이동 및 자바 컨벤션에 맞게 메서드 위치 수정
jhon3242 Sep 22, 2023
fe76eba
refactor(게임 진행) : 매직 변수 리펙터링
jhon3242 Sep 22, 2023
50e2c66
refactor : 패키지 수정
jhon3242 Sep 22, 2023
751146a
bug : 보스가 죽어도 게임이 끝나지 않는 버그 수정
jhon3242 Sep 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 52 additions & 0 deletions docs/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# 기능 구현 목록
## 게임 초기화
- [X] 보스 몬스터 HP 를 입력 받는다.
- [X] 보스 몬드터의 MP 는 HP 로 입력 받는 값이다.
- 예외 처리
- [X] 예외처리 : 빈 값을 입력한 경우
- [X] 예외처리 : 숫자가 아닌 값을 입력한 경우
- [X] 예외처리 : 100이상 300이하 값이 아닌 경워
- [X] 플레이어 이름을 입력 받는다.
- 예외 사항
- [X] 예외 처리 : 빈 값을 입력한 경우
- [X] 예외 처리 : 5자를 초과하는 값을 입력한 경우
- [X] 플레이어의 HP, MP 를 입력 받는다.
- [X] `,` 로 구분된 값을 입력 받는다.
- 예외 처리
- [X] 예외처리 : 빈 값을 입력한 경우
- [X] 예외처리 : 숫자가 아닌 값을 입력한 경우
- [X] 예외처리 : 0보다 작은 값을 입력한 경우
- [X] 예외처리 : 둘의 합이 200이 아닌 경우
- [X] 예외처리 : `,` 로 구분되지 않는 값이 입력된 경우

## 게임 진행
- [X] 보스 레이드 시작 문구 출력
- [ ] 보스가 죽거나 플레이어가 죽을 때 까지 진행 반복 진행
- [X] 보스 정보 출력
- [X] 보스의 HP, MP 정보를 출력한다.
- [X] 맨 처음 시작시 무표정의 보스 모습을 출력한다.
- [X] 이전에 공격을 한 경우 아파하는 보스 모습을 출력한다.
- [X] 플레이어 정보 출력
- [X] 공격 방식을 입력 받는다.
- [X] 1인 경우 물리 공격을 한다.
- [X] 보스 몬스터에게 10만큼 데미지를 준다.
- [X] MP 10을 회복한다.(최대치 이상은 회복하지 않는다)
- [X] 2인 경우 마법 공력을 한다.
- [X] 보스 몬스터에게 20만큼 데미지를 준다.
- [X] MP 30을 소모한다.
- 예외 처리
- [X] 예외처리 : 빈 값을 입력한 경우
- [X] 예외처리 : `1` , `2` 가 아닌 값을 입력한 경우
- [X] 보스 몬스터 채력이 0이 되면 게임이 종료된다.
- [X] 보스 몬스터가 플레이어를 공격한다.
- [X] 0~20 의 랜덤 데미지를 플레이어한테 입힌다.
- [X] 플레이어 HP 가 0이 되면 게임이 종료된다.

## 게임 종료
- 레이드의 성공한 경우
- [X] 몇 턴 만에 잡았는지 출력한다.
- 레이드의 실패한 경우
- [X] 웃고있는 모습과 함께 보스 정보를 출력한다.
- [X] 플레이어 정보를 출력한다.
- [X] `${플레이어} HP가 0이 되었습니다.` 문구를 출력한다.
- [X] 보스 레이드 실패 메시지를 출력한다.
25 changes: 25 additions & 0 deletions src/main/java/bossmonster/Converter.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package bossmonster;

import bossmonster.message.ExceptionMessage;

import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

public class Converter {
public static int stringToInt(String value) {
try {
return Integer.parseInt(value);
} catch (NumberFormatException e) {
throw new IllegalArgumentException(ExceptionMessage.NUMBER_FORMAT);
}
}

public static List<Integer> stringToSplitInt(String src, String delimiter) {
return Arrays.stream(src.split(delimiter))
.map(Converter::stringToInt)
.collect(Collectors.toList());
}


Comment on lines +23 to +24

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.

빈칸은 별 신경을 쓰지 않았는데 불필요한 공백을 없애는 방식으로 작성해야겠네요 감사합니다!

}
3 changes: 2 additions & 1 deletion src/main/java/bossmonster/Main.java
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

public class Main {
public static void main(String[] args) {
System.out.println("Hello world!");
MainController controller = new MainController();
controller.run();
}
}
94 changes: 94 additions & 0 deletions src/main/java/bossmonster/MainController.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,94 @@
package bossmonster;

import bossmonster.domain.player.AttackType;
import bossmonster.domain.boss.Boss;
import bossmonster.domain.GameService;
import bossmonster.domain.player.Player;
import bossmonster.view.InputView;
import bossmonster.view.OutputView;

import java.util.List;

import static bossmonster.domain.GameOption.DELIMITER;

public class MainController {
public static final GameService service = new GameService();

Choose a reason for hiding this comment

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

public static으로 설정하신 이유가 있으실까요?
이러한 구조를 가진다면 어떤 문제점이 생길 수 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

stateless 하게 서비스를 사용하고 싶어서 static 을 사용했는데 지금보니까 public 일 필요는 없다고 느껴지네요 오히려 외부에서 컨트롤러를 통해 서비스에 접근할 수 있어 보안성에도 안좋아 질 것 같습니다!
또한 이러한 구조를 가진다면 의존성에도 문제가 생길 것 같네요 생성자를 통해서 주입 받는 방식이 더 확장 가능한 설계가 되지 않을까 싶네요 ㅎㅎ


public void run() {
Boss boss = initBoss();
Player player = initPlayer();
proceedRade(boss, player);
printResult(boss, player);
}

private Boss initBoss() {
try {
int bossHp = Converter.stringToInt(InputView.readBossHp());

Choose a reason for hiding this comment

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

이곳에서 converter를 사용하신 이유가 따로 있으신가요?
InputView에 readInt()등의 메서드를 만들어 주는 것은 어떻게 생각하시나요?

추가로 InputView의 메서드 네이밍을 readBossMp로 짓는 순간부터 InputView는 논리적으로 비즈니스 로직과 의존성을 갖게 된다고 생각합니다.
(보스의 MP를 입력받는다는 정보를 알고 있는 상황이 되니까요)

이런 상황에서 어떠한 문제점이 있을 수 있고, 또 그에 반해 어떠한 이점을 얻을 수 있는 구조인지 원준님의 생각이 궁금합니다!

Copy link
Author

Choose a reason for hiding this comment

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

InputView에서 readInt 등을 사용하면 '값을 읽어 들임' 책임에 더해 '값을 변환함' 이라는 책임이 추가되어 단일 책임 원칙에 위배된다고 판단했어서 위와 같이 컨트롤러에서 converter 를 사용해서 변환해주었습니다! 하지만 이 부분을 작성하면 숫자를 입력 받는 작업이 자주 일어나서 매번 변환해주는 것이 가독성를 해친다는 느낌도 많이 받았어요 말랑님은 어떻게 생각하시는지 궁금합니다!

메서드 네이밍을 통해서도 의존성을 가질 수도 있다는 것을 처음 깨달게 되었네요 감사합니다! 이런 식으로 네이밍을 하면 비즈니스 로직이 변경될 때 메서드명도 변경되어야 하는 경우가 발생할 수도 있겠네요! 이를 해결하기 위해서는 어떤 방식이 있을지 궁금합니다! 처음 드는 생각은 readInt(Message.READ_BOSS)HP) 등 처럼 readInt, readString 과 같은 일관된 메서드로 출력할 메시지를 건내주는 방식인데 이런식으로 사용하면 readBossHp() 보다는 가독성이 떨어지지 않을까요??

Copy link

@shin-mallang shin-mallang Sep 23, 2023

Choose a reason for hiding this comment

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

InputView에서 readInt 등을 사용하면 '값을 읽어 들임' 책임에 더해 '값을 변환함' 이라는 책임이 추가되어 단일 책임 원칙에 위배된다고 판단했어서 위와 같이 컨트롤러에서 converter 를 사용해서 변환해주었습니다! 하지만 이 부분을 작성하면 숫자를 입력 받는 작업이 자주 일어나서 매번 변환해주는 것이 가독성를 해친다는 느낌도 많이 받았어요 말랑님은 어떻게 생각하시는지 궁금합니다!

정수값, 문자열 등 여러 값을 읽어드림도 값을 읽어 드림 이라는 책임을 가질 수 있을 것 같아요.
또한 만일 원준님 말씀대로 readInt를 사용하는 것이 InputView가 단일 책임 원칙을 위배한다고 가정해 볼게요.
그랬을 때 발생하는 문제점이 무엇이 있다고 생각하셨나요?
단일 책임 원칙을 무엇을 위해 지켜야 하는지 고민해보시면 좋을 것 같습니다!

무엇보다 현재와 같은 구조라면 정수가 필요할 때 마다, read를 통해 문자열을 입력받아서 정수로 바꾸어주는 작업을 여러 곳에서 진행하게 되면 중복 코드도 많이 발생할 것 같습니다.

결론적으로 저라면 readInt를 두고 사용할 것 같아요!
(편하니까요)

return new Boss(bossHp);
} catch (IllegalArgumentException e) {
OutputView.printError(e);
return initBoss();
}
}

private Player initPlayer() {
Player player = new Player();
initPlayerName(player);
initPlayerHPAndMP(player);
Comment on lines +35 to +37

Choose a reason for hiding this comment

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

Boss는 생성자로 인자를 넣어주는 반면, 플레이어의 경우에는 setter를 통해 세팅해주는 것으로 보이는데, 두 구조가 통일성이 없다고 생각하시지 않으시나요?

이러한 구조룰 어떤 식으로 개선할 수 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

이 부분이 가장 고민했던 부분 중 하나입니다! 플레이어 또한 생성자 인자를 통해 생성해주고 싶었는데 new Player(name, hp, mp) 와 같이 로직을 작성하면 생성자를 실행할 때 입력 값의 검증이 시작되어서 hp/mp 부분에서 예외가 발생하면 name 부터 다시 입력받아야 되는 문제가 발생했었습니다. 그래서 어쩔 수 없이 각각 setter 를 통해 따로 입력 받고 검증하는 방식으로 로직을 작성하게 되었어요 ㅠㅠ 그런데 지금 생각해보니까 Name 이라는 별도의 객체를 만들어서 검증을 처리해주면 해결할 수 도 있다는 생각이 드네요?

Name name = readName()
Stats stats = readStats()
Player player = new Player(name, stats)

위와 같은 방식으로 로직을 작성하면 각자의 생성자에서 예외 검증을 통해 해결 할 수 있지 않을까 싶네요 ㅎㅎ

Copy link

@shin-mallang shin-mallang Sep 23, 2023

Choose a reason for hiding this comment

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

맞습니다 ㅎㅎ
그런데 네이밍을 추가로 언급드리면,

  1. 이름은 플레이어만 입력받으며, 플레이어의 이름에 대해서만 비즈니스 규칙이 존재합니다.
  2. Stats은 보스와 플레이어의 비즈니스 규칙이 각각 다릅니다.

따라서 Name, Stats 보다는 PlayerName과 PlayerStats이 도메인을 더 잘 표현할 수 있는 네이밍이라 생각합니다 ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

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

아하 그렇겠네요 놓치고 있었던 부분을 알게되었네요 감사합니다!!

return player;
}

private void initPlayerName(Player player) {
try {
player.setName(InputView.readPlayerName());
} catch (IllegalArgumentException e) {
OutputView.printError(e);
initPlayerName(player);
}
}

private void initPlayerHPAndMP(Player player) {
try {
String playerStatsString = InputView.readPlayerInfo();
List<Integer> playerStats = Converter.stringToSplitInt(playerStatsString, DELIMITER);
player.setStats(playerStats);
} catch (IllegalArgumentException e) {
OutputView.printError(e);
initPlayerHPAndMP(player);
}
}

private void proceedRade(Boss boss, Player player) {
OutputView.printRadeStart();
while (boss.isAlive() && player.isAlive()) {
Copy link

@shin-mallang shin-mallang Sep 22, 2023

Choose a reason for hiding this comment

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

해당 조건은 게임이 지속되는 것을 정의하는 비즈니스 규칙을 포함하는 것 같은데요,
이렇게 비즈니스 규칙을 컨트롤러에서 관리한다면 Service는 어떠한 역할을 갖게 되는 것인지 궁금합니다!

Copy link
Author

Choose a reason for hiding this comment

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

이것도 고민했던 것 중 하나였는데 비즈니스 로직은 최대한 서비스에 두고 싶었으나 비즈니스 규칙을 검증하고 다시 입력 받는 작업을 위해서는 컨트롤러에 작성할 수 밖에 없다고 판단했습니다ㅠㅠ 그래서 뷰를 사용할 필요가 없는 비즈니스 로직은 Service 에 작성하고 그 외의 경우에는 컨트롤러에 남게 되었어요. proceedRade 도 비즈니스 로직이 포함되어 있지만 중간 중간에 출력을 해줘야 하는 부분이 있어서 컨트롤러에 남게 되었습니다. 이런 경우를 해결하기 위해 어떤 방법이 있을까요?? 제 나름대로 고민했지만 위와 같이 작성하는게 최선이라고 판단했습니다ㅠㅠ

Choose a reason for hiding this comment

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

여기에는 많은 방법들이 있을 수 있을 수 있겠지만, 다 설명드리긴 어려워서 몇가지만 말씀드려볼게요!

  1. Service에 게임을 계속 진행할 수 있는가?를 나타내는 메서드를 만들 수 있을 것 같아요.
  2. Boss와 Player를 묶은 어떠한 객체를 만들어서, 해당 객체에 게임을 계속 진행할 수 있는가? 를 표현하도록 할 수도 있을 것 같아요.
  3. 지금 상태도 괜찮다고 생각해요!

3번에 대해 조금 더 말씀을 드리면, 서비스 자체가 현재 당장 필요한 것인지도 한번 고민해 보시면 좋겠어요!
(서비스의 역할이 무엇인지에 대해 정해진 답은 없지만, 이를 바라보는 여러 시각이 존재하니 같이 찾아보셔도 좋을 것 같아요!
이렇다할 정답은 없지만요!)

이것도 고민했던 것 중 하나였는데 비즈니스 로직은 최대한 서비스에 두고 싶었으나

비즈니스 규칙을 서비스에 두어야 할 필요가 있을까요?
서비스 말고도 이러한 것을들 잘 표현할 수 있는 객체를 만들어 보는 건 어떨까요?

조금 더 예를 들어 볼게요.
만약 플레이어를 만드는 로직을 서비스에 작성한다고 가정하면

public Player createPlayer(String name) {
    if (name.length < 0 || name.length > 5) {
         // 예외
    } 
    return new Player(name);
}

위와 같이 비즈니스 규칙에 대한 검증을 서비스에서 처리하는 방법이 있을 수 있겠죠?

그런데 다음과 같이도 만들 수 있어요!

public Player createPlayer(String name) {
    return new Player(name);
}

class Player {
     public Player(String name) {
          validate(name);
          this.name = name;
     }
     
    private void validate(String name) {
        if (name.length < 0 || name.length > 5) {
             // 예외
        } 
    }
}

이렇듯 반드시 서비스에 비즈니스 규칙을 두어야 할 필요는 없으니, 여러 방법들과 각 방법들의 장단점에 대해 학습해 보시고, 현재 구조에서 어떤 방법이 가장 좋은 방법일지를 고민해 보신 뒤 적용해보시면 좋겠습니다 ㅎㅎ

Copy link
Author

Choose a reason for hiding this comment

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

시야가 조금 확장된 것 같네요 감사합니다!!

OutputView.printRadeInfo(boss, player);
initAttackType(player);

Choose a reason for hiding this comment

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

(개인적인 의견이나) init은 최초 1회의 의미를 가진다고 생각합니다.
그러나 해당 경우에는 반복문 내에서 계속해서 호출되므로 init이라는 네이밍이 적절하지 않은 것 같은 생각이 들어요!
어떻게 생각하시나요?

Copy link
Author

Choose a reason for hiding this comment

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

아하 그럴 수 있겠네요 좋은 관점을 제시해주셔서 감사합니다!

int playerToBossDamage = service.attackBoss(boss, player);
Copy link

@shin-mallang shin-mallang Sep 22, 2023

Choose a reason for hiding this comment

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

이곳에서는 Service를 사용하였는데요, Service의 책임과 역할을 어떻게 정의하고 사용하고 계신것인지 궁금합니다.

Copy link
Author

Choose a reason for hiding this comment

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

책임과 역할에 집중에서 설계하기 보단 단순히 "뷰를 사용하지 않는 비즈니스 로직은 서비스에 둬야겠다" 라는 생각으로 작성하다 보니 서비스가 어떤 책임을 가지고 어떤 역할을 하는지 명확하게 설계하지 못했네요 다음 부터는 책임과 역할에 집중해 봐야겠습니다!

Choose a reason for hiding this comment

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

위에서 언급드린 것 처럼, 서비스에 비즈니스 로직을 둘 수 있고, 비즈니스 규칙을 포함한 객체를 정의하는 것도 한가지 방법이 될 수 있습니다!
여러 방법에 대해 학습하고 고민해보시면 좋을 것 같아요!

int bossToPlayerDamage = service.attackPlayer(boss, player);
OutputView.printPlayerAttackResult(playerToBossDamage, player);
if (boss.isAlive()) {
OutputView.printBossAttackResult(bossToPlayerDamage);
}
}
}

private void initAttackType(Player player) {
try {
int attackNumber = Converter.stringToInt(InputView.readAttackType());
player.setAttackType(AttackType.findByNumber(attackNumber));
} catch (IllegalArgumentException e) {
OutputView.printError(e);
initAttackType(player);
}
}

private void printResult(Boss boss, Player player) {
if (player.isAlive()) {
OutputView.printRadeWin(player);
return;
}
OutputView.printRadeInfo(boss, player);
OutputView.printRadeLoss(player);
}

}
21 changes: 21 additions & 0 deletions src/main/java/bossmonster/domain/GameOption.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
package bossmonster.domain;

public class GameOption {

Choose a reason for hiding this comment

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

이렇게 한 클래스 내에서 여러 상수들을 관리한다면 어떠한 장단점이 있을까요?
또한 Enum을 사용하는 것과 public static final을 사용하는 것에는 어떠한 차이점과 장단점이 있을까요?

Copy link
Author

Choose a reason for hiding this comment

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

장점으로는 한 곳에서 옵션을 수정할 수 있어서 유지보수에 편리하지 않을까요?? 단점으로는 코드를 읽다가 해당 변수 값이 뭔지 탐색하는 과정이 추가로 필요해서 가독성의 저하되는 요소가 될 수도 있지 않을까 싶습니다.

Enum 과 다르게 public static final 를 사용하면 다른 타입의 변수들도 한 객체에서 설정할 수 있었어요! GameOption 에서 볼 수 있듯이 intString 처럼 다른 타입도 하나의 객체 안에서 설정할 수 있어서 관리하기가 편하더라고요 반면에 Enum 을 사용하면 서로 관련성이 있는 상수들을 한 곳에서 관리해줄 수 있었어요! 하지만 타입이 서로 다른 상수를을 관리하기는 불편하더라고요 그래서 Message 와 같이 String 으로 통일되는 상수들을 Enum 으로 관리해주었답니다! 하지만 Enum 나름대로 장점이 있는데 안전한 타입 비교가 가능하고 빠른 비교가 가능하다는 거죠! 그런데 이런 장점이 Message 를 다룰 때 활용하기엔 무리가 있다고 생각이 드네요 메시지 또한 public static final 로 관리하는 편이 더 낫지 않을까 생각이 듭니다!

Choose a reason for hiding this comment

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

장점으로는 한 곳에서 옵션을 수정할 수 있어서 유지보수에 편리하지 않을까요??

데이터의 수가 적은 경우에는 그럴 수 있을 것 같아요~
그런데 이곳에 존재하는 상수가 100개, 1000개, ...가 된다면 그 상황에서도 필요한 옵션만 수정하는 데 어려움이 없을까요?

단점으로는 코드를 읽다가 해당 변수 값이 뭔지 탐색하는 과정이 추가로 필요해서 가독성의 저하되는 요소가 될 수도 있지 않을까 싶습니다.

저도 이 부분에 동의합니다 :)

public static final int BOSS_HP_MAX_INCLUSIVE = 300;
public static final int BOSS_HP_MIN_INCLUSIVE = 100;
public static final int BOSS_DAMAGE_MAX_EXCLUSIVE = 21;

public static final int PLAYER_NAME_MAX_INCLUSIVE = 5;
public static final int PLAYER_SUM_VALUE = 200;
public static final int PLAYER_STATS_SIZE = 2;
public static final int PLAYER_MAGIC_ATTACK_MP_COST = 30;
public static final int PLAYER_MP_RECOVER = 10;

public static final String DELIMITER = ",";
public static final int HP_INDEX = 0;
public static final int MP_INDEX = 1;
public static final int MIN_HP = 0;
public static final int CURRENT_POINT_IDX = 0;
public static final int START_POINT_IDX = 1;
public static final int INITIAL_ATTACK_COUNT = 0;
}
24 changes: 24 additions & 0 deletions src/main/java/bossmonster/domain/GameService.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package bossmonster.domain;

import bossmonster.domain.boss.Boss;
import bossmonster.domain.boss.BossStatus;
import bossmonster.domain.player.Player;

public class GameService {

public int attackBoss(Boss boss, Player player) {
int attackDamage = player.attackBoss();
boss.attacked(attackDamage);
Comment on lines +10 to +11

Choose a reason for hiding this comment

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

해당 부분을 나누어 호출하는 이유가 있을까요?
player.attack(boss);는 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

그러네요 이제 보니까 굳이 나눌 필요가 있을까 의문이 듭니다! 말씀해주신 방법으로 작성하는게 가독성도 좋고 확장 가능성도 더 좋을 것 같네요!

return attackDamage;
}

public int attackPlayer(Boss boss, Player player) {
if (!boss.isAlive()) return 0;

Choose a reason for hiding this comment

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

보스가 죽은 경우 해당 메서드를 호출하면 예외가 아닌 0을 반환하는 이유가 있으실까요?
죽은 보스에게 플레이어를 공격하라는 것은 예외 상황이라고 생각합니더

Copy link
Author

Choose a reason for hiding this comment

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

예외를 발생시키는 것도 고민해보긴 했는데 그렇게되면 proceedRade 에서 try-catch 로 에외를 잡는 로직이 추가되야 하고 프로그램이 계속 진행되어야 한다고 판단했는데 생각해보니까 어차피 레이드가 종료가 되어야 하는 상황이니까 굳이 0 을 리턴하기 보단 예외를 발생시키고 레이드 종료 로직으로 가는게 맞겠네요!

Choose a reason for hiding this comment

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

어차피 레이드가 종료가 되어야 하는 상황이니까 굳이 0 을 리턴하기 보단 예외를 발생시키고 레이드 종료 로직으로 가는게 맞겠네요!

👍👍
추가로 한 가지 말씀드릴게요~

예외를 발생시키는 것도 고민해보긴 했는데 그렇게되면 proceedRade 에서 try-catch 로 에외를 잡는 로직이 추가되야 하고 프로그램이 계속 진행되어야 한다고 판단

Service가 Controller를 고려해서 로직을 바꾸게 된 것이네요~
그런데 Service가 Controller를 고려하는 것이 좋을지 한번 고민해 보시면 좋을 것 같아요~

int attackDamage = boss.attackPlayer();
player.attacked(attackDamage);
Comment on lines +17 to +18

Choose a reason for hiding this comment

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

마찬가지로 boss.attackPlayer(player);는 어떨까요?
혹은 player.attackedBy(boss)도 괜찮을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

이 부분에 대해서 공감합니다! 다음번에는 말씀해주신 방식대로 작성해봐야겠네요

if (!player.isAlive()) {
boss.handleBossStatus(false, true);

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.

isDead() 와 같은 메서드를 만드는 것이 가독성에 더 좋겠네요 감사합니다!

}
return attackDamage;
}
}
48 changes: 48 additions & 0 deletions src/main/java/bossmonster/domain/Point.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
package bossmonster.domain;

import bossmonster.message.ExceptionMessage;

public class Point {
private int startAmount;
private int curAmount;

Choose a reason for hiding this comment

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

currentAmount가 가독성이 더 좋을 것 같아요!

Copy link
Author

Choose a reason for hiding this comment

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

줄여쓰지 말자고 생각했는데 저도 모르게 사용하고 있었네요 감사합니다!


public Point(int amount) {
isNegative(amount);
this.startAmount = amount;
this.curAmount = amount;
}

private void isNegative(int amount) {
if (amount < 0) {
throw new IllegalArgumentException(ExceptionMessage.PLAYER_STATS_NEGATIVE);
}
}
Comment on lines +15 to +19

Choose a reason for hiding this comment

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

해당 메서드의 네이밍은 is...으로써 불린을 반환해야 할 것 같은데요, 그러나 실제로는 예외를 터뜨리고 있습니다.
이런 경우라면 is대신 validate라는 이름이 더 어울리는 것 같은데 어떻게 생각하시나요!

Copy link
Author

Choose a reason for hiding this comment

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

아 그렇네요 is 보다는 validate 가 더 맞을 것 같습니다!


public boolean isMoreTHen(int amount) {
return curAmount > amount;
}

public boolean isEqualOrMoreThen(int amount) {
return curAmount >= amount;
}

public boolean isLowerThen(int amount) {
return this.curAmount < amount;
}

public void reduceAmount(int amount) {
this.curAmount = Math.max(0, this.curAmount - amount);
}

public void addAmount(int amount) {
this.curAmount = Math.min(this.startAmount, this.curAmount + amount);
}

public int getStartAmount() {
return startAmount;
}

public int getCurAmount() {
return curAmount;
}
}
75 changes: 75 additions & 0 deletions src/main/java/bossmonster/domain/boss/Boss.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,75 @@
package bossmonster.domain.boss;

import bossmonster.message.ExceptionMessage;
import bossmonster.domain.GameOption;
import bossmonster.domain.Point;

import java.util.ArrayList;
import java.util.List;

import static bossmonster.domain.GameOption.BOSS_DAMAGE_MAX_EXCLUSIVE;

public class Boss {
private String name;

Choose a reason for hiding this comment

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

BOSS로 고정이니 Static final 형태로 잡아주는 것은 어떨까요?

Copy link
Author

Choose a reason for hiding this comment

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

확실히 변경 가능성이 없으니 그게 맞겠네요!

private Point hp;
private BossStatus status;

public Boss(int hp) {
validate(hp);
this.name = "BOSS";
this.hp = new Point(hp);
this.status = BossStatus.NORMAL;
}

private void validate(int hp) {
isLegalHpRange(hp);

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 void isLegalHpRange(int hp) {
if (hp < GameOption.BOSS_HP_MIN_INCLUSIVE ||
hp > GameOption.BOSS_HP_MAX_INCLUSIVE) {
throw new IllegalArgumentException(ExceptionMessage.BOSS_HP_RANGE);
}
}
Comment on lines +28 to +33

Choose a reason for hiding this comment

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

마찬가지로 is보다는 validate라는 이름이 더 어울리는 것 같습니다!

Copy link
Author

Choose a reason for hiding this comment

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

공감합니다! 감사합니다!


public boolean isAlive() {
return hp.isMoreTHen(GameOption.MIN_HP);
}

public void attacked(int damage) {
handleBossStatus(damage > 0, false);

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.

damage > 0 을 별도의 isAttacked(damage) 등으로 메서드를 분리할 수 있겠네요!

hp.reduceAmount(damage);
}

public void handleBossStatus(boolean isAttacked, boolean isVictory) {
if (isVictory) {
status = BossStatus.VICTORY;
return;
}
if (isAttacked) {
status = BossStatus.ATTACKED;
return;
}
status = BossStatus.NORMAL;
}

public int attackPlayer() {
return getAttackDamage();
}

private int getAttackDamage() {
return (int) (Math.random() * BOSS_DAMAGE_MAX_EXCLUSIVE);
}

public String getName() {
return name;
}

public List<Integer> getHp() {
return new ArrayList<>(List.of(hp.getCurAmount(), hp.getStartAmount()));
}

public String getStatusImg() {
return status.toString();
}
}
32 changes: 32 additions & 0 deletions src/main/java/bossmonster/domain/boss/BossStatus.java
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
package bossmonster.domain.boss;

public enum BossStatus {
NORMAL(0, " ^-^\n" +
" / 0 0 \\\n" +
"( \" )\n" +
" \\ - /\n" +
" - ^ -"),
ATTACKED(1, " ^-^\n" +
" / x x \\\n" +
"( \"\\ )\n" +
" \\ ^ /\n" +
" - ^ -"),
VICTORY(2, " ^-^\n" +
" / ^ ^ \\\n" +
"( \" )\n" +
" \\ 3 /\n" +
" - ^ -");

private final int status;
private final String img;

BossStatus(int status, String img) {
this.status = status;
this.img = img;
}

@Override
public String toString() {
return this.img;
}
}
Loading