[1단계 - 사다리 생성] 미아(이종미) 미션 제출합니다. #279

jongmee · 2024-02-22T07:15:56Z

안녕하세요 스티치! 반갑습니다 😊

사다리 타기 미션을 위해 생성한 사다리 관련 도메인들은 다음과 같습니다.

// domain 패키지
├── ladder
│   ├── Height.java
│   ├── Ladder.java
│   ├── LadderStep.java
│   └──  Path.java
├── generator
│   ├── LadderStepGenerator.java
│   └──RandomLadderStepGenerator.java
└── participant
    ├── Participant.java
    └──Participants.java

피드백을 받고 싶은 부분은 아래와 같습니다.

TDD를 적용하면서 커밋 역시 TDD cycle대로 진행하였습니다. TDD를 처음 해보아서 잘 적용한 것인지 피드백 받고 싶습니다!
요구사항에 '모든 원시값과 문자열을 포장한다'라는 항목이 있었습니다. 현재 Participant(참가자 도메인 클래스)에 String name 변수가 존재해서 Participant에서 name에 대한 검증을 진행하고 있습니다. 이 역시 도메인 클래스(Name)로 분리해야 좋은 코드라고 생각하시나요?

lxxjn0

안녕하세요 미아. 이번에 피드백을 맡게 된 스티치입니다.

도메인 객체를 잘 분리해서 코드를 작성해주신 것 같아요 👍🏼 전반적으로 어렵지 않은 미션이여서 간단하게 피드백 남겨두었는데 참고해서 반영 부탁드릴게요.

추가로 코드 전체적으로 final 키워드를 많이 사용하신 것 같은데 적절한 상황과 의도를 가지고 사용해보는 것은 어떨까요? final을 많이 사용하면 오히려 가독성을 해치는 경우도 있는 것 같아서요! (과거의 저도 그랬던 것 같아서 의견 전달드려봅니다)

그리고 남겨주신 질문에 대한 답변입니다.

TDD를 잘 하고 있는 것인지

TDD가 개발 과정에서 드러나다보니 제가 파악하기가 어려운 것 같습니다. 커밋의 경우 동작하는 코드 단위가 되어야 한다고 생각하기에 더더욱 TDD 프로세스를 드러내기 어려운 것 같습니다. 하지만 미아가 배운 과정에 맞게 요구사항에 대한 테스트를 작성하고 그에 맞는 프로덕션 코드를 작성한다면 올바른 과정이 맞지 않을까 생각합니다.

원시값에 대한 포장

이건 정답이 없는 부분이라고 생각합니다. 어떤 개발자가 생각하기엔 간단한 유효성 검증이 굳이 객체로 분리할 이유가 없다고 생각할 수도 있고 어떤 개발자는 유효성 검증 하나만으로도 충분히 의미를 가진다고 생각할 수도 있기 때문입니다. 이는 클로버가 더 적절하다고 생각하는 방향으로 결정하셔도 문제가 없을 것 같아요 :)

README.md

src/main/java/ladder/controller/LadderGame.java

lxxjn0 · 2024-02-23T02:47:10Z

src/main/java/ladder/exception/generator/ContinuousPathException.java

+public class ContinuousPathException extends RuntimeException {
+    private static final String MESSAGE = "연속된 사다리 발판이 존재합니다.";
+
+    public ContinuousPathException() {
+        super(MESSAGE);
+    }
+}


[질문] 예외 클래스를 메시지 단위로 정의하신 것 같아요. 미아는 어떠한 기준으로 예외 클래스를 구현하였을까요? 일반적인 예외 클래스의 사용과 커스텀 예외 클래스의 사용이 가지는 트레이드 오프는 어떤 점이 있을까요?

프로그램 규모가 작고 예외가 많지 않아 메시지 단위로 정의하였습니다. 약간의 오버헤드와 새로운 예외가 발생할 때마다 클래스를 생성해야 하는 단점이 있을 것 같습니다. 장점은 예외 클래스를 아예 정의해주는 것이 스트링으로 예외 메세지를 적어주는 것보다 가독성이 좋고(깔끔함) 테스트할 때도 해당 예외 클래스의 인스턴스인지 확인하면 돼서 편하다는 것입니다. 스티치는 메시지 단위 예외 클래스의 단점이 더 크다고 생각하시나요 ?!

