Final practice #2387

zangsu · 2023-11-21T08:37:42Z

No description provided.

zxcev · 2023-11-21T11:12:02Z

src/main/java/racingcar/Application.java

 public class Application {
+    private static final InitController initController = new InitController();
+    private static final PlayController playController = new PlayController();
+
    public static void main(String[] args) {


흐름을 한 눈에 볼 수 있도록 Application은 SpringBoot의 Application 파일과 마찬가지로 코드를 최소화 하고,

하나의 Controller로 입력-처리-출력을 위임하는 로직을 모으면 어떨까 싶습니다.

이 부분에 대해 정~~말 많은 고민이 아직까지도 진행중인데요.
조금 더 정확히는, 일반적인 웹 MVC와는 흐름 자체가 조금 다르다 보니, 전체 어플리케이션 흐름을 관리하는 역할을 어느 객체에게 맡겨야 할지가 고민입니다.

자동차 게임의 경우 그래도 어플리케이션이 비교적 단순하지만, 크리스마스와 같이 책임이 여러 가지로 구성되어 있는 상황에서 전체 흐름을 하나의 Controller가 관리하는 것이 맞는지를 아직 잘 모르겠습니다...ㅎㅎ

많은 분들의 코드를 보니 하나의 흐름 제어를 위한 MainController를 두는 방법도 있고, 흐름 제어를 Application.main()에 맡기는 방법도 있더라고요.

@zxcev 님은 하나의 컨트롤러가 전체의 흐름을 제어하고, 각각의 책임은 세부 책임을 가지는 도메인 객체의 메서드를 Controller에서 호출하는 것으로 구현하신거죠?

네. 간단한 프로그램의 경우 굳이 Service를 만들지 않고,

크리스마스같이 도메인 및 처리할 로직이 다수인 경우는 Service를 만들게 되더라구요.

우아한 테크코스에서 제출하는 미션이 모두 입력-처리-출력 3단계로 맞아 떨어지지 않고,

조금씩 벗어나는 예외 시 재입력, 상태 선택, 처리 도중 출력 등의 패턴이 종종 보이기도 하는데

대부분의 경우, 입력-처리-출력 3단계에 가까운 형태로 만들 수 있었고,

이렇게 구조를 정해놓는 경우 하향식으로 코드를 작성할 때, 틀 잡기가 좀 더 수월했으며

전체 흐름은 Controller에서, 핵심 로직은 Service에서, 입출력 및 변환은 InputView에서 빠르게 확인할 수 있어서 좋았습니다.

요청 및 응답이 1:1로 대응되는 웹 MVC와 흐름이 완전히 같지는 않고, CLI 환경에서 작업을 해본 적이 없어서 더 좋은 방법이 분명히 있겠지만, 단기간에 하기엔 익숙한 형태에 접목시키는 것이 더 수월할 것이라고 생각했습니다.

미션에 직접 사용했던 크리스마스 및 로또 Controller 쪽 코드는 아래와 같습니다.

// 크리스마스 미션 public final class OrderController { // ... public void run() { final OrderDiscounting orderDiscounting = inputView.inputOrder(); final OrderDto orderDto = orderService.order(orderDiscounting); outputView.printOrderDto(orderDto); } }

// 로또 미션 /** * 입력, 실행, 출력 3단계의 흐름 제어 역할을 맡는 Controller 계층입니다. * * 실제 로직이 작성된다기보다는, 전체적인 흐름 및 기능 목록을 한 눈에 볼 수 있도록, * * 하위 계층에게 대부분의 처리를 위임하여 적은 코드량을 유지하여 가독성을 높이는 것이 좋다고 생각했습니다. */ public final class LottoController { private final InputView inputView; private final OutputView outputView; private final LottoService lottoService; public LottoController( final InputView inputView, final OutputView outputView, final LottoService lottoService ) { this.inputView = inputView; this.outputView = outputView; this.lottoService = lottoService; } /** * 로또 프로그램은 총 2회의 입출력으로 이루어져 있음. 첫 번째는 금액 입력 및 로또 구매 후, 구매한 로또 번호 출력 두 번째는 당첨 번호 및 보너스 번호 입력 및 당첨 결과 확인, 그리고 당첨 통계 * 출력 * * 이를 2개의 메소드로 나누어 흐름을 보기 좋게 정리 */ public void run() { buyLottos(); drawLottos(); } /** * 첫 번째 입출력을 처리 * * 1. 로또 구매 금액 입력을 받는다. * * 2. 로직 처리를 서비스에 위임한다 * * 3. 로또 구매 결과를 출력한다. */ private void buyLottos() { final BuyLottosInput inputDto = inputView.inputBuyLottos(); final BuyLottosOutput outputDto = lottoService.buyLottos(inputDto); outputView.printLottosBuyingResult(outputDto); } /** * 두 번째 입출력을 처리 * * 1. 로또 당첨 번호 및 보너스 번호를 입력 받는다. * * -> 웹 페이지로 생각하면, 로그인 시에 이메일을 입력하고 다음 버튼을 누르면 페이지 이동이 아닌, 클라이언트 사이드 렌더링이 이루어 진 뒤, url 이동 없이 비밀번호 입력 창으로 살짝 전환되는 * 느낌으로 하나의 페이지에서 2개의 입력을 모아서 받는다고 생각하여 1개의 컨트롤러에서 처리. 2. 로직 처리를 서비스에 위임한다 3. 로또 구매 결과를 출력한다. * * 2. 저장된 모든 로또에 대한 추첨 로직 처리를 서비스에 위임한다. * * 3. 로또 당첨 통계 결과를 출력한다. */ private void drawLottos() { final DrawLottosInput inputDto = inputView.inputDrawLottos(); final DrawLottosOutput outputDto = lottoService.drawLottos(inputDto); outputView.printLottosDrawingResult(outputDto); } }

정답이 아닌 하나의 방법일 뿐이며, 한번 시도해보시면 인사이트를 얻을 수도 있겠다 싶어서 말씀드렸습니다.😊

@zangsu 저도 간단하게 의견을 공유하고 싶어요.

프로젝트가 고도화되고 규모가 확장되면 Controller 분리를 고민해야 하는 시점이 찾아온다고 생각해요. 그런데 이번 6기 프리코스에서 출제된 미션은 모두 하나의 목적을 구현하는 방식이라고 이해했습니다.

숫자 야구(승자 출력), 자동차 경주(승자 출력), 로또(당첨 여부 출력), 크리스마스(예약 내역 출력)

따라서 하나의 Controller에서 구현했고 책임이 과도해질 때 Service로 분리하는 방식으로 구현하고 있어요. 물론 최종에서 여러 가지 목적을 구현해야 하는 미션이 출제될 수도 있지만 고민하고 계신 부분에 도움이 되고 싶어 말씀드려 봅니다. 😄

@Hugh-KR
동의합니다!
생각해 보니 우리가 만나게 될 어플리케이션의 관심사는 한 문장으로 요약할 수 있는, 규모가 크지 않은 어플리케이션이군요!

덕분에 앞으로는 하나의 Controller에서 각각의 메서드로 동작만 구분시켜 구현을 하게 될 것 같습니다!

zxcev · 2023-11-21T11:14:54Z

src/main/java/racingcar/view/InputView.java

+    private final Reader reader = new Reader();
+    private final Writer writer = new Writer();
+
+    public List<String> getNames() {


InputView에서는 자동차 이름을 List<String>으로 입력 받고,

이를 InitController에서 호출하여 new Cars의 인자로 전달한 뒤,

Cars 생성자 내에서 검증 및 도메인 객체로의 변환이 이루어지고 있는데,

InputView에서 new Cars를 호출하여 도메인 객체를 Controller에 바로 변환해서 던져주면 좀 더 간결한 코드 작성이 가능할 것 같습니다.

시간 단축 및 코드 간소화 관점에서 작성한 리뷰입니다.

이 부분은 그렇다면 어느정도의 타협을 고려한 리뷰인거죠?
시간 단축을 고려한다면 이 방법 역시 나쁘지 않을 것 같네요!

그런데 만약, 예외 발생시 "해당 입력"부터 다시 진행해야 한다는 3~4주차의 요구사항을 고려한다면 예외 처리 로직 역시 View에서 사용되어야 할 것 같고, 그렇다면 View에 너무 많은 로직이 사용되지 않을까? 라는 생각 역시 드는데요.

이 부분을 고려하더라도 시간 단축을 위해서라면 타협할 만한 부분이라고 생각하시는 건가요?

네. 시간 단축을 위해 생각해보니 현재까지는 이 방법이 괜찮아 보입니다. 나중에 바뀔 수도 있겠지만요.

이렇게 작성할 경우 말씀하신대로 View에서 출력, 입력, 검증, 변환, 재입력 처리 등의 다양한 로직이 너무 남발되기는 합니다.

그래서 세부 로직은 마치 Service가 다양한 Domain에게 세부 로직 처리를 위임하듯,

Validator, Converter, RetryHandler 등에게 세부 처리를 위임하는 식으로 만들어보니 InputView가 크게 방대해지지는 않아서 지금은 이렇게 타협을 보고 있습니다.

이렇게 작성할 경우 재입력 로직을 처리하는 방법에 대해서 생각해본 내용입니다

예외 발생 시 실패한 부분부터 다시 입력을 받는 패턴이 여러 미션을 아울러 상당히 자주 나오는 것 같아서

이를 대비하여 아래와 같이 RetryHandler를 따로 분리해서 템플릿화 해두고,

View에서는 예외 발생 시 재입력하는 로직을 위임만 해주면 재입력 로직을 조립해주면 되기 때문에

// RetryHandler.java package christmas.common.exception; import io.writer.Writer; import java.util.function.Supplier; /** * 입력 도중 오류가 발생할 경우, 자동으로 재시도를 요청합니다. * * 재귀 호출 방식으로 작동하며, * * StackOverflow를 방지하기 위해 최대 재시도 횟수는 기본 100으로 제한되어 있습니다. * * 오류 발생 시, 메세지를 자동으로 출력해주며, "[ERROR]"를 앞에 공통으로 붙여줍니다. */ public final class RetryHandler implements Handler { /** * StackOverflow를 방지하기 위해, 최대 재시도 횟수를 제한합니다. */ public static final int MAX_RETRY_COUNT = 100; private final Writer writer; /** * 현재 재시도 횟수입니다. */ private int retryCount = 0; public RetryHandler(final Writer writer) { this.writer = writer; } /** * 주어진 콜백 함수가 성공할 때까지 재귀적으로 재실행합니다. */ public <T> T retryUntilSuccess(final Supplier<T> supplier) { try { final T result = supplier.get(); // 결과를 성공적으로 반환한 경우, 재시도 횟수를 초기화 한 뒤 결과를 반환합니다. retryCount = 0; return result; } catch (final IllegalArgumentException | IllegalStateException exception) { final String formattedMessage = String.format("[ERROR] %s\n", exception.getMessage()); writer.write(formattedMessage); exitIfReachedMaxCall(exception); return retryUntilSuccess(supplier); } } private void exitIfReachedMaxCall(final RuntimeException exception) { if (retryCount++ == MAX_RETRY_COUNT) { throw exception; } } }

public final class InputView { // ... public OrderDiscounting inputOrder() { new HeaderComponent().renderTo(writer); final Date date = retryHandler.retryUntilSuccess(this::readDate); final OrderMenus orderMenus = retryHandler.retryUntilSuccess(this::readOrderMenus); return OrderDiscounting.of(date, orderMenus); } /** * 사용자가 입력한 날짜를 읽어옵니다. * * 숫자 변환은 InputParser에게, * * 도메인 객체로의 변환은 Date에게 위임합니다. * * 이 때, 변환을 담당하는 InputParser와 Date가 각각 입력 검증 및 요구 사항 검증까지 처리합니다. * * 정확히는 InputParser는 검증을 InputValidator에게 위임하며, * * Date는 검증 로직이 범용적이지 않고 적으므로, 내부에 검증 로직을 포함하고 있습니다. */ public Date readDate() { new InputDateComponent().renderTo(writer); final int dayOfMonthInt = inputParser.toDayOfMonthInt(reader.readLine()); return Date.from(dayOfMonthInt); } /** * 사용자가 입력한 메뉴를 읽어옵니다. * * 입력된 단순 문자열을 중간 변환하는 역할은 InputParser에게 위임하며, * * 도메인 객체로의 변환은 Menus에게 위임합니다. * * InputParser는 검증을 InputValidator에게 위임하며, * * Menus 및 Menu의 검증은 MenuValidator에게 위임합니다. */ public OrderMenus readOrderMenus() { new InputMenusComponent().renderTo(writer); final List<Pair<String, Integer>> menusPairs = inputParser.toOrderMenusPairs(reader.readLine()); return OrderMenus.from(menusPairs); } // .... }

이런 패턴을 실제로 시험에서 만나게 된다면 RetryHandler를 템플릿으로 찍어내고 블록 맞추듯 끼워서 쓰면 시간 단축에 약간이나마 도움이 될 것 같습니다.

zxcev · 2023-11-21T11:16:25Z

src/main/java/racingcar/view/InputView.java

+        return reader.readStrings(RacingConst.DELIMITER);
+    }
+
+    public int getRoundNumber() {


이 부분 역시 InitController에서 getRoundNumber를 그대로 호출을 한 번 래핑하고 있는데,

래핑 없이 Controller에 그대로 꽂아서 사용해도 좋겠습니다.

확실히 시간 단축을 고려하지 않다 보니 메서드 호출 레벨을 맞추기 위한 메서드 래핑이 몇 군데 존재하는 것 같습니다!
저도 인지하지 못하고 있었네요.

이 부분을 고려하여 앞으로의 구현에서는 이 부분은 리팩토링의 부분으로 미루고, 리팩토링 중에서도 우선순위를 뒤로 미루어도 좋을 것 같습니다!
좋은 의견 감사합니다!!

zxcev · 2023-11-21T11:19:14Z

src/main/java/racingcar/view/OutputView.java

+        writer.printMessage("실행 결과");
+    }
+
+    public void printRountResult(List<Car> cars) {


출력 로직 깔끔합니다.

OutputView에 첫 호출임을 명시하는 flag 변수를 하나 만들고 printResultTitle 내에서 flag를 체크한 뒤,

첫 호출 때만 printResultTitle을 호출하도록 만들면 출력에 대한 복잡도를 OutputView가 스스로 짊어지게 되어,

외부에서 출력 양식을 신경쓰지 않아도 되서 전체 흐름을 봐야하는 Controller 쪽이 조금이나마 더 간결해지겠습니다.

(찾아보니 printResultTitle은 Application에서 사용되었네요.)

해당 방법은 대신 View에서 상태 저장을 위한 내부 변수를 가지고 있어야 하기에 일반적으로 정적 리소스로 사용되는 View의 부분과는 조금 다른 점이 생긴다고 생각하는데요.

저는 정적 리소스로 사용되는 View에서는 변수를 가지지 않는 것이 가장 바람직하다고 생각해 이렇게 구현한 것 같습니다.

맞아요. Controller의 코드를 정돈된 형태로 유지하기 위한 임시 방편이라 나쁜 구조라고 생각합니다.

시험 때 일회성으로 사용할 법한 패턴 정도로만 보고 지나가면 좋을 것 같습니다.

zxcev · 2023-11-21T11:19:31Z

src/main/java/racingcar/exception/RacingException.java

@@ -0,0 +1,26 @@
+package racingcar.exception;
+
+public enum RacingException {


zxcev · 2023-11-21T11:20:39Z

src/main/java/racingcar/domain/PlayController.java

+import racingcar.domain.car.Cars;
+import racingcar.view.OutputView;
+
+public class PlayController {


이전 리뷰와 이어지는 내용입니다.

Controller는 입력, 처리, 출력의 전체 흐름이 모두 보이는 것이 일반적이기 때문에,

InitController - 입력 위임

Application - 처리 위임

PlayController - 출력 위임

구조를 하나의 Controller에 모아주면 좋을 것 같습니다.

june-777

많이 배워가는 리뷰였습니다 😊
최종 코테까지 파이팅입니다!

june-777 · 2023-11-21T15:07:58Z

docs/README.md

커밋에 대한 리뷰입니다!
최종 코딩테스트에선 1 ~ 4 주차 모든 공통 피드백 및 기능 요구사항을 확인할 것 같습니다!
기능 단위 커밋도 시간이 오래걸리는 일이기 때문에, 이 점도 함께 연습하시면 시간 관리 연습에 도움이 되실 것 같습니다 😊

맞아요, 이번 구현에서 정말 아~무 생각 없이 구현을 시작하다 보니 놓쳤던 부분이라 저도 뼈저리게 아쉬운 부분입니다ㅠㅠ
감사합니다!

june-777 · 2023-11-21T15:09:54Z

src/main/java/racingcar/domain/InitController.java

+import racingcar.domain.car.Cars;
+import racingcar.domain.numbergenerator.RandomNumberGenerator;
+import racingcar.view.InputView;
+


InitController를 구현하신게 상당히 신박한 방법같습니다!
약간 repository 느낌도 나는데, 시간 관리를 염두하여 해당 방식으로 구현하신걸까요?

4주간의 미션을 진행하며 Controller의 책임에 대한 리뷰도 많이 받았는데요.
이번 미션 구현에서는 "각각의 책임들을 Controller에서 부터 구분하여 진행을 해 보자" 라는 생각으로 임했습니다.
다만, 자동차 게임의 경우 3~4주차 미션에 비해 조금은 단순한 어플리케이션이기에 로직 상의 책임을 나누기는 어려웠고, 게임 준비 / 게임 진행 의 두 가지 책임으로 구분하며 만들어진 Controller입니다!!

시간 관리를 생각한다면 하나의 Controller가 모든 로직의 흐름을 관리하는 것이 더 좋은 방법일 수 있을 것 같네요!!

june-777 · 2023-11-21T15:11:39Z

src/main/java/racingcar/Application.java

+        Cars cars = initController.initCars();
+        int rounds = initController.getRounds();
+        initController.closeConsole();
+


충분히 분리하려면 분리하셨을 것 같은데, 시간 관리를 염두하여 main 문에서 처리하도록 하신걸까요 ?!
rounds 변수명을 totalRound로 바꾸면 조금 더 가독성이 좋아질 것 같습니다 😊

이 부분을 분리한다면 어떤 식으로 분리를 생각하셨을까요??
저는 이번 미션에서는 Application이 전체 흐름을 관리하고, 각각의 Controller부터는 해당 Controller의 관심사에 해당하는 책임만을 신경쓰도록 구현해 보았고, 그 결과 main() 함수에서부터 로직의 흐름이 일부 존재하게 되었습니다.

이 부분은 저 역시 아직은 기준을 정하지 못한 부분이라 고민이 많이 되네요....ㅎㅎ

rounds 변수명에 대한 리뷰는 동의합니다!
아직도 네이밍에는 많이 약하군요...ㅎㅎ

해당 로직에서 제 생각을 말씀드리자면.

InitController, PlayController 를 통합하고 현재 main에 있는 로직을 모두 컨트롤러 내부에서 정의해서 사용하면 좋을 듯 합니다.

통상적인 Controller의 역할과도 부합하고 2개 생성되고 있는 컨트롤러의 생성 비용을 1개로 줄일 수 있으니까요.

Application 클래스에서 직관적으로 동작을 보여주고 싶다면 통합된 Controller에서 Init, Play 메서드로 분리해서 출력하면 Controller 생성, Init, Play 메서드 3개 로 main을 구성할 수 있습니다.

현재도 분리는 정말 잘하셨는데 고민하고 계신 것같아 코멘트 납깁니다. 😄

아직 "Controller와 Application 중 어느 곳이 전체 흐름을 총괄하는 데 더 적합한가?" 에 대한 결정은 내리지 못했습니다만, 만약 Controller 내부에서 전체 흐름을 총괄한다면 창혁님 말씀처럼 하나의 컨트롤러에 모든 로직을 집중시킬 것 같습니다!

저는 이번 미션에서는 Application이 전체 흐름을 관리하고, 각각의 Controller부터는 해당 Controller의 관심사에 해당하는 책임만을 신경쓰도록 구현해 보았고, 그 결과 main() 함수에서부터 로직의 흐름이 일부 존재하게 되었습니다.

어떤 의도로 구현하셨는지, 의도를 알게 되었네요!!

저는 크리스마스 프로모션 미션을 진행하며, FrontController에서 전체 흐름을 담당하게 하고 ChristmasController는 Service Layer만 호출하여 응답을 반환하는 구조로 진행했었습니다.
ChristmasController가 굉장히 얇아지는 장점도 있었지만, 일반적인 웹 개발처럼 repository가 없다보니 FrontController에서도 Domain 객체를 알고있게 되는 아쉬움이 뒤섞이더라구요. 전체 흐름을 관리하는 부분이 아직까지 참 고민이 많은 부분이네요 🤔🤔

june-777 · 2023-11-21T15:14:52Z

src/main/java/racingcar/exception/RacingException.java

+    public String getMessage() {
+        return PREFIX + message;
+    }
+


예외 자체를 Enum에서 던지도록 구성하신게 굉장히 인상 깊습니다! 👍
정말 좋은 방식인 것 같아요 배워갑니다!!!

정확히 말하자면 "예외 생성"의 책임을 enum에서 가지고 있으며 예외를 실제로 발생시키는 것은 각각의 예외 상황을 체크하는 validate 로직 내부입니다!
이 점을 다시 한번 짚는 이유는, enum 내부에서 예외를 던진다면, 해당 메서드를 호출하는 validate 로직이 반환 값이 필요한 상황에서는 예외가 던져 진 이후에도 반환값을 명시해 주어야 하기 때문이에요!

public int parseInt(String input){ try{ return Integer.parseInt(input); } catch (NumberFormatException e) { RacingException.throwException(); //내부적으로 예외를 throw 하는 메서드 return 0; //이 곳, 또는 try-catch문 외부에 또 하나의 return 값이 필요해 집니다! } }

혹여나 해당 방법을 차용하실 경우를 대비해 제가 겪었던 문제를 공유해 드립니다 ㅎㅎ

june-777 · 2023-11-21T15:17:16Z

src/main/java/racingcar/view/OutputView.java

+import racingcar.RacingConst;
+import racingcar.domain.car.Car;
+import racingcar.view.io.Writer;
+


실행 결과, %s, %s, 최종 우승자같은 출력화면들은 상수로 관리하면 추 후 디버깅 하실 때, 시간 관리 측면에서 도움되실 것 같아요 😊

맞아요, 이 부분 역시 아직 고민을 하는 중인데요.
상수 관리에 드는 시간 비용 VS 디버깅 도중 출력 형식에 문제가 생겨 수정을 해야하는 시간 비용
을 비교해 본다면 상수 추출이 더 적합한 방법일 수도 있겠네요!

굉장히 현명한 고민이라고 생각해요. 그런데 하드코딩과 매직넘버는 지양하라 는 공통 요구사항에 포함된다고 인지하고 있어서 함께 고민하면 좋을 듯합니다.

@Hugh-KR 해당 의견에도 동의합니다!
요구사항 및 피드백으로 나왔던 내용들은 최대한 신경을 쓰는 편이 좋을 것 같군요

june-777 · 2023-11-21T15:20:05Z

src/main/java/racingcar/view/io/Reader.java

+import java.util.Arrays;
+import java.util.List;
+import racingcar.exception.RacingException;
+


입력 검증 로직들이 굉장히 깔끔하네요 👍

약간 번외로
"01", "0000002"와 같이 0으로 시작하는 문자는 예외 처리가 합당하다 생각하시는지,
터무니 없이 긴 입력을 방지하기 위해 입력 최대 길이에 대한 검증을 수행하는건 어떻게 생각하시는지 여쭙고 싶습니다!

기본적으로는 "사용자의 편의"를 위해서 터무니 없는 값의 입력이 아니라면 해당 입력을 최대한 정상적인 입력으로 처리해 주어야 한다고 생각합니다.
"01"과 같은 값들이 이에 해당하죠.

터무니 없이 긴 입력 (예를 들면 Integer.MaxValue를 넘어가는 입력) 에 대해서는 검증 후 예외를 발생 시켜 주는 것도 좋을 것 같네요!
이런 부분에 대한 예외처리는 어떤 식으로 하면 좋을지 고민해 봐야 겠습니다!

june-777 · 2023-11-21T15:21:05Z

src/main/java/racingcar/view/io/Writer.java

+    public void printMessage(String message) {
+        System.out.println(message);
+    }
+


출력 로직쪽에 이거 하나 만들어두면 굉장히 편리할 것 같네요. 배워갑니다!! 👍

해당 메서드는 메서드의 호출 레벨을 맞춰주기 위한 메서드 래핑정도로 생각해 주시면 좋을 것 같습니다!
4주동안의 미션을 진행하며 저한테는 익숙해 진 로직이라 자연스럽게 이렇게 작성하게 되었는데 시간 관리의 측면에서는 이점을 가질지, 아니면 해가 될지는 잘 모르겠네요...ㅎㅎ

그래도 어짜피 템플릿 사용을 고려한다면 이런 부분은 미리 작성해 두어도 좋을 것 같습니다!

메서드화 해서 사용하는게 깔끔하겠다는 생각이 문득 드네요. 눈에 슬쩍 담아가겠습니다. 😄

june-777 · 2023-11-21T15:24:40Z

src/main/java/racingcar/domain/numbergenerator/NumberGenerator.java

@@ -0,0 +1,10 @@
+package racingcar.domain.numbergenerator;
+


default method를 구현하신 이유가 궁금합니다!

처음 제가 구현했던 방법은 이동에 사용되는 "숫자 자체"를 도메인에서 사용하는 방법이었습니다!
그리고, 이 "숫자를 생성하는 방법"에 대해 추상화를 진행했던 거고요!

리팩토링을 하며 이동에 사용되는 숫자를 클래스로 래핑하게 되었는데요. 그러나 이 상황 역시 "숫자 생성"에 대해서만 추상화를 제공하고, 생성된 숫자를 "MoveNumber"로 만드는 부분 부터는 중복되는 것으로 보였습니다.
그래서 기존의 숫자를 생성하는 Generator들을 그대로 두고 default 메서드로 해당 숫자를 사용해 MoveNumber 객체를 생성하는 부분만 새로 구현해 주었습니다!

june-777 · 2023-11-21T15:28:14Z

src/test/java/racingcar/domain/car/CarTest.java

+import org.junit.jupiter.params.provider.ValueSource;
+import racingcar.domain.numbergenerator.MoveNumber;
+import racingcar.exception.RacingException;
+


테스트 코드가 전반적으로 굉장히 깔끔하네요 👍

june-777 · 2023-11-21T15:44:35Z

src/main/java/racingcar/domain/car/Cars.java

+            throw RacingException.NO_CAR_FOUNDED.makeException();
+        }
+    }
+


설계에 대해 의견을 나누고 싶어 코멘트 드립니다!!

레이싱 차들을 움직이는 협력 에 대한 설계가 아래와 같은 구조인 것으로 보입니다.

위의 구조도 충분히 좋다고 생각됩니다! 아래와 같은 설계는 어떻게 생각하시는지 여쭤보고 싶어요!

Refactor Ver1

지극히 개인적인 의견으로는 번호를 생성해서 자동차가 움직이는 일련의 과정들은 자동차 본연의 책임이라는 생각도 드는데, 어떻게 생각하시는지 궁금해요!

장점으로는 Cars와 NumberGenerator 간의 의존 관계를 제거할 수 있는 측면이 있을 것 같고,

단점?으로는 Car에게 책임이 조금 더 집중되는 트레이드 오프가 있을 것 같습니다!

더 나은 설계인지는 저도 물음표네요 ㅎㅎ,, 그저 설계에 대한 다양한 의견들을 나누고 싶어 코멘트 드립니다 😊

정말 "좋은 설계" 에 대한 고민은 정답도 없을 뿐더러, 각각의 설계에 대한 장단점이 존재하기에 너무 어려운 것 같아요...ㅎㅎ

제가 위와 같이 설계를 하게 된 이유는 "자동차가 움직인다" 라는 기능을 테스트 할 때 외부에서 값을 받아 오는 편이 더 테스트 하기 쉬울 것이라 생각했기 때문이에요.
그래서 테스트 코드는 단순히 "~~의 값을 전달받으면 이동한다" 와 같이 자동차의 움직임을 더 쉽게 테스트 할 수 있었고요.

다만, @june-777 님의 의견처럼, 제 설계가 "이동의 책임을 가지고 있는 자동차가 자기 책임을 온전히 다하지 않는다"고 생각이 되기도 하네요.
Car 객체 자체가 NumberGenerator를 가지고 있다 할지라도, 고정된 값을 생성해 주는 FixedValueGenerator같은 것을 만들어 주입해 준다면 테스트는 똑같이 어렵지 않을 것 같습니다!

그리고 이 방법 역시 책임이 집중되는 것이긴 하나, 해당 어플리케이션에서의 중심이 되는 클래스이기에 어느 정도는 어쩔 수 없는 부분이지 않나 라는 생각도 들고요! ㅎㅎ

결론만 정리하자면, @june-777 님이 의견 주신 설계 정말 괜찮은 것 같습니다!!

맞습니다!! 설계는 정답이 없어서 너무 어려운 영역인 것 같아요 🤔
내가 구현한 설계가 어떤 트레이드 오프를 갖는지 인지하는게 중요한 것 같다는 생각이 드네요!!
여러 생각들을 공유해주셔서 감사합니다 😊 😊

Hugh-KR

구현하신다고 정말 고생하셨습니다. 😄

생각나는 의견들을 남겨봤어요. 추가로 기능단위 커밋에 대한 의견을 나눠보고 싶습니다.

구현하시면서 하나의 기능을 어떻게 정의하시는지 궁금해요.

Hugh-KR · 2023-11-22T14:33:37Z

src/main/java/racingcar/Application.java

 public class Application {
+    private static final InitController initController = new InitController();
+    private static final PlayController playController = new PlayController();
+
    public static void main(String[] args) {


@zangsu 저도 간단하게 의견을 공유하고 싶어요.

프로젝트가 고도화되고 규모가 확장되면 Controller 분리를 고민해야 하는 시점이 찾아온다고 생각해요. 그런데 이번 6기 프리코스에서 출제된 미션은 모두 하나의 목적을 구현하는 방식이라고 이해했습니다.

숫자 야구(승자 출력), 자동차 경주(승자 출력), 로또(당첨 여부 출력), 크리스마스(예약 내역 출력)

따라서 하나의 Controller에서 구현했고 책임이 과도해질 때 Service로 분리하는 방식으로 구현하고 있어요. 물론 최종에서 여러 가지 목적을 구현해야 하는 미션이 출제될 수도 있지만 고민하고 계신 부분에 도움이 되고 싶어 말씀드려 봅니다. 😄

Hugh-KR · 2023-11-22T14:44:06Z

src/main/java/racingcar/Application.java

+        Cars cars = initController.initCars();
+        int rounds = initController.getRounds();
+        initController.closeConsole();
+


해당 로직에서 제 생각을 말씀드리자면.

InitController, PlayController 를 통합하고 현재 main에 있는 로직을 모두 컨트롤러 내부에서 정의해서 사용하면 좋을 듯 합니다.

통상적인 Controller의 역할과도 부합하고 2개 생성되고 있는 컨트롤러의 생성 비용을 1개로 줄일 수 있으니까요.

Application 클래스에서 직관적으로 동작을 보여주고 싶다면 통합된 Controller에서 Init, Play 메서드로 분리해서 출력하면 Controller 생성, Init, Play 메서드 3개 로 main을 구성할 수 있습니다.

현재도 분리는 정말 잘하셨는데 고민하고 계신 것같아 코멘트 납깁니다. 😄

Hugh-KR · 2023-11-22T15:20:28Z

src/main/java/racingcar/domain/car/Cars.java

+    public Cars(List<String> carNames, NumberGenerator numberGenerator) {
+        validateCars(carNames);
+        cars = carNames.stream()
+                .map(Car::new)
+                .toList();
+        this.numberGenerator = numberGenerator;
+    }


해당 로직을 조금 더 효율적으로 만드는 방법을 고민해 봤어요.

Cars 객체는 생성과 동시에 validateCars(carNames);를 통해 비어있는 경우, 중복되는 경우를 검증하고 있습니다.

그렇다면 아래의 cars 리스트 생성 스트림과 validateCars의 위치를 바꾸면 어떻게 될까요?

Cars 객체가 생성과 동시에 리스트에 Car 객체를 생성 및 할당하고 Car 객체 에서도 빈값을 검증하고 있기에, 입력값이 ""인 경우와 pobi,"",Jon 인 경우를 한 번에 처리할 수 있습니다.

이로써 얻어지는 이점은 다음과 같습니다.

- 2번 할 수 있는 일을 1번으로 줄임 - validateNotEmpty() 메서드 제거 가능

이에 대해 의견을 나누고 싶습니다. 혁수님은 어떻게 생각하시나요?

어라, 그렇군요!
이제와서 다시 코드를 살펴보니 제가 왜 validateNotEmpty() 메서드를 만들었는지 모르겠네요 ㅋㅋㅋ

창혁님이 위치를 변경하는 가장 큰 이유로 중복되는 검증이 존재한다 라는 근거를 들어주셨는데, 저는 Cars.vaildateNotEmpty()만 제거해 주고 나머진 그대로 둘 것 같습니다.

중복을 확인하는 부분은 Car 객체보다 String으로 수행하는 것이 훨씬 편할 것이라 생각하기도 했고, 검증된 값들을 이용해서 객체 내부 필드를 업데이트 하는 쪽이 조금 더 취향에 가까워서요...ㅎㅎ

Hugh-KR · 2023-11-22T15:31:36Z

src/main/java/racingcar/view/OutputView.java

+import racingcar.RacingConst;
+import racingcar.domain.car.Car;
+import racingcar.view.io.Writer;
+


굉장히 현명한 고민이라고 생각해요. 그런데 하드코딩과 매직넘버는 지양하라 는 공통 요구사항에 포함된다고 인지하고 있어서 함께 고민하면 좋을 듯합니다.

Hugh-KR · 2023-11-22T15:34:47Z

src/main/java/racingcar/view/io/Writer.java

+    public void printMessage(String message) {
+        System.out.println(message);
+    }
+


메서드화 해서 사용하는게 깔끔하겠다는 생각이 문득 드네요. 눈에 슬쩍 담아가겠습니다. 😄

zangsu added 7 commits November 21, 2023 16:59

feat : 구현 완료

49162f5

test(MoveNumber) : 객체 생성 및 이동 테스트

0461bf3

refactor : 콘솔 닫아주기

4bc3767

refactor : 우승자 출력의 출력 형식 수정

9332a03

test : 통합테스트 추가

49777f2

style : 테스트 DisplayName 수정

dfa13b8

refactor : 변수 private final로 수정

d371887

zxcev reviewed Nov 21, 2023

View reviewed changes

src/main/java/racingcar/exception/RacingException.java

@@ -0,0 +1,26 @@

package racingcar.exception;

public enum RacingException {

Copy link

zxcev Nov 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

zxcev reviewed Nov 21, 2023

View reviewed changes

june-777 reviewed Nov 21, 2023

View reviewed changes

Hugh-KR reviewed Nov 22, 2023

View reviewed changes

		@@ -0,0 +1,26 @@
		package racingcar.exception;

		public enum RacingException {

Final practice #2387

Are you sure you want to change the base?

Final practice #2387

Conversation

zangsu commented Nov 21, 2023

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

june-777 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

Hugh-KR Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

june-777 Nov 24, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

zangsu Nov 22, 2023 • 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

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

Hugh-KR left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Hugh-KR Nov 22, 2023 • 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

Hugh-KR Nov 22, 2023 •

edited

Loading

june-777 Nov 24, 2023 •

edited

Loading

zangsu Nov 22, 2023 •

edited

Loading

Hugh-KR Nov 22, 2023 •

edited

Loading