[1단계 - 자동차 경주 구현] dino.shin(신종화) 미션 제출합니다. #13

jjongwa · 2024-03-26T08:11:58Z

안녕하세요! 이번 카카오 기술 온보딩 진행중인 디노라고 합니다! 🦖

이번 미션에서는 의식적으로 최소 단위를 찾아내는 것과
전체적인 구현 과정에 대해 tdd 사이클을 지키는 것을 중점적으로 두었습니다!

Name, Position, StepCount 등 원시값 포장을 통해 각각의 상태와 제약 조건을 관리하도록 구성했고,
객체의 리스트를 대상으로 진행되는 메서드들은 Cars와 Winners와 같은 일급 컬렉션을 만들어 책임을 부여해 주었습니다.

또한, 0~9 사이의 값을 랜덤으로 받아 진행하는 부분의 테스트는 NumberGenerator라는 인터페이스를 두고
production과 test에서 사용하는 구현체를 각각 구성해 가능한 최소단위의 테스트들을 모두 작성하려 했습니다!

이번 미션의 테스트를 진행하면서 여러 가지 메서드들을 활용해보려 했는데요,
한가지 궁금한 점이 생겨 질문 드립니다..!

Q. assertAll과 assertSoftly 사용의 기준

제가 현재 알고 있는 assertAll과 assertSoftly의 차이는 softly에서는 실패한 검증문의 위치를 결과에 바로 알려준다 입니다.
하지만 이러한 경우 무조건 softly가 all보다 좋아 굳이 all을 쓸 필요가 없지 않을까 하는 생각이 들었는데,
혹시 이 두 방식의 또 다른 차이점이 있는지, 리뷰어님은 어떤 기준에 따라 softly와 all을 쓰시는지 궁금합니다!

이번 미션 잘 부탁드려요!🙇🙇🙇

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

hyunssooo

안녕하세요. 🙂
이번 미션 함께 진행할 정현수입니다.
몇 가지 코멘트 남겼어요. 확인 부탁드립니다!

hyunssooo · 2024-03-26T09:51:11Z

src/test/java/stringcalculator/CalculatorTest.java

+
+    private final static Calculator CALCULATOR = new Calculator();


Suggested change

private final static Calculator CALCULATOR = new Calculator();

private static final Calculator CALCULATOR = new Calculator();

hyunssooo · 2024-03-26T13:27:04Z

src/main/java/stringcalculator/Calculator.java

+
+        final Matcher m = Pattern.compile(CUSTOM_REGEX).matcher(text);


Pattern.compile(CUSTOM_REGEX) 이 부분은 한번 생성하고 재사용 가능하지 않을까요?
이펙티브 자바 아이템 6. 불필요한 객체 생성을 피하라 를 읽어보시면 좋을 것 같아요.

말씀해 주신대로 Patten을 재사용할 수 있게 변경했습니다.! a1a9f11

hyunssooo · 2024-03-26T13:27:56Z

src/main/java/stringcalculator/Calculator.java

