[2단계 - 사다리 게임 실행] 조조(조은별) 미션 제출합니다. by eun-byeol · Pull Request #383 · woowacourse/java-ladder

eun-byeol · 2024-02-29T06:31:10Z

피케이 안녕하세요! 조조입니다.
2단계 미션 피드백 부탁드립니다😊

🤔 코드를 작성할 때 신경 쓴 점

도메인을 잘 몰라도 알아서 해주는 `친절한 코드`

이번 미션을 수행하면서 가장 고민이 깊어졌던 포인트입니다.

요구하는 쪽에서 일정 부분 로직 처리를 해주고 넘겨 줄 것인가 vs 받는 쪽에서 모든 로직을 처리 할 것인가

저는 후자를 지향합니다.
받는 쪽에서 다 해주면 정보 은닉이 잘 되고, 도메인을 모르는 사람도 쉽게 사용할 수 있다는 장점이 있지만
단, 추상화가 깊어지면 내부 로직을 통제하고 테스트 하기 어려워진다는 단점이 있었습니다.

고민에 대한 나름의 답을 코드에 담아봤습니다. 이에 대한 피케이의 의견 궁금합니다💭

💭 질문

Q. `작은 도메인에서 큰 도메인을 생성하기` vs `큰 도메인에서 작은 도메인을 생성하기`

Ladder 생성 시그니처에 대한 질문입니다. 피케이는 어떤 방식이 합리적이라고 생각하시나요?

Ladder 생성 시 List<Line>를 파라미터로 받기
Ladder 생성 시 LineGenerator를 파라미터로 받기

결과적으로, 2번 방식을 선택했지만 각 방식에 대한 장단점이 있었습니다.

방식1) 외부에서 List을 생성해서 Ladder에 넘겨주는 방식

장점
- Ladder 테스트가 쉽다. Ladder 테스트를 위해 프로덕션 코드에 인터페이스를 두지 않아도 된다.
단점
- 친절하지 않다. Ladder를 생성하는 데 Ladder 도메인에 이해도가 높아야 한다.

방식2) Ladder 내부에서 List을 생성하는 방식

장점
- 친절하다. Ladder를 생성하는 데 Ladder 도메인에 이해도가 높지 않아도 된다.
단점
- Ladder 테스트가 어렵다. 테스트를 위해 인터페이스를 둬야 한다.
- 이때, LineGenerator 인터페이스를 구현하는 테스트 로직(CustomLineGenerator)이 제한적이다.

1단계 미션 피드백이었던

과연 테스트를 위해 프로덕션 코드에 FunctionalInterface 를 추가하는지 좋을지 고민해보고
Ladder 를 테스트할 때는 위의 세 가지 요소를 몰라도 되도록 해보죠!

이 문제로 다시 돌아가게 되었지만, 인터페이스를 두는 방식이 더 '친절한' 코드라고 생각했습니다.
~~친절하고 테스트 코드 작성도 쉬운 방법이 떠오르지 않아 대 혼란의 시간을 보냈습니다..~~
그러나 아직 리팩터링이 필요한 부분이라고 생각합니다. 제 로직에 대한 의견 궁금합니다!

Q. `사다리의 위치값을 나타내는 인덱스`도 도메인으로 만들어야 하는가?

모든 원시 값과 문자열을 포장한다 요구사항에 대한 질문입니다.

사다리 타기 결과를 갖는 GameResult의 생성 메서드 시그니처입니다.

of(List<Integer> positions, Players players, Prizes prizes)

이때 파라미터로 받는 positions도 감싸야 하는 대상인가요?

저는 사다리 타기를 실행하는 Ladder.simulate의 반환값으로 GameResult를 생성하고 있어, 감싸지 않았습니다.

왜 모든 원시 값과 문자열을 포장해야 하는가?에 대한 의문이 있었습니다.
저는 일급 객체로 만들기 사용하기 위함이라고 생각하는데,
특별한 성질이나 로직을 포함하지 않는 경우 값을 감싸는 것이 오히려 복잡도를 증가시킨다고 생각합니다.
이에 대해 피케이는 어떻게 생각하시나요?

