[사다리 타기] 최인호 미션 제출합니다. #7

inhooo00 · 2024-05-12T13:07:23Z

어려웠던 점

TDD 구현이 막막해서 필수적인 모델만 진행하고 구현을 시작했습니다.
사다리의 구조를 2차원 배열을 쓰지 않고 어떻게 할지 고민했습니다.
enum 값이 필수인데, 어떤 부분으로 구현하는 게 좋은 코드일지 고민했습니다.

고민했던 부분 / 질문할 부분

LineTest 구현 중에 랜덤으로 생성되는 값들을 내가 직접 조절하기 위해서 인터페이스를 사용해서 해결했는데, 테스트 코드를 위해서 잘 돌아가는 코드를 수정하는 것이 맞는 것인지 궁금합니다. 혹시 이러한 부분도 TDD를 사용하면 자연스럽게 해결될까요?
LadderTest 구현 중에도 똑같이 인터페이스를 통해 해결하는 방법이 있었는데, 코드가 너무 가독성이 떨어져서 구현을 포기했습니다.
저희가 생각했을 때 Line에서 StringBuilder로 Line 필드를 만드는 것은 전혀 어색하지 않았습니다. 똑같이 Ladder도 사다리를 만드는 것이기 때문에 어색하지 않았습니다. 그런데 문득 StringBuilder면 OutputView가 맞지 않을까 하는 고민이 생겼습니다. StringBuilder로 라인을 만들거나 사다리를 만드는 로직을 OutputView로 따로 빼야 할까요?

- 입력, 출력, 기능으로 분류하여 과제를 진행합니다.

- 테스트 코드를 작성합니다. - 각종 기능 구현 -> 이사 필요 - StringBuiler 부분 view로 이동 필요

- Line 랜덤 값 Test 코드 작성 - Test 코드를 위한 FootholdGenerator 생성

jiyoungmerong

고민했던 부분 / 질문할 부분

LineTest 구현 중에 랜덤으로 생성되는 값들을 내가 직접 조절하기 위해서 인터페이스를 사용해서 해결했는데, 테스트 코드를 위해서 잘 돌아가는 코드를 수정하는 것이 맞는 것인지 궁금합니다. 혹시 이러한 부분도 TDD를 사용하면 자연스럽게 해결될까요?

=> 랜덤값을 인터페이스를 통해 뽑아내는 부분은 괜찮은 것 같아요.

테스트코드는 단순히 기능을 만들기 위해 사용하는 건 아니라고 생각해요!
물론 그런 이유도 있겠지만, 유지보수성을 키우거나 이후에 더 나은 코드를 작성하기 위함도 있죠.
테스트코드를 통과했다고 코드 작성이 끝나는게 아니라, 작성한 코드를 개선해야 한다고 생각해요.
당연히 이 과정에서 테스트 케이스를 통과해야 하구요.
코드를 변경해도 테스트 케이스를 통해 기능을 확실하게 확인할 수 있다는 점에서 장점이 드러나는 것 같습니다 👍🏻
인호님이 어떤 식으로 코드를 수정할지는 모르지만, 어찌되었든 리팩토링을 할 의지가 있다는 점 정말 좋은 것 같아요!
테스트 코드를 위해서 잘 돌아가는 코드를 수정하는 것이 아니라,
테스트 코드를 통과하면서 지금보다 더 효율성이 좋고 가독성 좋은 코드를 짜보아용

LadderTest 구현 중에도 똑같이 인터페이스를 통해 해결하는 방법이 있었는데, 코드가 너무 가독성이 떨어져서 구현을 포기했습니다.
저희가 생각했을 때 Line에서 StringBuilder로 Line 필드를 만드는 것은 전혀 어색하지 않았습니다. 똑같이 Ladder도 사다리를 만드는 것이기 때문에 어색하지 않았습니다. 그런데 문득 StringBuilder면 OutputView가 맞지 않을까 하는 고민이 생겼습니다. StringBuilder로 라인을 만들거나 사다리를 만드는 로직을 OutputView로 따로 빼야 할까요?

=> StringBuilder를 사용하는 부분은 리뷰에 작성해두었습니다.
라인이나 사다리 생성 로직을 뷰에서 진행해야겠다고 생각하신 이유가 있을까요?!
뭔가 저는 뷰에 맞는 책임은 아니라고 생각해서, 한번 생각을 들어보고 싶어요!🤤