+
+    private Integer getSum(final String[] inputNumbers) {
+        final List<Integer> numbers = Arrays.stream(inputNumbers)


return int 와 Integer 는 어떤 차이가 있을까요?

Integer는 원시값 타입인 int를 래핑하여 추가적인 함수를 제공하고,
null 값을 처리할 수 있는 것으로 알고 있습니다!

스트림을 통해 List로 변환하는 과정에서 파싱된 String 값 중 null이 있는지 체크하는 과정이 필요할까요?

큰 의미가 있는 코멘트는 아니였어요. Calculator#calculate 와 Calculator#getSum 리턴 타입 차이가 있어 남긴 코멘트였습니다!

이펙티브 자바 아이템 61. 박싱된 기본 타입보다는 기본 타입을 사용하라
가볍게 읽어보셔도 좋을 것 같아요!

hyunssooo · 2024-03-26T13:51:51Z

src/test/java/racingcar/model/CustomNumberGenerator.java

+
+public class CustomNumberGenerator implements NumberGenerator{
+


테스트용 구현체 👍

hyunssooo · 2024-03-26T14:27:10Z

src/main/java/racingcar/model/Name.java

+
+public class Name {
+


원시값 포장 👍
불변 객체로 잘 작성해 주셨습니다.

String name1 = new Name("a"); String name2 = new Name("a");

두 객체는 같다고 볼 수 있을까요? Value Object에 대해서 학습해 보면 좋을 것 같아요.

각각 다른 인스턴스에 해당하기 때문에 다르게 인식됩니다!
VO에 해당하는 객체들에 equals & hashCode 재정의 하는 걸 놓쳤네요..

해당 작업 진행 & 테스트에서 usingRecursiveComparison() 사용하던 테스트 변경헀습니다! d97e22c

hyunssooo · 2024-03-26T14:43:33Z

src/main/java/racingcar/model/CarAction.java

+
+    public static boolean isMove(final CarAction targetCarAction) {
+        return targetCarAction.equals(MOVE);
+    }


Enum 의 인스턴스도 객체인데 각 객체에게 위임할 수 있지 않을까요? 여기서 static 메서드가 필요할까요?

확실히 해당 객체에게 역할을 부여해 주는게 더 자연스러워 보이네요.!

변경했습니다! 8e029d6

hyunssooo · 2024-03-26T14:43:54Z

src/main/java/racingcar/model/Cars.java

+
+public class Cars {
+


일급컬렉션 👍

hyunssooo · 2024-03-26T14:47:59Z

src/main/java/racingcar/model/Cars.java

+    public Winners getWinners() {
+        return new Winners(values.stream()
+                .max(Comparator.comparingInt(car -> car.getPosition().getValue()))
+                .map(car -> values.stream()
+                        .filter(c -> c.getPosition().getValue() == car.getPosition().getValue())
+                        .map(Car::getName)
+                        .collect(Collectors.toUnmodifiableList()))
+                .orElseThrow(() -> new IllegalArgumentException(INVALID_CARS_STATE_MESSAGE)));
+    }
+


car.getPosition().getValue() 를 통해서 값을 꺼내서 비교하고 있는데요. 객체 자체로 비교하거나 객체에게 메시지를 보내보는 것은 어떨까요?

Car의 Position을 통해서 비교하고 있는데 Position 자체를 비교할 수 있는 방법은 없을까요? 더 나아가 Car끼리 비교할 수 있는 방법은 없을까요??

compareTo를 재정의 하여 포지션끼리의 비교를 해당 객체 자체가 맡을 수 있도록 변경하였고,
Car 끼리의 비교도 isAt 메서드를 통해 비교하도록 변경해 봤습니다!

또한, 우승자를 도출하는 메서드가 이중 스트림으로 구성되어 있어 이를 findWinnerPosition()과 getWinners()로 분리하는 작업도 추가적으로 진행해 보았습니다! 4bdee22

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

jjongwa · 2024-03-27T06:10:14Z

@hyunssooo 꼼꼼한 리뷰 감사합니다! 🙇🙇

특히 Cars에서 우승자 도출 부분은 생각하지 못했는데,
아무래도 스트림이 완전히 익숙하지 않다보니 구현하면서 객체지향 원칙들을 어기게 된 것 같습니다.. 아직 갈 길이 머네요..

남겨주신 리뷰에 대한 답글 & 리팩토링 간단히 진행했습니다!

감사합니다!

+앞서 질문사항 하나 남겼는데, 이 부분도 확인해 주시면 감사하겠습니다..!

hyunssooo

안녕하세요. 🙂
몇 가지 코멘트 남겼어요.
다음 step 진행하시면서 확인 부탁드립니다!

hyunssooo · 2024-03-27T14:22:17Z

src/main/java/racingcar/model/Position.java

+
+public class Position implements Comparable<Position>{
+


Comparable, equals, hashCode 👍
이 클래스도 VO로 만들기 적당한 것 같은데 어떻게 생각하시나요?

Car에서 Position을 final로 가지고 있게 하기 위해서 Position의 상태는 변화하도록 구성했었는데,

Position은 VO로 만들어 불변을 유지하고 Car가 가지고 있는 Position 필드에 final을 제거하는게 조금 더 자연스럽게 느껴지네요!
다음과 같이 변경해 봤습니다! 46e075c

hyunssooo · 2024-03-27T14:29:33Z

src/main/java/racingcar/model/Winners.java

+    public List<String> getNames() {
+        return names.stream()
+                .map(Name::getValue)
+                .collect(Collectors.toUnmodifiableList());
+    }


일반적으로 getter는 필드 그대로를 리턴합니다.
view를 위해서라면 Name 자체가 VO이니 view에 그대로 나가도 괜찮지 않을까요?

아하 저는 도메인(model) 객체가 view에 나가는 것은 지양해야 한다.
-> VO도 도메인의 범주에 들어간다
-> VO도 view로 빠져나가 안된다

의 흐름이었는데,
불변을 유지하는 VO가 밖으로 빠져나간다 해도 크게 사이드 이펙트가 발생할 일은 없겠군요..!

그러면 이러한 VO은 DTO의 역할도 함께 포함한다고 보면 되는 걸까요?

hyunssooo · 2024-03-27T14:31:49Z

src/main/java/stringcalculator/Calculator.java

+
+    private Integer getSum(final String[] inputNumbers) {
+        final List<Integer> numbers = Arrays.stream(inputNumbers)


큰 의미가 있는 코멘트는 아니였어요. Calculator#calculate 와 Calculator#getSum 리턴 타입 차이가 있어 남긴 코멘트였습니다!

이펙티브 자바 아이템 61. 박싱된 기본 타입보다는 기본 타입을 사용하라
가볍게 읽어보셔도 좋을 것 같아요!

hyunssooo · 2024-03-27T14:37:46Z

src/main/java/racingcar/model/RacingGame.java

+    public List<CarResponse> extractCarInfos() {
+        return cars.getValues().stream()
+                .map(CarResponse::new)
+                .collect(Collectors.toUnmodifiableList());
+    }


RacingGame 자체는 도메인 모델로 보이는데 CarResponse 와 같은 DTO를 의존해도 괜찮을까요? 이 부분은 함께 고민해 보면 재밌을 주제 같습니다. 개인적으로는 도메인 모델이 DTO를 의존하는 것은 선호하지 않는데요. 의존성 방향 때문이에요. 어떻게 생각하시는지 의견 남겨주시면 감사합니다~

의존성 방향에 대해서 아주 동의합니다! 이 부분은 step2 PR에서 조금 더 의견 남기겠습니다ㅎㅎ

hyunssooo · 2024-03-27T14:41:41Z

src/test/java/racingcar/model/CarsTest.java

+    void winnersTest() {
+        // given
+        final Cars cars = new Cars("lucas,dino,eve,helen");
+
+        // when
+        cars.step(new CustomNumberGenerator(List.of(9, 4, 2, 0)));
+
+        // then
+        assertAll(
+                () -> assertThat(cars.getWinners().getNames()).hasSize(2),
+                () -> assertThat(cars.getWinners().getNames()).contains("lucas"),
+                () -> assertThat(cars.getWinners().getNames()).contains("dino")
+        );
+    }


Cars#getWinners 는 Winners 객체를 리턴하는데 객체 비교로도 충분하지 않을까요?

final Winners winners = new Winners( List.of( new Name("lucas"), new Name("dino") ) ); assertThat(cars.getWinners()).isEqualTo(winners);

해당 테스트가 통과하려면 어떻게 코드를 변경해야 할까요?

예상되는 우승자 리스트(expectedWinner)를 만들어 객체끼리 비교하는게 가독성 측면에서도 더 좋아 보이네요.!

Winner의 equals를 재정의 하는 것 보단 usingRecursiveComparison()을 사용해 검증하는 방식으로 바꿔봤는데, 현수님이 의도하신 부분이 맞는지 모르겠네요..!
아니라면 바로 다시 알려주세요! 6154fc1

hyunssooo · 2024-03-27T14:44:46Z

src/test/java/racingcar/model/CarsTest.java

+        assertAll(
+                () -> assertThat(carsValues.get(0).getPosition()).isEqualTo(onePosition),
+                () -> assertThat(carsValues.get(1).getPosition()).isEqualTo(onePosition),
+                () -> assertThat(carsValues.get(2).getPosition()).isEqualTo(zreoPosition),
+                () -> assertThat(carsValues.get(3).getPosition()).isEqualTo(zreoPosition)
+        );


Q. assertAll과 assertSoftly 사용의 기준

제가 질문을 놓쳤네요. ㅠㅠ 저도 assertSoftly는 이번에 처음 알았네요.
말씀하신 것 처럼 softly에서는 실패한 검증문의 위치를 결과에 바로 알려준다 라면 굳이 assertAll을 사용하지 않아도 괜찮을 것 같아요! 취향 차이인 것 같습니다! 🙂

jjongwa and others added 19 commits March 25, 2024 14:46

docs(README): 구현할 기능 목록 작성

aa8c41f

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

feat: 이름(Name) 구현

7b48951

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

feat: 위치(Position) 구현

201d42c

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

feat: 자동차(Car) 구현

7a41ba6

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

feat: 자동차들(Cars) 구현

a14c2b2

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

feat: 경주게임(RacingGame) 구현

a1eb71c

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

feat: RacingGameApplication 구현

6c130ec

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

chore: 패키지 세분화

7617c57

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

refactor: CustomNumberGenerator를 통한 랜덤값 주입 메서드 테스트

9b556a2

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

feat: 시도 횟수(StepCount) 구현

2e63229

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

test: 게임 진행 후 우승자 반환 테스트 작성

a39ada0

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

feat: RacingGameApplication 구현

25e5c86

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

feat: Winners 구현

e610465

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

chore: 매직넘버, 메시지 상수화

0b9be17

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

refactor: 메서드 분리

cad92ee

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

feat: 문자열 덧셈 계산기 학습테스트 진행

51b6a55

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

refactor: 도메인 객체가 외부로 빠져나오지 않도록 변경

899dd26

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

refactor: CarInfo의 getter 리턴 값 변경

8ba301f

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

refactor: InputView 메서드 네이밍 변경

fb13669

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

hyunssooo requested changes Mar 26, 2024

View reviewed changes

jjongwa and others added 5 commits March 27, 2024 14:26

refactor(Calculator): Custom pattern 캐싱

a1a9f11

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

refactor: VO에 대해 equals & hashCode 재정의

d97e22c

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

refactor: isMove() 메서드 변경

8e029d6

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

chore: 클래스 이름 변경 및 패키지 이동

241fc80

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

refactor: 위치 비교에 대한 기능을 Position이 맡도록 변경

4bdee22

Co-authored-by: zeroberry <98147817+zeroberry@users.noreply.github.com>

hyunssooo approved these changes Mar 27, 2024

View reviewed changes

hyunssooo merged commit 9e150d6 into next-step:jjongwa Mar 27, 2024

jjongwa mentioned this pull request Mar 28, 2024

[2단계 - 자동차 경주 리팩터링] dino.shin(신종화) 미션 제출합니다. #47

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 자동차 경주 구현] dino.shin(신종화) 미션 제출합니다. #13

[1단계 - 자동차 경주 구현] dino.shin(신종화) 미션 제출합니다. #13

jjongwa commented Mar 26, 2024 •

edited

Loading

hyunssooo left a comment

hyunssooo Mar 26, 2024

jjongwa Mar 27, 2024

hyunssooo Mar 26, 2024

jjongwa Mar 27, 2024

hyunssooo Mar 26, 2024

jjongwa Mar 27, 2024

hyunssooo Mar 27, 2024

hyunssooo Mar 26, 2024

hyunssooo Mar 26, 2024

jjongwa Mar 27, 2024

hyunssooo Mar 26, 2024

jjongwa Mar 27, 2024

hyunssooo Mar 26, 2024

hyunssooo Mar 26, 2024

jjongwa Mar 27, 2024

jjongwa commented Mar 27, 2024

hyunssooo left a comment

hyunssooo Mar 27, 2024

jjongwa Mar 28, 2024

hyunssooo Mar 27, 2024

jjongwa Mar 28, 2024

hyunssooo Mar 27, 2024

hyunssooo Mar 27, 2024

jjongwa Mar 28, 2024

hyunssooo Mar 27, 2024

jjongwa Mar 28, 2024

hyunssooo Mar 27, 2024


		private final static Calculator CALCULATOR = new Calculator();


	private final static Calculator CALCULATOR = new Calculator();

	private static final Calculator CALCULATOR = new Calculator();


		final Matcher m = Pattern.compile(CUSTOM_REGEX).matcher(text);


		private Integer getSum(final String[] inputNumbers) {
		final List<Integer> numbers = Arrays.stream(inputNumbers)


		public class CustomNumberGenerator implements NumberGenerator{

[1단계 - 자동차 경주 구현] dino.shin(신종화) 미션 제출합니다. #13

[1단계 - 자동차 경주 구현] dino.shin(신종화) 미션 제출합니다. #13

Conversation

jjongwa commented Mar 26, 2024 • edited Loading

Q. assertAll과 assertSoftly 사용의 기준

hyunssooo left a 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

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

jjongwa commented Mar 27, 2024

hyunssooo left a 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

jjongwa commented Mar 26, 2024 •

edited

Loading