- RandomBinaryNumbers util 삭제 및 RandomBridgeGenerator로 변경 - 랜덤숫자 반환 타입 List<Integer>에서 List<Bridge>로 수정 - Bridge enum 필드 삭제 및 로직 단순화 - Line 정적 팩토리 메서드에서 생성자로 변경

prepare 메서드에서 스스로 재입력을 받도록 수정

Player의 MAX_NAME_LENGTH 접근제한자 private으로 변경

RandomBridgeGenerator -> RandomLineGenerator로 이름 변경 Ladder 생성 시 LineGenerator 주입

pkeugine

안녕하세요 조조,
step 2 정말 잘 진행해주셨네요
코드와 커밋에서 굉장히 많은 고민을 하신 흔적이 보였습니다.
자잘한 리뷰 남겨보았으니 한 번 확인해주세요!
주말 잘 보내시고, 궁금한 점 있으면 언제든지 DM 주세요

pkeugine · 2024-03-01T09:50:21Z

+- [x] 생성
+  - [x] 참여자 이름은 쉼표(,)를 기준으로 구분해 입력한다
+  - [x] 최대 사다리 높이는 숫자로 입력한다
+  - [x] 실행 결과는 쉼표(,)를 기준으로 구분해 입력한다
+- [x] 조회
+  - [x] 결과를 보고 싶은 사람을 입력한다
+  - [x] 전체(all) 조회 전까지 조회 입력을 반복한다


readme 정말 잘 관리하셨네요 👍

pkeugine · 2024-03-01T09:50:26Z

+        Prizes prizes = preparePrizes(players);
+        LadderHeight ladderHeight = prepareLadderHeight();
+
+        RandomLineGenerator randomLineGenerator = new RandomLineGenerator();


Suggested change

RandomLineGenerator randomLineGenerator = new RandomLineGenerator();

LineGenerator randomLineGenerator = new RandomLineGenerator();

인터페이스 적극 활용해보죠!

디테일한 부분들을 놓쳐버렸네요..🥲 바로 수정했습니다!

pkeugine · 2024-03-01T09:51:15Z

+    public Integer moveLeft(int position) {
+        return position - movement;


이 메서드는 moveRight 와는 다르게 int 형 대신 Integer 를 반환하는 이유가 있나요?
이참에 boxing 과 unboxing, 그리고 autoboxing 이 뭔지 한 번 알아보아요 :)

의도한 것은 아닙니다..!
primitive과 wrapper 사용 기준에 대해 확신이 없었는데, autoboxing이 판단 기준이 될 수 있을 것 같아요.

autoboxing 관점에서 생각해보니 moveLeft 메서드의 반환 타입을 int로 하는 것이 더 좋다고 생각합니다.
불필요한 autoboxing 과정을 없앨 수 있기 때문입니다.
반환 시 boxing 과정을 생략됩니다. 해당 메서드를 호출하는 쪽에서도 primitive type으로 연산되다보니, unboxing 과정도 사라지겠네요!

꼼꼼한 리뷰 감사합니다☺️

boxing, unboxing, autoboxing 정리

boxing

primitive type을 wrapper class type으로 변환하는 것

ex) int → Integer

unboxing

wrapper class type을 primitive type으로 변환하는 것

ex) Integer → int

autoboxing

자바 5.0 이후, boxing/unboxing 과정이 자동으로 이루어지는 기능

변환을 컴파일러가 자동으로 처리

pkeugine · 2024-03-01T09:51:19Z

@@ -0,0 +1,50 @@
+package model.gameResult;


패키지명은 웬만하면 전체 소문자로 작성해요:
https://google.github.io/styleguide/javaguide.html#s5.2.1-package-names