우진님이랑 페어셔서 우진님 것도 리뷰 작성하는 중에 사다리 생성이 아니라 사다리 그리는 걸 뷰에서 하고싶다는 말씀인걸로,, ㅎㅎ 그리는 로직은 view에서 진행되면 좋을 것 같아요. 꼭 OutputView가 아니라 사다리 그리는 view를 따로 생성하는 것도 괜찮을 것 같네요!

jiyoungmerong · 2024-05-15T13:48:32Z

src/main/java/controller/LadderGameController.java

+    public LadderGameController() {
+        this.inputView = new InputView();
+        this.outputView = new OutputView();
+    }


내부에서 의존성을 생성하는 것과,
외부로부터 의존성을 주입받는 것의 차이점에 대해 알아보면 좋을 것 같아요!

제가 알고 있는 건 외부에서 의존성 주입은

외부로부터 다양한 값들을 가져올 수 있다는 점. (업케스팅? 느낌)

결합도를 낮춰서, 위 기준으로는 LadderGameController 안에 코드 수정이 일어나지 않게 하기 위함.
정도로 알고 있습니다.

위 코드 기준으로는 할당받을 inputView와 outputView가 다양한 값들이 들어올 일이 없어서 직접 생성하는 방법을 선택했는데 혹시 외부에서 주입받는 방법을 사용하는 것이 좋을까요??

jiyoungmerong · 2024-05-15T13:53:05Z

src/main/java/controller/LadderGameController.java

