Skip to content

Conversation

@lamodadite
Copy link

java lv0 잼민펀치 기능 구현 후 테스트 완료했습니다. 확인 부탁드립니다!

@lamodadite lamodadite requested review from chyo1 and saewoo1 December 26, 2024 12:02
@lamodadite lamodadite self-assigned this Dec 26, 2024
Copy link
Contributor

@saewoo1 saewoo1 left a comment

Choose a reason for hiding this comment

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

캬 너무너무 잘하셨습니다!
출력부 분리, 매직넘버 관련해서 조금만 더 고치면 더 멋진 코드가 될 것 같아요 고생 많으셨습니다!!

Copy link
Contributor

Choose a reason for hiding this comment

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

하나의 클래스는 머로 구성될까여??
간단히 생각하면 변수 + 메서드라고 생각하는데, 쪠이팍 클래스가 좀 쓸쓸해보여요
쪠이팍 클래스는 어디까지 책임져야 하는것인가..

Copy link
Contributor

Choose a reason for hiding this comment

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

매직넘버를 쓰는 습관을 들여봅시다!!

  1. 뉴비가 코드를 봤을 때, 80, 121이 먼의미지? 하는 의문이 들 수 있음
  2. 지금은 여기서 한번밖에 안쓰는데, 같은 숫자가 클래스 내에 100개가 있게 될 경우, 이제부턴 80이 아니라 79로 바꿔주세요 하면 100줄의 코드를 모두 79로 바꿔야하는 노가다쑈 시작(단축키가 있지만 그래도..)

80, 121, 0, 21, 10, 31... 수가 너무 많은데 매직넘버로 다 어케바꿔요? 라는 생각이 든다면
범위 설정을 잘못했나.. 스스로 의심을 해보기!

Copy link
Author

@lamodadite lamodadite Jan 11, 2025

Choose a reason for hiding this comment

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

매직넘버를 상수로 바꾸니 가독성이 훨씬 좋아졌습니다!!

Copy link
Contributor

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.

출력부를 분리할 생각을 왜 못했을까요.. 확실히 저 부분은 생성자에서 할 일이 아닌것 같네요!

Copy link
Contributor

Choose a reason for hiding this comment

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

angerLimit이라는 멤버 변수를 대욱이 갖고 있으니, 멤버 변수에 대한 임계점 판별 메서드를 스스로 판단하도록 하는 책임 분리 좋아여~!

Copy link
Contributor

Choose a reason for hiding this comment

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

탈출 조건을 잘 명시해뒀으니, 무한루프 대신 while(daewoole.isOverAngerLimit())
아예 이렇게 걸어두면 좀 더 가독성이 좋을 것 같슴다!

Copy link
Author

Choose a reason for hiding this comment

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

수정햇습니다! 가독성 업!

Copy link
Contributor

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.

출력 부분 Util 클래스로 분리했습니다! 확실히 책임분리를 하니 보기도 좋고 확장성도 좋은것 같습니다!

Copy link

@chyo1 chyo1 left a comment

Choose a reason for hiding this comment

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

수고하셨습니다~

Comment on lines +14 to +18
Copy link

Choose a reason for hiding this comment

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

MENT가 바뀌지 않는다면, 생성자가 아닌 provocations에 List.of()로 직접 값을 불변 객체로 넣어도 될 것 같아요

Comment on lines +14 to +16
Copy link

Choose a reason for hiding this comment

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

Suggested change
System.out.printf("지원은 '%s'를 시전하여 대욱의 분노를 %d 증가시켰다.\n현재 대욱의 분노 수치 : %d\n\n", ment,
anger,
daewooleAngerLevel);
System.out.printf(
"지원은 '%s'를 시전하여 대욱의 분노를 %d 증가시켰다.\n현재 대욱의 분노 수치 : %d\n\n",
ment,
anger,
daewooleAngerLevel
);

하우 어바웃 디스

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.

4 participants