중요한 컨벤션을 놓쳤었네요..! Google Java Style Guide 숙지 완료했습니다.
(+ 인텔리제이 코드 스타일에도 적용시켰습니다.)

pkeugine · 2024-03-01T09:51:22Z

+    private final ConcurrentHashMap<Player, Prize> result;
+
+    public GameResult(ConcurrentHashMap<Player, Prize> result) {
+        this.result = result;
+    }


인터페이스인 Map 을 사용해보는건 어떨까요?
그리고 ConcurrentHashMap 을 사용하신 이유가 궁금해요.

인터페이스 타입.. 꼭 명심하겠습니다..!

ConcurrentHashMap 사용 이유
멀티 스레드 상황에서 동시성을 보장해주기 때문에 사용했습니다. 당장은 여러 요청이 동시에 들어오는 상황은 아니지만, 실 서비스에서는 멀티 스레드 상황이 존재하기 때문에 동시성 처리를 고려했습니다.

그러나, 더 공부해보니 이번 미션에는 적절하지 않아 HashMap으로 수정했습니다.

이유

멀티 스레드 상황이 아니다.

put 상황이 여러번 발생하지 않는다. gameResult는 불변객체로서 생성될 때 최초 1번 쓰기 작업이 이뤄진다.

동시성 처리를 고려하고자 한다면, 통일성 있게 해줬어야 했다. ArrayList나 정적 메서드, 일반 변수에 대한 연산도 동시성을 보장하지 않는다.

스프링 부트 강의를 들으면서 애플리케이션에 HashMap 대신 ConcurrentHashMap을 사용하라는 것이 주입됐던 것 같아요..
더 분명한 목적을 갖고 사용하겠습니다.
질문 주신 덕분에 더 공부할 수 있었습니다. 감사합니다!👍

현재는 멀티쓰레드 환경이 아니기 때문에 HashMap 을 사용하기로 한 판단 👍
개인적으로 아직 필요하지 않은 '미래에 있을 수도 있는 요구사항' 을 미리 적용하는 것을 지양하는 편이에요.
확장하기 좋은 구조로 만드는 것으로 충분한 것 같거든요.

제가 인강을 듣는편이 아니라서 해당 강의 내용을 완전히 파악한건 아니지만, 아마 그 수업 내용도 Spring 에서 ConcurrentHashMap 을 늘 사용하라는 의미는 아니었을 것 같아요. DB 를 사용하지 않고 메모리에 데이터를 저장할 때 (또는 메모리에 그 데이터를 캐싱할 때) ConcurrentHashMap 을 사용했을 것 같은데, 이런 상황에서는 확실히 여러 쓰레드가 동시에 해당 map 에 접근할 수 있으니까 사용하는게 맞을 것 같네요. 즉 GameResult 는 사다리 게임을 Spring 으로 만들더라도 ConcurrentHashMap 을 사용하지 않아도 될 것 같아요.

네 말씀해주신 상황이 정확합니다.
당시 실습에 메모리에 데이터를 저장하는 예제를 다뤘었습니다.

아직 필요하지 않은 '미래에 있을 수도 있는 요구사항' 을 미리 적용하는 것을 지양

저도 이 기준에 동의합니다! 리팩터링을 하는 데 좋은 기준이 될 수 있을 것 같아요. 감사합니다😊

pkeugine · 2024-03-01T09:53:00Z

+    public static void printGameResultAll(GameResult gameResult) {
+        gameResult.getPlayers()
+                .forEach(player -> printPlayerAndPrize(gameResult, player));
+    }


Suggested change

public static void printGameResultAll(GameResult gameResult) {

gameResult.getPlayers()

.forEach(player -> printPlayerAndPrize(gameResult, player));

}

private static void printGameResultAll(GameResult gameResult) {

gameResult.getPlayers()

.forEach(player -> printPlayerAndPrize(gameResult, player));

}

추가Q. 이런 실수들을 줄일 수 있는 도구나 팁이 있을까요?
커밋 전에 일일이 더블 체크하는 것도 당연하겠지만, 혹시 알고 계신 도구나 팁이 있다면 공유해주시면 감사드리겠습니다😊

