[1단계 - 자동차 경주 구현] 레디(최동근) 미션 제출합니다. #728

reddevilmidzy · 2024-02-15T08:50:54Z

안녕하세요 범블비!

레디라고 합니다:)

리뷰 받았으면 좋을 것 같은 부분 남겨보아요!

예외 커스텀
- 자동차 경주에서 사용자가 입력하는 값 중 발생할 수 있는 예외는 이름 입력할 때와 시도횟수 입력할 때가 있습니다.
- 이 부분에 대해서 IllegalArgumentException을 상속받아 예외를 커스텀 할지부터 예외 메시지를 던져줄때 자세하게 알려주는게 좋을지 아니면 크게 "유효하지 않은 자동차 이름입니다." 라는 형태로 사용하는게 좋을지 구민을 하였습니다.
- 예외를 커스텀하지 않은 이유에 대해서 설명해보자면 예외를 커스텀한다고 하여도 결국에 커스텀 하는 건 클래스 이름뿐이였습니다.
- 클래스 이름만 커스텀하는 것이 메시지를 다르게 출력하는 것보다 장점이 크게 없다고 보아 예외를 커스텀하지는 않았습니다.
- (질문을 작성하다보니 예외 클래스 커스텀은 자바에게 알려주고 메시지는 사용자에게 알려준다는 차이점이 있다는 것을 발견했네요:))
- 메시지를 사용하기로 정하고, 다시 고민했던 점은 메시지를 세분화해야할지에 대한 고민이였습니다.
- "유효하지 않은 자동차 이름입니다." 라는 형태가 "자동차 이름의 길이는 5를 초과할 수 없습니다" 보다 여러 군데에서 사용할 수 있다는 장점이 있어, 해당 형태를 택하였는데, 이렇게 사용자에게 무엇이 잘못됐는지 자세하게 알려주지 않아도 괜찮을까요?
예외 입력시 재입력 받는 기능을 구현하는 코드
- 이 기능을 구현하기 위해 while 문을 사용할지 재귀를 사용할지 페어(백호)와 갈등이 있었어요.
- 이번 프로그래밍 요구사항에서 한 메서드에 오직 한 단계의 들여쓰기(indent)만 한다.가 있어 이 조건을 맞추기 위해 결국 재귀를 사용하여 구현을 하였습니다.
- 하지만 재귀를 사용한다면 특정 값 이상 예외를 입력하면 스택 오버 플로우가 발생한다는 것을 확인하였습니다.
- 요약해보자면,, 프로그래밍 요구사항에 따를지 아니면 과감히 어겨보고 스택 오버 플로우에 안전한 코드를 작성할지 고민이 있습니다. (아니면 재귀를 쓰지않으면서 한 단계의 들여쓰기를 만족할 수 있는 방법도 있을까요??)
final과 관련된 질문
- 파라미터 값에도 final이 붙고 메서드 안에서도 final 이 붙어있는 변수들이 많이 있는데, 이렇게 하다보니 너무 과도하다는 생각이 들었습니다.
- 메서드 안에서의 final 과연 필요할까요??

ddaaac

안녕하세요 레디 👋

첫 번째 미션 구현 잘 해주셨습니다.

몇 가지 코멘트 남겼으니 다음 단계에 반영해주세요!

ddaaac · 2024-02-15T12:00:37Z

src/test/java/racingcar/model/CarNameTest.java

+        CarName carName = new CarName(name);
+        Assertions.assertThat(carName.getName()).isEqualTo(name);
+    }
+}


POSIX new line에 대해서 알아볼까요?

인텔리제이에서는 옵션으로 항상 개행을 붙여줄 수 있습니다 :)

테스트 패키지 안에 클래스만 개행이 추가가 안되고 있었는데 올려주신 링크 덕분에 해결했습니다!

ddaaac · 2024-02-15T12:10:58Z

src/main/java/racingcar/util/ExceptionRoofer.java