커스텀 예외를 정의하는 것이 메시지를 적어주는 것보다 가독성 측면에서 더 장점이 있는지는 저도 명확히는 모르겠습니다. 커스텀 예외를 정의하면서 메시지도 드러나게 작성이 가능하니깐요.
저 같은 경우는 커스텀 예외를 예외 로직을 처리하기 위한 목적에 더 가까운 것 같아요. 만약 A라는 예외가 발생할 때 특정한 로직을 수행해야 한다면 그런 경우는 커스텀 예외를 작성해서 처리할 수 있도록 사용합니다.

아래 글을 한번 읽어보시면서 커스텀 예외에 대한 생각을 고민해보면 좋을 것 같아요!!

https://tecoble.techcourse.co.kr/post/2020-08-17-custom-exception/

현재 프로그램에서는 커스텀 예외를 활용했다 말할 수 있는 로직이 없네요..! 불필요한 것 같아 IllegalArgumentException이나 RuntimeException으로 대체하였습니다 감사합니다!

lxxjn0 · 2024-02-23T02:49:21Z

src/main/java/ladder/domain/participant/Participants.java

+    }
+
+    private void validateDuplicatedNames(final List<String> names) {
+        final Set<String> uniqueNames = Set.copyOf(names);


[질문] Set의 of 메서드가 아닌 copyOf 메서드를 사용하였는데 특별한 이유가 있을까요?

of 를 사용하려면 가변인자를 매개변수로 넘겨주어야 하는데, List를 바로 Set으로 복사시켜주는 copyOf가 더 적절하다고 판단했습니다 !

copyOf는 결국 새로운 영역에 동일한 데이터를 생성해서 사용하기에 메모리 소비가 발생할 수 밖에 없습니다. 개발자가 사용하는 상황 내에서 데이터 변조의 위험성이 없는 경우에는 굳이 copyOf를 사용해서 얻는 장점이 있는지 모르겠어요.

외부에 제공되는 데이터가 아닌 특정 메서드 내 로컬 변수에서 사용이라면 이런 부분도 고려해보면 좋을 것 같아요

메서드 시그니처에 집중하다 보니 불필요한 메모리 소비를 고려하지 못했네요. step2에서 수정해보도록 하겠습니다 !

lxxjn0 · 2024-02-23T02:53:47Z

src/main/java/ladder/domain/ladder/Path.java

@@ -0,0 +1,27 @@
+package ladder.domain.ladder;
+
+public enum Path {


해당 클래스는 출력에 해당하는 값들을 관리하는 것으로 보입니다. 도메인 객체로 볼 수 있을까요?

Path 객체 자체는 도메인 객체가 맞다고 생각합니다. LadderStep에서 하나의 사다리 발판을 나타내기 위해 도입한 enum이기 때문입니다. 단순히 boolean 값으로 표시하는 것보다 가독성과 유지보수성을 고려하면 더 좋을 것이라 판단했었습니다..! 이에 따른 로직도 Path 객체의 메서드로 관리되고 있습니다.

다만, 말씀주신 것처럼 Path의 속성이 출력에만 쓰이고 있습니다. 저는 출력을 넘어서는 도메인으로서의 의미가 있다고 생각했는데, 이렇게 활용하고 싶다면 출력에 쓰이는 shape 속성을 OutputView에서 상수로 관리해야 하는게 맞다고 생각하시나요? 또한 저는 shape로 enum에 대한 이해를 높일 수 있을 거라 생각했습니다.

도메인은 말 그래도 비즈니스의 관심의 영역입니다. 지금의 Path는 비즈니스 상 의미가 있는 값은 맞지만 로직 내에서 비즈니스를 가지는 객체냐고 생각하면 그렇지 않은 것 같아요. 객체가 사용되는 영역과 상황에 맞게 패키지를 관리하는게 적절하지 않을까 생각합니다.

지금은 해당 객체가 결국 view 패키지에서 사용되기에 객체의 위치가 적절하지 않은 것 같아요.

미아가 말한 것처럼 오히려 뷰에서 상수로 관리하는 것이 더 적절할 것 같아요

객체의 영역에 대해서 깊게 생각해보지 못했네요..! View에서 사용되는 shape 속성을 OutputView에서 상수로 관리하도록 변경하였습니다. 피드백 감사합니다 👍

src/main/java/ladder/domain/ladder/Height.java

src/main/java/ladder/domain/generator/LadderStepGenerator.java

src/main/java/ladder/view/InputView.java

src/test/java/ladder/domain/participant/ParticipantsTest.java

lxxjn0 · 2024-02-23T04:23:27Z

src/test/java/ladder/domain/ladder/PathTest.java

+        assertEquals("     ", Path.EMPTY.getShape());
+        assertEquals("-----", Path.EXIST.getShape());


AssertJ에서 제공하는 메서드를 사용해보는 것은 어떨까요?

assertThat(Path.EMPTY.getShape()).isEqualTo(" ");

이 메서드를 추천하신게 맞을까요?

기능은 같은 것 같은데 특별히 AssertJ 메서드를 추천해주신 이유가 궁금합니다!

AssertJ가 제공하는 검증 메서드의 종류가 다양하다보니 여러 상황에서 사용하기에 편하다고 생각합니다. 한번 AssertJ 라이브러리를 공부해보면 좋을 것 같아요!!

- 랜덤한 사다리 스텝을 생성하는 역할만 RandomLadderStepGenerator에게 맡기고, 보정은 LadderStep에서 진행한다. 항상 신뢰도 있는 LadderStep 객체를 사용할 수 있다.

lxxjn0

미아, 피드백 반영하시느라 수고 많으셨습니다!!

몇 가지 추가적인 부분에 대한 피드백이 필요할 것 같은데 이는 다음 단계 진행하면서 알려드리는게 좋을 것 같아요!! 다음 단계 코드를 보면서 추가적인 리뷰 전달드리도록 할게요!

lxxjn0 · 2024-02-24T09:49:52Z

src/main/java/ladder/manager/LadderGameManager.java

+import ladder.view.InputView;
+import ladder.view.OutputView;
+
+public class LadderGameManager {


해당 클래스는 굳이 존재할 필요가 없을 것 같아요. 특별한 책임을 가지는 클래스가 아니라면 기존처럼 컨트롤러 단에서 생성하는 것만으로도 충분하지 않을까요??

저는 같은 OutputView 객체를 ExceptionManager와 LadderGame에서 사용하도록 생성자 주입을 사용해주었는데요..! 따로 LadderGameManager 클래스를 만들기보다는 main application에서 의존성을 주입해 주는 것이 더 적절하려나요 🤔 step2에서 더 고민해보도록 하겠습니다 ..!

jongmee and others added 30 commits February 20, 2024 14:13

docs: 구현 기능 목록 작성

14fb069

test: 참가자 생성 및 이름 검증 테스트

6a1ea73

feat: Participant 생성 및 이름 검증 기능 구현

de65c9d

test: Participant 예외 클래스 구체적으로 수정

bd77170

test: 입력된 문자열로 Participants 생성 및 검증 테스트

cdeb4fa

feat: Participants 생성 및 구분자 검증 기능 구현

3e3e05a

test: LadderPathGenerator에서 사다리 생성 검증 테스트

25b4c95

test: LadderPathGenerator 테스트 메서드 명 변경

ba03899

feat: 사다리 한 단 생성 및 검증 구현

a1efa99

refactor: 사다리 한 단 이름 Path에서 Step으로 변경

bd3f5d5

refactor: generator 패키지로 분리

72494a8

test: RandomLadderStepGenerator에서 스텝 생성 테스트

0eb906d

feat: RandomLadderStepGenerator에서 스텝 생성 구현

249b700

refactor: RandomLadderStepGenerator 패키지 이동

f51252c

refactor: LadderStepGenerator에서 스텝 폭 필드 추가

df33e16

test: LadderStep 생성 테스트

e7d51d4

feat: LadderStep 생성

88f81d8

refactor: LadderStepGenerator 반환 타입을 LadderStep으로 변경

36546fb

test: Ladder 생성 테스트

32e308c

feat: Ladder 생성

0d6c9e1

test: PathAvailability 형태 반환 테스트

d5ac292

test: expected와 actual 위치 변경

5cac338

feat: PathAvailability enum 클래스 생성

8c8c855

test: PathAvailability 조회 기능 테스트

db687ad

feat: PathAvailability 조회 기능 구현

0c1e8b0

test: Boolean 타입을 PathAvailability로 변경

fa8880e

refactor: LadderStep Boolean 타입을 PathAvailability로 변경

bc04163

refactor: LadderStepGenerator에 PathAvailability 적용

b1aac95

refactor: PathAvailability를 Path로 변경하고 변수명 수정

0c8d1e6

feat: 사용자 입력 요청 기능 구현

9fd49c8

jongmee and others added 16 commits February 21, 2024 17:47

refactor: 참가자 이름 구분자 상수 이름 변경

6383b81

refactor: 예외 발생 시 재입력 요청 로직 추가

a70e263

refactor: controller에서 불필요한 메서드 제거

62d5ab7

test: 최대 사다리 높이 객체 생성자 테스트

b40296c

feat: 사다리 높이 도메인 클래스 구현

94f5bc1

refactor: Ladder 생성자 매개변수 타입을 int에서 Height로 변경

534e822

test: Participants 생성시 참가자 수 검증 테스트

ac613ae

feat: Participants 생성시 참가자 수 검증 구현

0f9a3f9

fix: 참가자 이름 출력 형식 변경

17cc4c3

fix: 예외 출력 로직 누락 수정

b5aef35

refactor: LadderStep을 record로 수정

4438077

refactor: ladderSteps 조회 시 변경 불가능하도록 수정

ed3e70e

refactor: exception 클래스들 구체적인 패키지로 분리

a4389ec

feat: Participants에서 필요한 사다리 너비를 계산하는 기능 구현

3a4c9bd

refactor: 누락된 final 제어자 추가

bec23f9

refactor: 사다리 출력 방식 변경

a464605

jongmee changed the title ~~[1단계 - 사다리 생성] 미아(이종미) 미션 제출합니다~~ [1단계 - 사다리 생성] 미아(이종미) 미션 제출합니다. Feb 22, 2024

khabh added 2 commits February 23, 2024 13:19

refactor: LadderStepGenerator 생성자를 protected로 수정

2032958

refactor: LadderStep 생성자 재정의

0903323

lxxjn0 requested changes Feb 23, 2024

View reviewed changes

jongmee added 7 commits February 23, 2024 15:51

refactor: Height 도메인 클래스의 isGreaterThan 메서드 매개변수 변경

5c079af

test: LadderStep에서 랜덤한 사다리 스텝을 보정하는 기능 테스트

a2d88af

refactor: LadderStep에서 랜덤한 사다리 스텝을 보정하기

28d7f6d

- 랜덤한 사다리 스텝을 생성하는 역할만 RandomLadderStepGenerator에게 맡기고, 보정은 LadderStep에서 진행한다. 항상 신뢰도 있는 LadderStep 객체를 사용할 수 있다.

test: JUnit5에서 AssertJ로 테스트 메서드 변경

0040973

test: LadderStep 보정 ParameterizedTest로 테스트

99e9044

refactor: 예외시 입력 다시 받는 기능을 재귀함수에서 while문으로 변경

9d5692b

refactor: ExceptionHandler 객체로 예외 처리 기능 분리, 생성자 주입으로 객체 의존성 관리

400cf74

lxxjn0 requested changes Feb 24, 2024

View reviewed changes

lxxjn0 approved these changes Feb 24, 2024

View reviewed changes

lxxjn0 merged commit 57acf0a into woowacourse:jongmee Feb 24, 2024

		@@ -0,0 +1,27 @@
		package ladder.domain.ladder;

		public enum Path {

		assertEquals(" ", Path.EMPTY.getShape());
		assertEquals("-----", Path.EXIST.getShape());

[1단계 - 사다리 생성] 미아(이종미) 미션 제출합니다. #279

[1단계 - 사다리 생성] 미아(이종미) 미션 제출합니다. #279

Uh oh!

Conversation

jongmee commented Feb 22, 2024

사다리 타기 미션을 위해 생성한 사다리 관련 도메인들은 다음과 같습니다.

피드백을 받고 싶은 부분은 아래와 같습니다.

Uh oh!

lxxjn0 left a comment

Choose a reason for hiding this comment

그리고 남겨주신 질문에 대한 답변입니다.

TDD를 잘 하고 있는 것인지

원시값에 대한 포장

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lxxjn0 Feb 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lxxjn0 left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

lxxjn0 Feb 24, 2024 •

edited

Loading