제가 아는 선에서는 딱히 없어요. 이거는 그냥 코드 작성하다보면 습관이 되는 것 같아요. 그리고 저런 실수를 방지하기 위해 코드 리뷰를 하는 것도 있겠죠!

pkeugine · 2024-03-01T09:53:02Z

+    private static void printGameResult(String playerName, GameResult gameResult) {
+        Prize prize = gameResult.findPrizeByPlayerName(playerName);
+        System.out.println(prize.getName());
+    }


여러 메서드가 추가되면서 메서드 순서가 조금 바뀐 것 같아요 :)

수정 완료했습니다!!
제가 확인하지 못한 버그였는데 꼼꼼하게 봐주셔서 감사합니다😊

버그까진 아닌데 😂 협업하는 사람들이 많아질수록 이런게 중요한 것 같긴해요.

pkeugine · 2024-03-01T09:53:10Z

        assertThat(ladder.getLines().size())
                .isEqualTo(5);


이전 step 에서 작업해주신 내용인데, 이렇게 쓸 수도 있어요:

Suggested change

assertThat(ladder.getLines().size())

.isEqualTo(5);

assertThat(ladder.getLines())

.hasSize(5);

pkeugine · 2024-03-01T09:53:38Z

+                Arguments.of(List.of("꽝", "식권", "꽝"), Players.of(List.of("pobi", "dora"))),
+                Arguments.of(List.of("배민상품권", "꽝"), Players.of(List.of("pobi", "doraa", "jojo"))),
+                Arguments.of(List.of(), Players.of(List.of("doraa", "jojo")))
+        );


Q. 작은 도메인에서 큰 도메인을 생성하기 vs 큰 도메인에서 작은 도메인을 생성하기
질문을 잘 이해했는지 확실하진 않은데,

public static Ladder of(LadderHeight height, Players players, LineGenerator lineGenerator) {

public static Ladder of(List<Line> lines) {

이 둘의 차이를 의미하시는 걸까요?
조조가 말해주신 것처럼 어떤 방식을 택하더라도 그 트레이드 오프가 있어요.
그런데 저는 외부에서 List을 생성해서 Ladder에 넘겨주는 방식 의 단점이 친절하지 않다. Ladder를 생성하는 데 Ladder 도메인에 이해도가 높아야 한다. 라고 생각하지 않아요.

Ladder ladder = new Ladder(List.of( new Line(List.of(CONNECTED, UNCONNECTED)), new Line(List.of(CONNECTED, UNCONNECTED)), new Line(List.of(CONNECTED, UNCONNECTED)) ));

이렇게 꽤나 자유롭게 만들 수 있고, Line 에는 중복되는 연속된 Bridge 가 올 수 없다는 것만 인지하고 있으면 돼요.
이정도 도메인 지식은
public static Ladder of(LadderHeight height, Players players, LineGenerator lineGenerator) {
위의 방식을 사용하더라도 비슷한 양 또는 위의 메서드가 그 이상인 것처럼 느껴지기도 해요.

그런데 이렇게 도메인 생성 과정을 간단하게 만들면 외부에서 랜덤한 Line 들을 직접 만들어서 넣어줘야하긴 해요.
그런데 개인적으로 Ladder 가 그걸 다 만드는 책임을 다 가져갈 필요는 없다고 생각해요. 테스트 뿐만 아니라 다른 개발자가 Ladder 라는 객체를 사용하는 것도 꽤나 어려워지는 것 같거든요.

네 정확하게 이해해주셨어요!

Ladder 생성 로직에 대해 고민이 많았습니다. 특히 Ladder 테스트 코드를 만드는 것이 너무 복잡했던 문제가 있었어요.
말씀해주신 지점들을 여러 번 생각해보고 수정해보니
테스트 코드 기준에서 기존 로직이 오히려, Ladder를 생성하는 데 너무 많은 정보를 요구하고 있다고 느꼈어요.
테스트가 쉬워야 한다라는 기준으로 보더라도, 외부에서 사다리를 생성하는 것이 합리적이라고 생각해 리팩터링했습니다.

수정사항1 - 외부에서 사다리 생성

controller의 코드입니다.

Ladder ladder = RandomLadderGenerator.generateLadder(ladderHeight, ladderWidth);

수정사항2 - Generator의 역할 변경(Line 생성 -> Ladder 생성)

Line을 랜덤으로 생성하는 기존 RandomLineGenerator에서, Ladder를 랜덤으로 생성하는 RandomLadderGenerator로 수정했습니다.

public static Ladder generateLadder(LadderHeight height, LadderWidth width) { return IntStream.range(0, height.getValue()) .mapToObj(i -> generateLine(width)) .collect(collectingAndThen(toList(), Ladder::new)); }

좋네요! 이러면 Ladder 를 직접 만든다고 하더라도 꽤나 쉽게 만들어줄 수 있겠네요 👍

pkeugine · 2024-03-01T09:53:40Z

+                Arguments.of(List.of(), Players.of(List.of("doraa", "jojo")))
+        );
+    }
+}