+    private Players createPlayersFromInput() throws IOException {
+        String[] playerNamesArray = inputView.readPlayerNames().split(",");
+
+        return new Players(Arrays.asList(playerNamesArray));
+    }
+
+    private List<String> extractPlayerNames(Players players) {
+        return players.getPlayers().stream()
+                .map(Player::getName)
+                .collect(Collectors.toList());
+    }
+
+    private Height createHeightFromInput() throws IOException {
+        return new Height(inputView.readLadderHeightNumber());
+    }
+
+    private Ladder createLadder(Players players, Height height) {
+        Ladder ladder = new Ladder();
+
+        ladder.makeLines(height, players);
+
+        return ladder;


각각의 메소드가 하나의 역할만 수행하고 있네요 👍🏻

jiyoungmerong · 2024-05-15T14:24:27Z

src/main/java/exception/LadderGameValidationException.java

@@ -0,0 +1,47 @@
+package exception;
+
+public class LadderGameValidationException {


각각 예외클래스 이름을 잘 지어주셨네요! 👍🏻

크게 리팩토링 해야할 부분은 아닌 것 같지만
만약에 발생할 수 있는 예외가 달라진다면 상속하는 부분을 일일이 바꿔줘야 할 것 같아요!
(예시로 IllegalArgumentException에서 그냥 RuntimeException으로 변경하라고 한다면?!)

이럴 경우에 어떤 방법이 있을지 고민해보면 좋을 것 같아요!

jiyoungmerong · 2024-05-16T11:43:24Z

src/main/java/model/Ladder.java

+import util.RandomFootholdGenerator;
+
+public class Ladder {
+    private final StringBuilder lines;


StringBuilder는 싱글스레드에 적합할까요, 멀티스레드에 적합할까요?
그리고 StringBuilder에 final을 붙인 이유는 무엇인가요?!
위 질문에 대해 고민해보고, StringBuilder를 채택해야하는 이유를 생각해보세요!

jiyoungmerong · 2024-05-16T11:52:20Z

src/main/java/model/Players.java

+    public int findNumberOfPlayers() {
+        return this.players.size();
+    }


players의 크기만 반환하는데 메소드 이름이 조금 애매한 것 같아요!

jiyoungmerong · 2024-05-16T12:31:07Z

src/main/java/model/Players.java

+    private List<Player> makePlayers(final List<String> playerNames) {
+        return playerNames.stream()
+                .map(Player::new)
+                .toList();


16부터 toList() 가 생기면서 정말 편리해진 것 같아요!👍🏻

jiyoungmerong · 2024-05-16T12:56:56Z

src/main/java/view/OutputView.java

+        if (name.length() > width) {
+            return name.substring(0, width);
+        }
+        return String.format("%" + width + "s", name);


format 사용 👍🏻

jiyoungmerong · 2024-05-16T13:06:48Z

src/test/java/model/HeightTest.java

+    @Test
+    @DisplayName("객체 생성 테스트")
+    void Height_Object_Create_Success_Test() {


메소드 코드를 보고 테스트코드임을 충분히 알 수 있을 것 같아요.
메소드 이름에 Test는 제외시키는 건 어떨까요?
Success_Test가 있으니, 실패할 때를 대비한 메소드도 만들어보는 것도 좋을 것 같아요!

jiyoungmerong · 2024-05-16T13:10:15Z

src/test/java/model/LadderTest.java

+    private Ladder ladder;
+    private Players players;
+    private Height height;
+
+    @BeforeEach
+    void setUp() {
+        Ladder ladder = new Ladder();


인스턴스 변수가 초기화될까요, 새로운 setUp() 메소드 내의 지역변수를 선언하게 될까요,
아니면 상관 없는 코드일까요?!

jiyoungmerong · 2024-05-16T13:13:39Z

src/test/java/util/validator/LadderHeightValidatorTest.java

+    @Test
+    void ladderHeightValidator_정상_입력() {
+        String ladderHeight = "5";
+        assertDoesNotThrow(() -> LadderHeightValidator.validateLadderHeightNumberIsCorrect(ladderHeight));
+    }
+
+    @Test
+    void ladderHeightValidator_정수가_아닌_입력() {
+        String ladderHeight = "abc";
+        assertThrows(LadderGameValidationException.NotNumericException.class, () -> LadderHeightValidator.validateLadderHeightNumberIsCorrect(ladderHeight));
+    }
+
+    @Test
+    void ladderHeightValidator_음수_입력() {
+        String ladderHeight = "-1";
+        assertThrows(LadderGameValidationException.NotNaturalNumberException.class, () -> LadderHeightValidator.validateLadderHeightNumberIsCorrect(ladderHeight));
+    }
+
+    @Test
+    void ladderHeightValidator_공백_입력() {
+        String ladderHeight = "";
+        assertThrows(LadderGameValidationException.NotAllowEmptyInputException.class, () -> LadderHeightValidator.validateLadderHeightNumberIsCorrect(ladderHeight));


예외가 발생할 수 있는 부분도 적절하게 테스트 해주셨네요!👍🏻

- 메서드 이름 수정 - 개행 삭제 - 메서드 분

inhooo00 · 2024-05-17T14:37:22Z

StringBuilder는 싱글스레드에 적합합니다. 지금 코드는 싱글스레드 환경이라고 생각해서 StringBuffer보다는 StringBuilder가 맞다고 생각했습니다.
isNotLastPlayer 메서드는 가독성 때문에 분리한 것이 맞습니다!
실패를 대비한 테스트는 validator 예외처리 부분에서 만들어 놓았어서 따로 만들지 않았습니다.
LadderTest class에 setUp 부분도 인스턴스 변수 초기화로 코드 수정 했습니다.

inhooo00 and others added 3 commits May 7, 2024 23:12

docs: 구현할 기능 목록 추가

7bcbdeb

- 입력, 출력, 기능으로 분류하여 과제를 진행합니다.

feat: 전체적인 코드 구현

52d3e29

- 테스트 코드를 작성합니다. - 각종 기능 구현 -> 이사 필요 - StringBuiler 부분 view로 이동 필요

[feat] Line 테스트코드 수정

06e9f84

- Line 랜덤 값 Test 코드 작성 - Test 코드를 위한 FootholdGenerator 생성

jiyoungmerong reviewed May 16, 2024

View reviewed changes

[refactor] 코드 리뷰 반영 수정

5cbf25d

- 메서드 이름 수정 - 개행 삭제 - 메서드 분

inhooo00 requested a review from jiyoungmerong May 17, 2024 14:37

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[사다리 타기] 최인호 미션 제출합니다. #7

[사다리 타기] 최인호 미션 제출합니다. #7

inhooo00 commented May 12, 2024

jiyoungmerong left a comment •

edited

Loading

jiyoungmerong May 15, 2024

inhooo00 May 17, 2024

jiyoungmerong May 15, 2024

jiyoungmerong May 15, 2024

jiyoungmerong May 16, 2024

jiyoungmerong May 16, 2024

jiyoungmerong May 16, 2024

jiyoungmerong May 16, 2024

jiyoungmerong May 16, 2024

jiyoungmerong May 16, 2024

jiyoungmerong May 16, 2024

inhooo00 commented May 17, 2024

		@@ -0,0 +1,47 @@
		package exception;

		public class LadderGameValidationException {

[사다리 타기] 최인호 미션 제출합니다. #7

Are you sure you want to change the base?

[사다리 타기] 최인호 미션 제출합니다. #7

Conversation

inhooo00 commented May 12, 2024

어려웠던 점

고민했던 부분 / 질문할 부분

jiyoungmerong left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

inhooo00 commented May 17, 2024

jiyoungmerong left a comment •

edited

Loading