+
+public class ExceptionRoofer {
+
+    public static <T> T generate(final Supplier<T> supplier, final Consumer<String> consumer) {


제가 구현을 한다면 에러가 날 가능성이 있는 재귀를 사용하지는 않을 것 같네요.
depth를 1로 두면서 재입력을 받게 구현도 가능합니다. 이건 시간을 들여서 고민해보시면 좋을 것 같아요.

다만, 에러가 나면 그냥 에러가 나게 두는 것도 방법입니다. 특별한 요구사항이 없다면 재입력 없이 에러가 나게 둬도 좋을 것 같아요.

재입력 받고 싶은 욕심이 있었네요😅

ddaaac · 2024-02-15T12:11:51Z

src/main/java/racingcar/util/ExceptionRoofer.java

+public class ExceptionRoofer {
+
+    public static <T> T generate(final Supplier<T> supplier, final Consumer<String> consumer) {


람다를 파라미터로 받을 때의 가독성, 유지보수의 용이함 등을 고민해보시면 좋을 것 같습니다.

ddaaac · 2024-02-15T12:12:27Z

src/main/java/racingcar/dto/CarDto.java

+public record CarDto(String name, int position) {
+
+    public static CarDto from(final Car car) {
+        return new CarDto(car.getName(), car.getPosition());


이 미션에서 DTO가 필요할까요? DTO의 어떤 특징/장점을 살리고 싶어서 사용하셨나요?

DTO의 장점은 model과 view의 중간단계에서 model와 view가 서로 직접적으로 의존하지 않게 함으로써 변경에 쉽게 대응할 수 있다는 점이라고 생각합니다.

질문을 주신걸 보고 다시 코드를 살펴보니 현재 CarDto는 Car와 필드가 완전히 똑같기에 위의 장점을 살리지 못하고 있는거 같네요🤨

ddaaac · 2024-02-15T12:17:47Z

src/main/java/racingcar/model/CarName.java

+            throw new IllegalArgumentException(ErrorMessage.INVALID_CAR_NAME.get());
+        }
+
+        if (name.trim().length() > MIN_LENGTH) {
+            throw new IllegalArgumentException(ErrorMessage.INVALID_CAR_NAME.get());


저도 이번 미션에서는 표준 예외를 사용했을 것 같습니다. 이번 미션에서는 커스텀 예외를 사용하는 장점을 느끼기 힘들 것 같아요.

예외 내용의 상세함은 정책에 따라 달라지는 사항입니다. 보안이 필요하다면 자세하게 알려주지 않는 게 더 나을 때도 있겠죠. 이 부분은 레디가 미션에 맞게 고민해보시면 좋을 것 같아요. 다만, 많은 경우에서 예외 내용이 상세했을 때 얻는 이득이 더 컸던 것 같네요.

ddaaac · 2024-02-15T12:21:48Z

src/main/java/racingcar/model/CarName.java

+
+import racingcar.message.ErrorMessage;
+
+public class CarName {


값 객체를 사용해주셨네요.

값 객체를 사용했을 떄는 어떤 장점이 있을까요?

CarName이라는 명확한 이름이 생겼고, 자동차 이름은 5이하라는 제약조건을 자동차가 아닌 자동차 이름이 책임질 수 있어 응집도가 높아진다고 생각합니다!

ddaaac · 2024-02-15T12:22:16Z

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

+import java.util.stream.Collectors;
+import racingcar.message.ErrorMessage;
+
+public class Cars {


일급 컬렉션도 활용해주셨어요. 일급 컬렉션에는 어떤 장점이 있을까요?

비즈니스에 딱 맞는 자료구조를 만들 수 있습니다! 일급 컬렉션을 만들면 검증이나 요구사항을 외부에서 수행하는 것이 아니라 외부에서는 수행하라는 메시지만을 보내고 수행 로직은 일급 컬렉션이 스스로 결정할 수 있다는 장점이 있습니다.

ddaaac · 2024-02-15T12:23:43Z

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

+    public List<Car> findWinner() {
+        final int maxPosition = getMaxPosition();
+
+        return cars.stream()
+                .filter(car -> car.getPosition() == maxPosition)
+                .toList();
+    }
+
+    private int getMaxPosition() {
+        return cars.stream()
+                .mapToInt(Car::getPosition)
+                .max()
+                .orElseThrow(IllegalStateException::new);
+    }


지금 로직도 훌륭하지만 조금 더 극단적으로 getter를 사용하지 않는 훈련을 해봐도 좋을 것 같아요.

Car 객체에 적당한 책임을 부여하는 방식을 사용하면 getter를 사용하지 않아도 될 것 같아보여요.
두 Car 중에 winner가 누군지 찾는 로직, 두 Car 객체가 비겼는지 판단하는 로직 등을 만들어보는 건 어떨까요?

ddaaac · 2024-02-15T12:24:00Z

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

+        return cars.stream()
+                .mapToInt(Car::getPosition)
+                .max()
+                .orElseThrow(IllegalStateException::new);


예외에 메세지를 담아볼까요?

ddaaac · 2024-02-15T12:27:19Z

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

+    static class TestNumberGenerator implements NumberGenerator {
+
+        private static int value = 2;
+        @Override
+        public int generate() {
+            return value++;


꽤나 위험한 방식의 테스트인 것 같습니다.
테스트가 추가/제거될 때 TestNumberGenerator 때문에 다른 테스트가 깨질 수도 있을 것 같아요.

항상 움직이는 Generator와 항상 움직이지 않는 Geneartor를 만들어서 테스트에 사용하면 테스트도 명확해지고 테스트 간의 연관관계도 끊어낼 수 있을 것 같습니다.

reddevilmidzy

남겨주신 질문에 대한 답변 남깁니다!

reddevilmidzy · 2024-02-16T08:19:32Z

src/test/java/racingcar/model/CarNameTest.java

+        CarName carName = new CarName(name);
+        Assertions.assertThat(carName.getName()).isEqualTo(name);
+    }
+}


테스트 패키지 안에 클래스만 개행이 추가가 안되고 있었는데 올려주신 링크 덕분에 해결했습니다!

reddevilmidzy · 2024-02-16T08:20:32Z

src/main/java/racingcar/model/Car.java

+    public Car copy() {
+        return new Car(name, position);
+    }


좋은 의견 감사합니다!!

reddevilmidzy · 2024-02-16T08:28:46Z

src/main/java/racingcar/dto/CarDto.java

+public record CarDto(String name, int position) {
+
+    public static CarDto from(final Car car) {
+        return new CarDto(car.getName(), car.getPosition());


DTO의 장점은 model과 view의 중간단계에서 model와 view가 서로 직접적으로 의존하지 않게 함으로써 변경에 쉽게 대응할 수 있다는 점이라고 생각합니다.

질문을 주신걸 보고 다시 코드를 살펴보니 현재 CarDto는 Car와 필드가 완전히 똑같기에 위의 장점을 살리지 못하고 있는거 같네요🤨

reddevilmidzy · 2024-02-17T08:31:51Z

src/main/java/racingcar/util/ExceptionRoofer.java

+
+public class ExceptionRoofer {
+
+    public static <T> T generate(final Supplier<T> supplier, final Consumer<String> consumer) {


재입력 받고 싶은 욕심이 있었네요😅

reddevilmidzy · 2024-02-18T03:44:33Z

src/main/java/racingcar/model/CarName.java

+
+import racingcar.message.ErrorMessage;
+
+public class CarName {


CarName이라는 명확한 이름이 생겼고, 자동차 이름은 5이하라는 제약조건을 자동차가 아닌 자동차 이름이 책임질 수 있어 응집도가 높아진다고 생각합니다!

reddevilmidzy · 2024-02-18T03:49:37Z

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

+import java.util.stream.Collectors;
+import racingcar.message.ErrorMessage;
+
+public class Cars {


비즈니스에 딱 맞는 자료구조를 만들 수 있습니다! 일급 컬렉션을 만들면 검증이나 요구사항을 외부에서 수행하는 것이 아니라 외부에서는 수행하라는 메시지만을 보내고 수행 로직은 일급 컬렉션이 스스로 결정할 수 있다는 장점이 있습니다.

Arachneee added 30 commits February 14, 2024 15:56

docs: 구현 기능 목록 작성

e2724e8

feat: 전진 여부 결정하기 기능 구현

e594dfc

feat: 랜덤 값 추출하기

09b9130

chore: 패키지 변경

7c5212f

feat: 이름 입력 받기

8a3ec78

feat: 이름 문자 쉼표 기준으로 구분하기

722ec73

test: 자동차 이름 정상 등록 및 예외

eea9787

feat: 시도 횟수 입력받기

bb92d84

feat: 시도 횟수 문자거나 음수 예외

d063aab

docs: 구현 목록 추가

54ccbe0

style: tryCount 이름 변경

3aa1ca7

feat: 라운드 진행 기능 추가

1b83d99

test: 라운드 진행 기능 추가

a936c59

feat: 라운드 반복하기

5686bc3

feat: 실행 결과 출력하기

d6fdccb

feat: 최종 우승자 판단하기

bb06565

feat: 최종 우승자 출력

9ae80d1

feat: 최종 위치 출력

62cf854

refactor: CarName 클래스 추출

eed1e8b

feat: 잘못된 입력시 재입력 기능

d01d79b

feat: 올바르지 않은 구분자 확인

d04b78d

docs: 구현 기능 추가

e5f413b

feat: 공백 제외 5글자 예외

6043425

feat: 중복 이름시 예외

24adf45

fix: 이름 중복시 예외 버그 수정

61e994f

feat: 예외 메시지 출력하기

265101d

test: 랜덤 한 값 생성

fcaa5dd

test: cars의 getter 불변성 확인

07d371d

refactor: 예외 시 재입력 클래스 분리

f5593c8

refactor: 메소드명 변경

7b7529e

Arachneee added 3 commits February 15, 2024 15:51

refactor: 메소드 분리

c04b90f

test: 자동차 전진

e3b6b40

refactor: final 추가

9e5eb50

ddaaac approved these changes Feb 15, 2024

View reviewed changes

ddaaac merged commit 57ae5d3 into woowacourse:reddevilmidzy Feb 15, 2024

reddevilmidzy commented Feb 18, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[1단계 - 자동차 경주 구현] 레디(최동근) 미션 제출합니다. #728

[1단계 - 자동차 경주 구현] 레디(최동근) 미션 제출합니다. #728

reddevilmidzy commented Feb 15, 2024

ddaaac left a comment

ddaaac Feb 15, 2024

reddevilmidzy Feb 16, 2024

ddaaac Feb 15, 2024

reddevilmidzy Feb 17, 2024

ddaaac Feb 15, 2024

ddaaac Feb 15, 2024

reddevilmidzy Feb 16, 2024

ddaaac Feb 15, 2024

ddaaac Feb 15, 2024

reddevilmidzy Feb 18, 2024

ddaaac Feb 15, 2024

reddevilmidzy Feb 18, 2024

ddaaac Feb 15, 2024

ddaaac Feb 15, 2024

ddaaac Feb 15, 2024

reddevilmidzy left a comment

reddevilmidzy Feb 16, 2024

reddevilmidzy Feb 16, 2024

reddevilmidzy Feb 16, 2024

reddevilmidzy Feb 17, 2024

reddevilmidzy Feb 18, 2024

reddevilmidzy Feb 18, 2024


		public class ExceptionRoofer {

		public static <T> T generate(final Supplier<T> supplier, final Consumer<String> consumer) {


		import racingcar.message.ErrorMessage;

		public class CarName {

[1단계 - 자동차 경주 구현] 레디(최동근) 미션 제출합니다. #728

[1단계 - 자동차 경주 구현] 레디(최동근) 미션 제출합니다. #728

Conversation

reddevilmidzy commented Feb 15, 2024

ddaaac 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

reddevilmidzy 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