Q. 사다리의 위치값을 나타내는 인덱스도 도메인으로 만들어야 하는가?
저도 저번 step 에서 조조에게 리뷰 남겼던 것처럼 모든 원시값을 포장해서 사용하는 것에 의문을 가져요.

저는 일급 객체로 만들기 사용하기 위함이라고 생각하는데,
저도 저번 step 에서 조조에게 리뷰 남겼던 것처럼 모든 원시값을 포장해서 사용하는 것에 의문을 가져요.
물론 그렇게 하는 사람도 있을 것이고, 제가 생각했을 때 요구사항의 목적은 객체지향과 객체의 책임에 대한 고민을 해보라는 의미에서 포함하지 않았을까 싶어요.

특별한 성질이나 로직을 포함하지 않는 경우 값을 감싸는 것이 오히려 복잡도를 증가시킨다고 생각합니다.
저도 과한 클래스 분리는 오히려 유지보수하기 어렵게 만든다고 생각해요.

따라서 꼭 포장해서 사용하지 않아도 된다고 생각합니다.
특히 index 의 경우엔 관련된 로직이 많이 없으니까요.

- 패키지명 변경: gameResult -> gameresult - 필드에 개행 추가

- 파라미터 Player 대신 LadderWidth로 변경 - LadderWidth 클래스 정의

- Ladder.simulate 메서드 삭제 - LadderResult 클래스 생성시 Ladder을 인자로 받아 생성 결과 계산 - LadderWidth 정적 팩토리 메서드 인자 타입 int로 변경

생성 로직 성공 테스트 라이브러리 수정 - jupiter의 assertDoesNotThrow에서 core의 assertThatCode로 통일

- LineGenerator 인터페이스 및 구현체 삭제, RandomLadderGenerator 로 대체 - Ladder의 생성 로직을 RandomLadderGenerator에 분리 - Ladder 생성 시 Line 리스트를 받는 방식으로 수정

결과 조회 시 오류 메시지와 인트로 구문이 함께 출력되는 문제 해결

- 기존 '참가자와 실행 결과를 연결해 게임 결과 생성한다' 테스트 '존재하는 참가자의 실행 결과 조회 성공' 테스트로 리팩터링 - '존재하지 않는 참가자를 조회하면 예외 발생' 테스트 추가

eun-byeol · 2024-03-03T11:56:30Z

꼼꼼하게 봐주셔서 감사드립니다!
특히 지난 리뷰 요청에 실수하지 않아야 할 부분에 실수가 많아서 아쉬움이 있었지만,
제가 어떤 부분에 실수하는 지 알고, 이를 줄일 수 있는 방법에 대해서도 공부하게 되었습니다. 감사드려요!

