[1단계 - 사다리 생성] 몰리(김지민) 미션 제출합니다. #314
Conversation
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
- InputView 에 입력받은 텍스트를 파싱해 전달하는 메서드 작성 Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
Co-authored-by: HaiSeong <[email protected]>
| @@ -0,0 +1,19 @@ | |||
| ### 기능 요구사항 정리 | |||
|
|
||
| import java.util.List; | ||
|
|
||
| public class People { |
| @@ -0,0 +1,21 @@ | |||
| package formatter; | |||
There was a problem hiding this comment.
view 을 위한 객체이니 view 패키지에 위치시키는것은 어떻게 생각하시나요?
가장 관련된 곳에 위치시키면 다른 개발자가 봤을 때 어디서 사용되는지 바로 눈치챌 수 있다는 장점이 있어요 😄
formatter 라는 패키지를 분리해야될 정도로 범용적인 클래스였을까요? 🤔
There was a problem hiding this comment.
찰리 의견에 동의합니다!
view에서만 사용하는, view를 위한, formatter인만큼 view 패키지 안에 위치시켜도 될 것 같다는 생각을 했어요 😄
오히려 현재처럼 루트 패키지에 위치시킬 경우 범용적으로 사용되는 패키지로 오해할 수 있겠네요!
좋은 의견 감사합니다 :)
| @@ -0,0 +1,16 @@ | |||
| package model.path; | |||
|
|
|||
| public enum Path { | |||
| import java.util.List; | ||
|
|
||
| public interface PathGenerator { | ||
| List<Path> generate(final int count); |
There was a problem hiding this comment.
경로 생성은 interface로 추상화 👍
어떤 의도로 추상화를 해주셨는지 궁금해요!
There was a problem hiding this comment.
[추상화 시도 이유]
처음 테스트 코드를 작성할 때는,
Line 클래스 내부에서 Random을 사용하여, 랜덤한 List<Boolean> (= 지금의 List<Path>)에서 true가 연달아 나오는가? 를 테스트했는데요!
// 수정 전의 LineTest.java
Line line = new Line(personCount);
for (int i = 0; i < personCount - 1; i++) {
assertThat(line.get(i)).isNotEqualTo(line.get(i + 1));
}하지만 위 테스트는 제어할 수 없는 Random 때문에 신뢰할 수 없는 테스트라는 생각이 들었습니다...!
때문에 Line에는 List를 주입받고 Ladder에서 전달을 해야겠네? -> 그러면 Ladder에서 Random을 사용하는 건 괜찮을까? -> 인터페이스를 받아서 상황에 따른 구현체를 사용하자
라는 생각의 흐름으로 나아갔던 것 같아요!
There was a problem hiding this comment.
[추가 궁금한 점 - RandomPathGenerator의 생성시점]
그런데 위 생각을 작성하다가 궁금한 점이 생겼어요. 찰리!
현재는 controller 패키지 안의 LadderGame에서 RandomPathGenerator를 생성하는데요!
// LadderGame.java
private Ladder initLadder(final int personCount) {
final int height = inputView.inputHeight();
return Ladder.from(height, personCount, new RandomPathGenerator()); // RandomPathGenerator 생성
}RandomPathGenerator의 생성 시점은 언제가 더 적절할 지에 대한 찰리의 의견이 궁금합니다!
저는 일단 2가지의 시점을 생각해보았어요!
- LadderGame 생성 시
- LadderGame 내부의 Ladder 생성 시(현재 방법)
1번 방법은,
장점으로 현재는 테스트하기 어려운 LadderGame 자체를 테스트 해볼 수 있다는 생각이 들었고, 단점으로 LadderGame 생성 시 PathGenerator까지 생성해주어야 한다는 사실을 다른 개발자가 이해하기 힘들 수도 있고, PathGenerator를 실행부까지 노출 시켜도 되는 것인가 하는 생각이 들었어요.
제가 현재 2번 방법을 선택한 이유는
단위 테스트를 통해 핵심 모델에 대해 테스트를 진행했고 때문에 LadderGame를 테스트해야 할 필요성을 느끼지 못했었어요.
또 현재 LadderGame가 컨트롤러의 역할을 하는데, LadderGame보다는 모델인 Ladder의 생성 시점에 RandomPathGenerator를 전달하는 것이 자연스러울 것 같았기 때문이에요..!
찰리의 의견이 궁금합니다..!
There was a problem hiding this comment.
Line 클래스 내부에서 Random을 사용하여, 랜덤한 List (= 지금의 List)에서 true가 연달아 나오는가? 를 테스트했는데요!
하지만 위 테스트는 제어할 수 없는 Random 때문에 신뢰할 수 없는 테스트라는 생각이 들었습니다...!
테스트를 작성하고 고민해보니 더 좋은 방향으로 리팩토링된 케이스군요 👍
이 부분도 테스트의 장점이라고 생각해요! 더 나은 구조로 변경하게 해주는 것 😃
도메인 내에 제어할 수 있는 부분만 남았으니 코드를 이해하는데 훨씬 간결해진다고 생각해요!
때문에 Line에는 List를 주입받고 Ladder에서 전달을 해야겠네? -> 그러면 Ladder에서 Random을 사용하는 건 괜찮을까? -> 인터페이스를 받아서 상황에 따른 구현체를 사용하자
생각의 흐름이 훌륭하네요 💯 💯 💯 💯
There was a problem hiding this comment.
중요한 고민이라고 생각해요!! 👍
각각의 방법에서 생각하는 장점도 말씀해주셔서 감사해요 🙇
- LadderGame이 생성되는 시점
- 생성자에서 인자로 받거나 생성자 내부에서 RandomPathGenerator를 생성하는 방법이 떠오르네요 :)
- 이렇게 했을 때 장점은
- 말씀대로 LadderGame을 테스트하는데 Random 요소를 제어할 수 있고
- LadderGame에 다양한 PathGenerator 방법을 주입해줄 수 있곘네요 :)
- 말씀해주신 단점은
- PathGenerator 생성 여부가 알기 힘들다는 생성자에서 PathGenerator를 인자로 받는다면 명확하게 알려줄 수 있는 부분이겠네요 :)
- 실행부(Application) 까지 model이 노출되어도 되는가? 는 고민해야하는 부분이네요! 🤔 몰리는 어떻게 생각하시나요?
- LadderGame 내부의 Ladder 생성 시점
단위 테스트를 통해 핵심 모델에 대해 테스트를 진행했고 때문에 LadderGame를 테스트해야 할 필요성을 느끼지 못했었어요.
동의하는 부분입니다 👍 저도 LadderGame을 테스트했을 때 얻는 장점이 커보이지 않아요
현재 LadderGame가 컨트롤러의 역할을 하는데, LadderGame보다는 모델인 Ladder의 생성 시점에 RandomPathGenerator를 전달하는 것이 자연스러울 것 같았기 때문이에요..!
좋은 전략이라 생각해요 👍
Ladder를 생성하기 위해 어떤 방법을 쓰는지 코드상에서 명확하네요!
src/main/java/model/Line.java
Outdated
| final Path left = paths.get(i); | ||
| final Path right = paths.get(i + 1); | ||
| if (left == Path.EXIST && right == Path.EXIST) { | ||
| throw new IllegalArgumentException("사다리의 경로는 연달아 있을 수 없습니다."); |
There was a problem hiding this comment.
여긴 depth 가 두 번 들어갔군요..!
아래의 프로그래밍 요구사항을 지키도록 리팩토링 해볼 수 있겠네요 😄
indent(인덴트, 들여쓰기) depth를 2를 넘지 않도록 구현한다. 1까지만 허용한다
There was a problem hiding this comment.
앗 이 부분을 놓쳤네요...🥲
요구사항을 준수하도록 수정하겠습니다!
src/main/java/parser/NameParser.java
Outdated
| @@ -0,0 +1,13 @@ | |||
| package parser; | |||
There was a problem hiding this comment.
parser도 formatter와 비슷한 맥락으로 보이네요..!
view 패키지에 위치하는것은 어떻게 생각하시나요! 😃
src/main/java/model/Line.java
Outdated
| for (int i = 0; i < paths.size() - 1; i++) { | ||
| final Path left = paths.get(i); | ||
| final Path right = paths.get(i + 1); | ||
| if (left == Path.EXIST && right == Path.EXIST) { |
| } | ||
|
|
||
| @Test | ||
| @DisplayName("사다리의 경로는 연달아 있을 수 없다.") |
There was a problem hiding this comment.
DisplayName을 잘 지어주셔서 무엇을 검증하는지 한 눈에 파악할 수 있네요 👍
| @@ -0,0 +1,28 @@ | |||
| package model; | |||
There was a problem hiding this comment.
TDD를 실천하면서 구현했을 때 몰리가 느낀 장점이나 단점이 있었을까요? 😄
There was a problem hiding this comment.
우선 장점을 얘기하자면,
저는 설계 단계에서 도출한 객체와 구조를 최대한 바꾸지 않도록 하려고 고민하지만, 잘하지는 못하는 것 같아요 🥲
때문에 제가 한 설계에 종종 대해 확신하지 못했는데, TDD는 테스트 코드를 짜고 딱 그때 필요한 객체를 생성하는 방식이기 때문에 초기 설계에 집착할 필요가 없어 심리적으로 편했습니다..!
단점은..
사실 TDD를 진행하면서 너무 만족했기 때문에 크게 느끼지 못했어요..
그런데 TDD를 하면서 스스로에게 조금 아쉬웠던 부분은, 처음에는 cycle을 잘 지키기 위해 노력했지만, 하다보니 한번씩 테스트 코드와 상관없이 로직을 변경해버렸다는 것 같아요😭
There was a problem hiding this comment.
저는 설계 단계에서 도출한 객체와 구조를 최대한 바꾸지 않도록 하려고 고민하지만, 잘하지는 못하는 것 같아요 🥲
때문에 제가 한 설계에 종종 대해 확신하지 못했는데, TDD는 테스트 코드를 짜고 딱 그때 필요한 객체를 생성하는 방식이기 때문에 초기 설계에 집착할 필요가 없어 심리적으로 편했습니다..!
크.. TDD의 장점을 잘 느끼셨군요!
도메인에 대한 이해만 확실하게 하고 큰 설계없이 코드 작성하는것 👍
사실 TDD를 진행하면서 너무 만족했기 때문에 크게 느끼지 못했어요..
그런데 TDD를 하면서 스스로에게 조금 아쉬웠던 부분은, 처음에는 cycle을 잘 지키기 위해 노력했지만, 하다보니 한번씩 테스트 코드와 상관없이 로직을 변경해버렸다는 것 같아요😭
그럼에도 TDD로 잘 진행하신것으로 보여요 😮
익숙하지 않은 방식임에도 노력하는게 멋지네요 👍
|
|
||
| import java.util.List; | ||
|
|
||
| public class FixedPathGenerator implements PathGenerator { |
There was a problem hiding this comment.
테스트를 위한 객체는 테스트가 있는 패키지로 이동시켜도 되겠어요!
-> 다른 개발자에게 혼란을 줄 수 있으니까요 😃
| } | ||
|
|
||
| private Path getNextPath(final List<Path> paths) { | ||
| if (!paths.isEmpty() && getLastPath(paths) == Path.EXIST) { |
There was a problem hiding this comment.
아래와 같이 메서드를 추출하면 한눈에 알 수 있을것같아요
물론 지금도 읽기에 무리는 없어보이네요 ㅎㅎ.... 넘어가셔도됩니다
| if (!paths.isEmpty() && getLastPath(paths) == Path.EXIST) { | |
| if (!paths.isEmpty() && this.existLastPath(paths)) { |
| } | ||
|
|
||
| @Override | ||
| public boolean equals(final Object o) { |
There was a problem hiding this comment.
Ladder 테스트에서 생성한 Ladder의 Line이 주입한 Line과 동일한지를 통해 Ladder가 잘 생성되는지를 검증하려고 추가를 했습니다...ㅎㅎ
답변을 적다보니equals를 사용하지 않고 구현할 수 있을 것 같네요.. 수정하겠습니다..!
| import model.Ladder; | ||
| import model.People; | ||
|
|
||
| public record Result(List<String> names, List<LineInfo> lines) { |
안녕하세요 찰리!
이번 미션을 하면서 TDD에 대해 있었던 오해를 풀 수 있었던 것 같아요 :)
그동안 TDD는 어떻게 해야할 지 감이 안오고 어려운 것이라고 막연하게 생각했었는데, 막상 TDD를 하다보니 기존의 방식(설계 단계에 객체 도출 후 구현)보다 더 재미있었던 것 같습니다!ㅎㅎ
리뷰 잘 부탁드립니다🙏