이번 리뷰 요청에 주요 수정 사항

1. Ladder 외부에서 사다리 생성, Generator의 역할 변경(Line 생성 -> Ladder 생성)

LineGenerator 인터페이스와 구현체 삭제, RandomLadderGenerator 클래스 추가
커밋 895764b - refactor: LineGenerator 삭제 및 Ladder 생성 로직 변경

2. Players, Prizes 없이 사다리 결과 생성

LadderResult 클래스를 추가
커밋 e21ded1 - refactor: 사다리 결과 생성 클래스 분리

3. 재입력 로직을 InputView로 위임

커밋 007b591 - refactor: 재입력 로직 InputView로 이동

추가 질문 사항

pkeugine

안녕하세요 조조! 리뷰를 남길 내용이 이제 딱히 없네요!
슬슬 merge 해도 괜찮을 것 같긴한데 그래도 아직 하루가 남아있으니 request changes 로 둘게요.
조조가 원하시는 변경 적용해주시면 돼요.
혹시나 내일 하루동안에도 모르는게 생기거나 제 리뷰 이해 못하셨다면 언제든 DM 주세요!

pkeugine · 2024-03-03T17:12:09Z

-    @CsvSource({"pobiii", "dooraaa", "jojojojojojo"})
+    @ValueSource(strings = {"pobiii", "dooraaa", "jojojojojojo"})


pkeugine · 2024-03-03T17:16:21Z

+        System.out.println(GAME_RESULT_INTRO);
+        System.out.println(prize.getName());


이번 step 에서는 System.lineSeparator()` 사용하셔서 System.out.println 사용 횟수를 줄여보는건 어떨까요?
출력단계에서 걸리는 시간은 출력하는 문자열의 길이 보다는 출력 횟수 가 더 큰 영향을 끼치는데요, 그 이유를 한 번 알아봐도 좋을 것 같아요.

사용 횟수를 줄이는 방식으로 리팩터링했습니다!
System.out.println 성능에 대해 공부해 본 적이 없었는데, 좋은 공부 주제인 것 같아요😊

출력 횟수가 더 큰 영향을 끼치는 이유

I/O 작업은 시스템 콜을 거쳐 커널모드에서 작업하기 때문에 시스템 오버헤드가 크다.

println 메서드 내부에 synchronized 키워드로 인해 lock이 걸린다.

즉, 출력 작업이 끝날 때까지 대기해야 하기 때문에 여러번 호출하는 것은 성능에 불리하다.

log 관리에 println이 사용되지 않는 근거가 되기도 한다.

개행문자 \n 대신 System.lineSeparator() 사용

- getter에만 get 키워드 사용하도록 변경

eun-byeol · 2024-03-04T08:23:04Z

한 번 더 리뷰 받을 수 있는 기회를 주셔서 감사합니다😊
더 해 볼 수 있는 것이 뭐가 있을까 생각하다가, 피드백 주신 내용 + 몇가지 더 수정해 보았습니다.

수정 사항

생성자 방어적 복사 적용 - 8a89321
Line 정적 팩토리 메서드로 수정 - aca2452
System.lineSeparator() 사용 - 2d8e9af
get 키워드 포함된 메서드 이름 리팩터링 - 95e49e7

질문 사항

네이밍 팁 주신 내용 추가 설명 요청

pkeugine

안녕하세요 조조!
이제 사다리 게임은 머지해도 좋을 것 같아요.
step 1, step 2 에 걸쳐서 정말 많은 것들에 대해 얘기했는데, 다음 미션에서 도움이 되었으면 하네요! 고생 많으셨고 다음 미션도 파이팅입니다 :)

eun-byeol added 30 commits February 26, 2024 14:42

refactor: 컨트롤러 재입력 로직 수정

8f3c269

prepare 메서드에서 스스로 재입력을 받도록 수정

refactor: Player 상수와 OutputView 상수 따로 관리

f365ddb

Player의 MAX_NAME_LENGTH 접근제한자 private으로 변경

docs: 기능 목록 추가

83fde73

test: 실행 결과가 5자 이하이면 객체 생성 성공

6bf2afe

feat: 실행 결과가 5자 이하이면 객체 생성 성공

71286da

test: 실행 결과가 5자 초과면 예외 발생

1907908

feat: 실행 결과가 5자 초과면 예외 발생

e669e60

test: 실행 결과 수가 참여자 등록 수와 같으면 객체 생성 성공

a4a3c79

feat: 실행 결과 수가 참여자 등록 수와 같으면 객체 생성 성공

91f466e

test: 실행 결과 수가 참여자 등록 수와 다르면 예외 발생

23bc581

feat: 실행 결과 수가 참여자 등록 수와 다르면 예외 발생

7f1c1a5

test: ApplicationTest 참여자 이름 입력 예외 테스트 메시지 수정

35f13ae

refactor: 랜덤 다리 생성 로직 인터페이스로 분리

e887e9a

RandomBridgeGenerator -> RandomLineGenerator로 이름 변경 Ladder 생성 시 LineGenerator 주입

test: 전체 참여자의 실행 결과를 계산한다

14daac9

feat: 전체 참여자의 실행 결과를 계산한다

b5460da

feat: 실행 결과는 쉼표(,)를 기준으로 구분해 입력한다

911d033

feat: 사다리를 출력할 때 실행 결과도 같이 출력한다

d465c03

feat: 결과를 보고 싶은 사람을 입력한다

4f23f94

feat: 개인별 이름 조회 시 개인별 실행 결과를 출력한다

18371a0

feat: 전체(all) 조회 전까지 조회 입력을 반복한다

c16f055

feat: 전체(all) 조회 시 전체 참여자의 실행 결과를 출력한다

c3f40b4

remove: RandomBridgeGenerator 삭제

c2fcae7

refactor: 컨트롤러, 뷰 메서드 리팩터링

a080140

refactor: LadderHeight 레코드에서 클래스로 변경

a05af47

refactor: Ladder 불변 리스트로 생성하도록 수정

6bc7377

refactor: Line 메서드 리팩터링

6ab3f08

refacotr: Player 메서드 위치 수정

d0dce85

refactor: Line 지역변수 이름 수정

7126026

refactor: Line 매직넘버 상수화

9b35975

test: Application 사다리 생성 결과 출력 테스트 리팩터링

f8b6828

pkeugine requested changes Mar 1, 2024

View reviewed changes

eun-byeol added 19 commits March 2, 2024 01:17

refactor: randomLineGenerator 타입 인터페이스로 수정

b6542f4

refactor: 메서드 반환 타입 wrapper에서 primitive로 변경

ea853fe

style: Google Java Style 적용

a3c0727

- 패키지명 변경: gameResult -> gameresult - 필드에 개행 추가

refactor: ConcurrentHashMap을 HashMap으로 수정

8ab7129

refactor: Ladder 정적 팩토리 메서드 파라미터 수정

129ff31

- 파라미터 Player 대신 LadderWidth로 변경 - LadderWidth 클래스 정의

refactor: 사다리 결과 생성 클래스 분리

e21ded1

- Ladder.simulate 메서드 삭제 - LadderResult 클래스 생성시 Ladder을 인자로 받아 생성 결과 계산 - LadderWidth 정적 팩토리 메서드 인자 타입 int로 변경

refactor: Line 형성 로직 이름 수정

f0b8474

refactor: LineGenerator 상수 필드 삭제 및 구현체로 이동

b35d63a

refactor: private 메서드 static 키워드 삭제 및 메서드명 수정

94cfeb6

refactor: OutputView의 printGameResultAll 접근제어자 수정

11cd842

test: 테스트 라이브러리 변경

4e631ea

생성 로직 성공 테스트 라이브러리 수정 - jupiter의 assertDoesNotThrow에서 core의 assertThatCode로 통일

refactor: LineGenerator 삭제 및 Ladder 생성 로직 변경

895764b

- LineGenerator 인터페이스 및 구현체 삭제, RandomLadderGenerator 로 대체 - Ladder의 생성 로직을 RandomLadderGenerator에 분리 - Ladder 생성 시 Line 리스트를 받는 방식으로 수정

fix: 게임 결과 출력 순서 수정

0866372

결과 조회 시 오류 메시지와 인트로 구문이 함께 출력되는 문제 해결

refactor: 컨트롤러 run 내부 메서드 분리

25c5712

refactor: 재입력 로직 InputView로 이동

007b591

refactor: Prizes 정적 팩토리 메서드명 수정

4fa238a

test: 참가자와 실행 결과를 연결해 게임 결과 생성한다

13981df

test: CsvSource에서 ValueSource 테스트로 수정

2cbada7

pkeugine requested changes Mar 3, 2024

View reviewed changes

eun-byeol added 4 commits March 4, 2024 15:41

refactor: View 개행 출력 방식 수정

2d8e9af

개행문자 \n 대신 System.lineSeparator() 사용

refactor: get 키워드 사용된 메서드 이름 수정

95e49e7

- getter에만 get 키워드 사용하도록 변경

refactor: Line 정적 팩토리 메서드로 수정

aca2452

refactor: 생성자에 방어적 복사 적용

8a89321

pkeugine approved these changes Mar 4, 2024

View reviewed changes

pkeugine merged commit 577abb6 into woowacourse:eun-byeol Mar 4, 2024

	RandomLineGenerator randomLineGenerator = new RandomLineGenerator();
	LineGenerator randomLineGenerator = new RandomLineGenerator();

		public Integer moveLeft(int position) {
		return position - movement;

		@CsvSource({"pobiii", "dooraaa", "jojojojojojo"})
		@ValueSource(strings = {"pobiii", "dooraaa", "jojojojojojo"})

		System.out.println(GAME_RESULT_INTRO);
		System.out.println(prize.getName());

Conversation

eun-byeol commented Feb 29, 2024

🤔 코드를 작성할 때 신경 쓴 점

도메인을 잘 몰라도 알아서 해주는 친절한 코드

💭 질문

Q. 작은 도메인에서 큰 도메인을 생성하기 vs 큰 도메인에서 작은 도메인을 생성하기

방식1) 외부에서 List을 생성해서 Ladder에 넘겨주는 방식

방식2) Ladder 내부에서 List을 생성하는 방식

Q. 사다리의 위치값을 나타내는 인덱스도 도메인으로 만들어야 하는가?

Uh oh!

pkeugine 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!

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

boxing, unboxing, autoboxing 정리

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!

eun-byeol Mar 3, 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!

Choose a reason for hiding this comment

Uh oh!

eun-byeol Mar 3, 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!

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

수정사항1 - 외부에서 사다리 생성

수정사항2 - Generator의 역할 변경(Line 생성 -> Ladder 생성)

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eun-byeol commented Mar 3, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

이번 리뷰 요청에 주요 수정 사항

1. Ladder 외부에서 사다리 생성, Generator의 역할 변경(Line 생성 -> Ladder 생성)

2. Players, Prizes 없이 사다리 결과 생성

3. 재입력 로직을 InputView로 위임

추가 질문 사항

Uh oh!

pkeugine left a comment • 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!

Choose a reason for hiding this comment

Uh oh!

도메인을 잘 몰라도 알아서 해주는 `친절한 코드`

Q. `작은 도메인에서 큰 도메인을 생성하기` vs `큰 도메인에서 작은 도메인을 생성하기`

Q. `사다리의 위치값을 나타내는 인덱스`도 도메인으로 만들어야 하는가?

eun-byeol Mar 3, 2024 •

edited

Loading

eun-byeol Mar 3, 2024 •

edited

Loading

eun-byeol commented Mar 3, 2024 •

edited

Loading

pkeugine left a comment •

edited

Loading