[LBP] 정상화 자동차 경주 미션 제출합니다. #84

borntocode2 · 2025-02-11T08:50:50Z

페어 : 김준
리뷰어 : 이지윤

안녕하세요. 리뷰어 지윤님!
초록스터디 '자동차 경주 - 초간단 애플리케이션' 미션을 제출합니다.

메서드를 설계하고 분리하는 과정에서 네이밍에 대해 고민이 많았는데요,
혹시 코드를 리뷰해주실 때, 메서드, 변수 네이밍을 유심히 봐주실 수 있을까요? 감사합니다.

로직에 관해서 RacingGameController의 racingGame메서드에는 전체적인 로직이 담겨있어요.

car이름을 받는 inputNames() 메서드.

시도횟수를 받는 inputGameCount() 메서드.

car이름을 구분자인 ','기준으로 split하는 splitCarNames() 메서드.

요구사항에 맞게 이름을 검증하는 validateInputName() 메서드.

게임을 진행시키는 playRacingGame()메서드.

최종우승을 가리기 위해 움직인 거리를 반환하는 getLongestMoveDistance()메서드.

우승자를 출력하는 pringWinners() 메서드.

로 구성되어 있는데요,

사실 5번 메서드와 6번 메서드, 7번 메서드는 service레이어에서 로직을 처리합니다.

나머지 1, 2, 3, 4메서드는 자동차 경주 프로그래밍을 위한 '준비단계'라고 할까요? 그래서 controller 레이어에서 자체적으로 로직을 수행하는데요.
이 1,2,3,4메서드를 service에서 처리해야 했는지 고민이 됐어요.

본인이 생각하기에 본인은 controller - service - domain에 어떤 메서드를 넣고 로직을 설계해야하는지 감이 오질 않습니다. 혹시 본인이 고민에 방향성을 알려줄 키워드가 있을까요? 감사합니다.

기능 요구사항

기능 요구사항
주어진 횟수 동안 n대의 자동차는 전진 또는 멈출 수 있다.
각 자동차에 이름을 부여할 수 있다. 전진하는 자동차를 출력할 때 자동차 이름을 같이 출력한다.
자동차 이름은 쉼표(,)를 기준으로 구분하며 이름은 5자 이하만 가능하다.
사용자는 몇 번의 이동을 할 것인지를 입력할 수 있어야 한다.
전진하는 조건은 0에서 9 사이에서 random 값을 구한 후 random 값이 4 이상일 경우 전진하고, 3 이하의 값이면 멈춘다.
자동차 경주 게임을 완료한 후 누가 우승했는지를 알려준다. 우승자는 한 명 이상일 수 있다.

기존 프로그래밍 요구사항

자바 코드 컨벤션을 지키면서 프로그래밍한다.
기본적으로 Java Style Guide을 원칙으로 한다.
indent(인덴트, 들여쓰기) depth를 2를 넘지 않도록 구현한다. 1까지만 허용한다.
예를 들어 while문 안에 if문이 있으면 들여쓰기는 2이다.
힌트: indent(인덴트, 들여쓰기) depth를 줄이는 좋은 방법은 함수(또는 메서드)를 분리하면 된다.
3항 연산자를 쓰지 않는다.
else 예약어를 쓰지 않는다.
else 예약어를 쓰지 말라고 하니 switch/case로 구현하는 경우가 있는데 switch/case도 허용하지 않는다.
힌트: if문에서 값을 반환하는 방식으로 구현하면 else 예약어를 사용하지 않아도 된다.
함수(또는 메소드)의 길이가 15라인을 넘어가지 않도록 구현한다.
함수(또는 메소드)가 한 가지 일만 잘 하도록 구현한다.

미션 링크 : https://edu.nextstep.camp/s/EaGHivA8/ls/HxK92pJ0

미션 4단계 MVC패턴 리팩토링

안녕하세요 !
지금까지 지윤님이 남겨주신 리뷰와 LMS 4단계 리팩토링을 참고하여 기존 코드를 리팩토링 해보았답니다.

음, 우선 지윤님이 다른 리뷰이님께 남겨주신

흐름제어: Controller
입/출력: View
검증 : Car
게임 관련 로직 : RacingGameService-

패턴을 참고하여 코딩하였어요.

덕분에, Main함수 역할을 하는 RacingGameController의 racingGame()메서드는 생각보다? MVC패턴을 준수하고 흐름을 관리하는 역할을 부여할 수 있었어요 !

불과 며칠 전만 하더라도 MVC패턴이며 InputView며 Controller랑 Service, Domain 레이어들을 어떻게 설계해야하는지 정말 no idea였는데, 어찌저찌 설계해보았습니다.

public void racingGame() {
       // 1단계
        try (InputView inputView = new InputView()) {
            String[] inputNames = inputView.getCarNames();
            int gameCount = inputView.getGameCount();

            printEmptyLine();
       // 2단계
            List<Car> cars = racingGameService.playRacingGame(inputNames, gameCount);
       // 3단계
            ResultView.printWinners(racingGameService.getWinners(cars));
        }
    }

본인의 Controller안에 있는 racingGame메서드 인데요,

resource를 안전하게 관리하는 try문이 1단계 InputView를 담당합니다.
playRacingGame메서드로 cars를 저장하는 게임 실행 단계인 2단계입니다.
마지막으로 getWinners에 playRacingGame메서드 반환값인 cars를 넣어주고 승자를 출력하는 ResultView가 3단계인데요,

LMS에 힌트인 메인메서드를 보자면요,
본인의 1,2,3단계랑 다르게 cars파라미터가 이동하지 않습니다. (printEmptyLint()과 같이 출력예시를 맞추는 기능들은 흐름을 관리하는 controller레이어에 넣는게 맞다고 생각해요..)

음 이것을 해결하고자, 전역변수로 선언하던가, List cars와 우승자를 가려내기 위한 LongestMoveDistance를 갖는 gameResult라는 클래스를 만들어보기도 했는데요,

결국 cars파라미터가 이동하는 것이 코드가 가장 깔끔하고 가독성이 좋아 이렇게 마무리했답니다.

혹시 리뷰해주실 때, MVC패턴을 중점으로, cars파라미터가 이동하는 것 외에 다른 수가 있다던가, printEmptyLine()와 같은 정말 부가적인? 메서드들을 숨기는 방법이라던가 등의 'Controller의 흐름관리'에 측면에서 리뷰를 해주실 수 있을까요? 아직 Controller는 정말 '흐름관리', 'service레이어를 부르는 최종 단' 정도 밖에 생각하지 못했어요. 누군가는 Entity가 Dto로 변환되는 과정을 MVC패턴과 Controller의 쓰임이라고 하던데요. 잘 와닿지가 않습니다.

감사합니다. :D

dd-jiyun

안녕하세요 상화 👋

이번 미션 수고하셨습니다! 미션 요구 사항과 네이밍에 많이 신경쓰신 게 느껴졌어요 👍🏻

상화님이 말씀해주신 메서드,변수 네이밍을 위주로 봤고, 질문주신 각 클래스별 역할에 대해서도 리뷰를 남겼습니다!

추가적으로 궁금하신 부분이 있다면 편하게 질문해주세요 !

dd-jiyun · 2025-02-12T08:20:02Z

src/main/java/controller/RacingGameController.java

+
+public class RacingGameController {
+    RacingGameService racingGameService = new RacingGameService();
+    Scanner scanner = new Scanner(System.in);


현재 Scanner를 RacingGameController의 필드로 직접 선언하여 사용하고 있는데, 입력을 담당하는 별도의 InputView 클래스를 만들어 분리해보면 어떨까요?

이렇게 하면 입력 방식을 변경해야 할 때 더 유연하게 대응할 수 있고, 테스트 시에도 직접 Scanner를 사용하지 않고 입력을 주입하는 방식으로 개선할 수 있을 거예요.

그리고 Scanner는 리소스를 사용하므로 한 번만 생성하고 닫는 것이 안전한데, 지금 구조에서는 Scanner를 안전하게 관리하는 게 어려울 수도 있어요.

고민해보면 좋을 부분

Scanner를 필드로 선언하는 방식에서 어떤 단점이 있을까요?

여러 개의 컨트롤러가 생긴다면?

테스트 환경에서 입력을 주입하려면?

입력 방식을 확장해야 한다면, 현재 구조에서 얼마나 유연할까요?

예를 들어, 파일 입력, 네트워크 입력도 가능하게 만들려면?

이런 점들을 한 번 생각해보면 더 좋은 개선 방향이 떠오를 것 같아요!

리뷰 감사합니다.

리뷰 달아주신대로 global 디렉토리에 InputView클래스를 생성하고 AutoCloseablef implement하여, 객체로 생성하고 try-with-resources 구문을 활용해 안전하게 resources를 다루는 방법으로 유연하게 구현해보았습니다. !

Scanner를 필드로 선언하면 실수로 memory leak이 발생할 수 있고, 테스트 환경에서도 구현해주어야겠습니다.

새 클래스를 생성하고 객체를 생성하여 사용하는 방식으로 여러 개의 컨트롤러에서도, 테스트환경에서도, 파일 입력, 네트워크 입력시에도 유연하게 input을 받을 수 있게 구현해보았습니다.

감사합니다 .!

dd-jiyun · 2025-02-12T08:27:55Z

src/main/java/controller/RacingGameController.java

+    }
+
+    private boolean isIllegalInputName(String inputName) {
+        return inputName == null || inputName.isEmpty() || inputName.equals(" ") || isLongerThan5(inputName);


" "(공백)이나 예외 메세지 값들이 하드 코딩 되어있는데 상수로 분리하면 가독성이 더 좋아질 것 같아요!
나중에 값이 변경되더라도 한 곳에서만 수정하면 되니까 유지보수에도 유리하겠죠?

Q. 혹시 상화님은 어떤 기준으로 상수를 분리하는게 가장 적절할 지에 대해서 생각해보신 적 있으신가요??

이런 부분을 한번 생각해보시면서 리팩토링하면 더 개선할 부분이 보일 수도 있을 것 같아요.

리뷰 감사합니다.

Q. 혹시 상화님은 어떤 기준으로 상수를 분리하는게 가장 적절할 지에 대해서 생각해보신 적 있으신가요??
-> 이참에 생각해보았습니다. 크게 '불변성'과 '가독성'의 측면으로 생각해보았는데요. 사실, 불변성은 전부터 생각했습니다. 원주율처럼 프로그램 전체를 관장하는, 말 그대로의 '상수'로서의 기능을 하는 측면이라고 생각하는데요. 이번에 '가독성'에 대한 측면을 생각하면서 어떤 기준으로 상수를 분리하는지 조금 더 명확해진 것 같아요. split(",")과, split(DEFAULT_NAME_DELIMITER)와 같이 상수로 그 의미를 표현한다면 더욱 더 이해하기 쉽다고 생각해요. 앞으로도 '가독성'과 '불변성'을 생각하며 상수화 기준으로 삼아볼까 합니다.

감사합니다.

dd-jiyun · 2025-02-12T08:34:05Z

src/main/java/controller/RacingGameController.java

+
+        racingGameService.playRacingGame(cars, gameCount);
+
+        int LongestDistance = racingGameService.getLongestMoveDistance(cars);


다른 변수들은 카멜 케이스를 잘 맞추셨네요!
이 변수도 카멜 케이스로 수정해주시면 더 일관성 있을 거 같아요 👍🏻

리뷰 감사합니다.

확인한다고 했는데, 더 꼼꼼하게 확인하는 습관을 기르도록 하겠습니다. !

감사합니다.

dd-jiyun · 2025-02-12T08:50:12Z

src/main/java/controller/RacingGameController.java

+    private boolean isLongerThan5(String str) {
+        return str.length() > 5;
+    }


메서드 이름에 "5"와 같은 특정 값을 명시하면, 나중에 다른 조건으로 변경될 경우 메서드명을 수정해야 할 수 있어요.

따라서 의도를 명확히 드러내면서도 구체적인 값에 의존하지 않는 방식으로 작성하는 것이 좋습니다. 예를 들어, "5" 대신 MAX_NAME_LENGTH와 같은 상수를 사용하면 가독성도 높아지고, 나중에 변경될 때 더 편리해질 거예요.

또한, 현재 변수명이 str인데, 다른 변수들은 inputName이나 count와 같이 좀 더 의미를 명확하게 표현하고 있어서, 일관성 있게 변수명을 맞추는 것도 좋을 것 같아요. str보다는 name이나 inputName으로 바꾸면 더욱 직관적일 것 같아요!

리뷰 감사합니다.

메서드를 분리하는 이유가 '재사용성'의 측면에서 효율을 챙기기 위해 하나의 이유가 된다고 생각하는데요. !

메서드이름이 isLongerThan5인 이유도 '어떠한' 문자열이든간에 5가 넘는지 검사한다라는 뜻을 담고자 하여 메서드를 작성하였어요.

음, 사실 프로그램 요구사항에는 말씀하신대로,

private boolean isInputNameLongerThanMaxNameLength(String inputName)과 같이 사용해도 나중에 재사용할 일이 없으니 구체화의 목적은 달성했다고 생각합니다.

혹시, 만약에 본인의 프로그램이 기능이 추가되어서 inputName을 검사하는 것이 아닌 다른 문자열의 길이를 검사하는 메서드를 작성해야할 일이 생긴다면 본인의 기존 코드인 IsLongerThan메서드 명이나 str과 같은 변수 네이밍이 조금 더 재사용성이 가능한 메서드가 아닐까요?

'재사용성'과 '의미전달'의 두 측면에서 네이밍을 어떻게 짓느냐가 고민입니다. 혹시 지윤님의 생각을 코멘트로 달아주실 수 있을까요? 감사합니다.

메서드이름이 isLongerThan5인 이유도 '어떠한' 문자열이든간에 5가 넘는지 검사한다라는 뜻을 담고자 하여 메서드를 작성하였어요.

네! 상화님이 말씀하신 것처럼 메서드명이 일반적인 용도로도 활용될 수 있도록 고민하신 점 좋아요 👍🏻
다만, 추후에 길이 기준이 변경될 경우 메서드명까지 수정해야 한다는 점에서 조금 불편함이 생길 수도 있을 것 같아요.
만약 재사용성을 고려해서 일반화하려는 목적이라면, 오히려 더 유연한 설계를 고민해볼 수도 있을 것 같아요!

혹시, 만약에 본인의 프로그램이 기능이 추가되어서 inputName을 검사하는 것이 아닌 다른 문자열의 길이를 검사하는 메서드를 작성해야할 일이 생긴다면 본인의 기존 코드인 IsLongerThan메서드 명이나 str과 같은 변수 네이밍이 조금 더 재사용성이 가능한 메서드가 아닐까요?

이 부분도 좋은 고민이네요! 다만, 현재 자동차 경주 미션을 기준으로 봤을 때, 자동차 이름 외에 다른 문자열 길이를 검사해야 할 필요가 있을지 한 번 더 생각해보시면 좋을 것 같아요.
제 생각에는 현재 요구사항을 벗어나 너무 일반적인 형태로 설계하려 하면, 오히려 코드의 의미가 흐려질 수도 있을 것 같아요.

혹시 상화님이 고민하신 추가적인 기능이 어떤 부분일까요? 공유해주시면 더 다양한 방향으로 함께 이야기 나눌 수 있을 것 같아요 😄

메서드이름이 isLongerThan5인 이유도 '어떠한' 문자열이든간에 5가 넘는지 검사한다라는 뜻을 담고자 하여 메서드를 작성하였어요.

네! 상화님이 말씀하신 것처럼 메서드명이 일반적인 용도로도 활용될 수 있도록 고민하신 점 좋아요 👍🏻 다만, 추후에 길이 기준이 변경될 경우 메서드명까지 수정해야 한다는 점에서 조금 불편함이 생길 수도 있을 것 같아요. 만약 재사용성을 고려해서 일반화하려는 목적이라면, 오히려 더 유연한 설계를 고민해볼 수도 있을 것 같아요!

혹시, 만약에 본인의 프로그램이 기능이 추가되어서 inputName을 검사하는 것이 아닌 다른 문자열의 길이를 검사하는 메서드를 작성해야할 일이 생긴다면 본인의 기존 코드인 IsLongerThan메서드 명이나 str과 같은 변수 네이밍이 조금 더 재사용성이 가능한 메서드가 아닐까요?

이 부분도 좋은 고민이네요! 다만, 현재 자동차 경주 미션을 기준으로 봤을 때, 자동차 이름 외에 다른 문자열 길이를 검사해야 할 필요가 있을지 한 번 더 생각해보시면 좋을 것 같아요. 제 생각에는 현재 요구사항을 벗어나 너무 일반적인 형태로 설계하려 하면, 오히려 코드의 의미가 흐려질 수도 있을 것 같아요.

혹시 상화님이 고민하신 추가적인 기능이 어떤 부분일까요? 공유해주시면 더 다양한 방향으로 함께 이야기 나눌 수 있을 것 같아요 😄

메서드를 작성할 때, 프로그램 로직의 scope를 벗어나지 않고 메서드의 재사용성을 고려해야하는 것이군요 ! 저는 지윤님께서 말씀하신 '일반적인 형태'에 초점을 전부 두었던 것 같아요. 앞으로 프로그램 scope를 생각하면서 메서드와 파라미터의 네이밍을 잘 고려해보겠습니다 :D 감사합니다.

dd-jiyun · 2025-02-12T09:01:58Z

src/main/java/controller/RacingGameController.java

+        if (isIllegalInputName(inputName)) {
+            throw new IllegalArgumentException("name is illegal");


현재 예외 메시지가 "name is illegal"로 너무 간단하게 작성되어 있는데, 예외가 발생한 정확한 원인을 사용자나 개발자가 쉽게 이해할 수 있도록 좀 더 구체적인 메시지를 제공하는 것이 좋을 것 같아요. 예를 들어, "입력한 이름이 비어있거나 공백이 포함되어 있습니다." 또는 "이름은 5자 이하로 입력해야 합니다."와 같이 조건을 명시적으로 표현하면 좋을 것 같아요.

추가로, 예외 메시지를 작성할 때 참고할 수 있는 좋은 자료를 공유드립니다!

예외 메시지 작성 가이드

리뷰 감사합니다. !

어디에서 배운건지 예외 메시지를 짧고 강렬하게 작성하고자 하는 이상한 습관이 있었어요.
구체적인 메시지를 전달할 수 있도록 수정하였습니다.

네이밍에 관련해서 예외메시지도 추가하여 구체적이고 의미를 정확히 전달할 수 있게 신경써보겠습니다.

감사합니다. !

dd-jiyun · 2025-02-12T09:21:34Z

src/main/java/controller/RacingGameController.java

+    }
+
+    private boolean isIllegalInputName(String inputName) {
+        return inputName == null || inputName.isEmpty() || inputName.equals(" ") || isLongerThan5(inputName);


Suggested change

return inputName == null || inputName.isEmpty() || inputName.equals(" ") || isLongerThan5(inputName);

return inputName == null || inputName.trim().isEmpty() || isLongerThan5(inputName);

isEmpty()와 equals(" ")가 중복되어 있는 부분이 있어요. 위의 방법을 사용하면 문자열 앞뒤의 공백을 제거하고 빈 문자열인지 확인할 수 있어서 코드가 더 깔끔해질 거예요.

리뷰 감사합니다.

알려주신대로 trim메서드를 활용하여 문자열 양 끝의 공백을 제거하고 빈 문자열인지 확인할 수 있게 수정하였는데요.

감사드립니다. !

dd-jiyun · 2025-02-12T09:34:08Z

src/main/java/domain/Car.java

+import java.util.List;
+import java.util.Random;
+
+public class Car {


지금은 RacingGameController에서 Car 객체의 이름을 검증하고 있는데, 이 검증 로직을 Car 객체 자체에서 처리하도록 개선하면 어떨까요?

객체가 생성될 때, 자신의 이름을 스스로 검증하고 불완전한 상태로 생성되지 않도록 할 수 있어요. 이렇게 하면 책임을 객체로 분리할 수 있어서, Car 클래스의 응집력도 높아지고, 만약 규칙이 변경된다면 Car 클래스만 수정하면 되어 유지보수가 더 쉬워질 거예요.

즉, RacingGameController는 객체를 생성하는 역할만 맡고, Car는 자신의 상태를 스스로 관리하게 되면 코드가 더 직관적이고, 테스트하기에도 쉬워집니다!

리뷰 감사합니다.

말씀해주신대로 Car객체에 name을 검증하는 로직으로 구현하였어요 !
Car클래스에 getInstancesByNames메서드에 구현했답니다.

사실 기존 코드에서의 controller에서 Car객체의 이름을 검증하는 이유는 '입력받는 name을 검사한다.' 라는 이유였어요.

지윤님의 리뷰를 토대로 수정하면서 들었던 고민과 생각에 코멘트를 부탁드려도 될까요?
->

입력받은 input을 검사하는 것이니 Car객체에는 어울리지 않다.

하지만 input은 결국 Car객체의 name이 될 테니, Car객체에 넣어야하나?

그럼 Car를 생성하는 생성자에 이름을 검증하는 로직을 구현해야할까?

아, 구분자로 split된 String inputNames를 Car객체로 변환하여 List로 인스턴스화 하는 getInstancesByNames라는 메서드가 있구나.

그럼 getInstancesByNames메서드 안에다가 구현해야할까?

getInstancesByNames이라는 네이밍은 '이름으로 인스턴스를 만든다'라는 의미를 담고 있으니, Car의 name을 검증하는 로직은 메서드가 가지고 있는 단일성을 위배하는 것은 아닐까?

결국 마지막 6번의 고민을 해결하지 못하고 구현하였어요.

코멘트 남겨주시면 정말 감사하겠습니다.

감사합니다.

상화님이 고민과 생각을 잘 적어주셨네요 👍 SRP 관점에서 역할과 책임을 어떻게 분리할지 고민하신 것 같아요.

Car 객체가 '자동차'라는 도메인을 표현하는 역할을 한다면, 이름 검증 로직이 포함되는게 자연스러울 수 있어요. 결국 Car의 name이 유효한지를 보장하는 것도 Car의 책임이 될 수 있으니까요.

다만, getInstancesByNames는 "이름을 받아 인스턴스를 생성하는 역할"을 가진 메서드인데, 여기서 검증까지 수행한다면 "이름 검증 + 객체 생성"이라는 두 가지 책임을 갖게 됩니다. 이렇게 되면 고민하신 것과 같이 단일 책임 원칙(SRP)을 위배할 가능성이 있어요.

그래서 생성자에서 검증하도록 하면 더 자연스러울 수 있어요!

그럼 getInstancesByNames()는 단순히 List<Car>를 생성하는 역할만 수행하고 검증 책임은 Car 생성자에서 담당하게 됩니다!

상화님만의 역할과 책임에 대한 기준을 가지고 각각의 장단점을 알아본 뒤 리팩토링해보시면 어떨까요?

상화님이 고민과 생각을 잘 적어주셨네요 👍 SRP 관점에서 역할과 책임을 어떻게 분리할지 고민하신 것 같아요.

Car 객체가 '자동차'라는 도메인을 표현하는 역할을 한다면, 이름 검증 로직이 포함되는게 자연스러울 수 있어요. 결국 Car의 name이 유효한지를 보장하는 것도 Car의 책임이 될 수 있으니까요.

다만, getInstancesByNames는 "이름을 받아 인스턴스를 생성하는 역할"을 가진 메서드인데, 여기서 검증까지 수행한다면 "이름 검증 + 객체 생성"이라는 두 가지 책임을 갖게 됩니다. 이렇게 되면 고민하신 것과 같이 단일 책임 원칙(SRP)을 위배할 가능성이 있어요.

그래서 생성자에서 검증하도록 하면 더 자연스러울 수 있어요!

그럼 getInstancesByNames()는 단순히 List<Car>를 생성하는 역할만 수행하고 검증 책임은 Car 생성자에서 담당하게 됩니다!

상화님만의 역할과 책임에 대한 기준을 가지고 각각의 장단점을 알아본 뒤 리팩토링해보시면 어떨까요?

네 ! 리뷰 감사합니다 !
말씀하신대로, MVC 패턴으로 리팩토링을 하면서 메서드가 가지는 단일성을 잘 고려하고 어느 곳에 책임을 둘 것 인지 나름대로? 명확히 하면서 리팩토링을 진행하였어요 ! 지금까지 리뷰를 받아보니, 역할과 책임에 대한 기준의 '답'이 있는 것이 아니라, 상식이 통하는 기준안에서 본인만의 기준을 갖는 것이 중요하군요. 앞으로 다른 사람과 타협이 가능한 코드를 작성하도록 많은 시간을 할애해야할 것 같습니다. 감사합니다. 지윤님 !

dd-jiyun · 2025-02-12T09:38:14Z

src/main/java/domain/Car.java

+    private int getRandomNumber() {
+        return new Random().nextInt(10);
+    }


현재 Car 클래스에서 Random 객체를 직접 생성해서 사용하는 방식이네요.
이렇게 직접 생성하고 사용하면 어떤 문제점이 있을까요?
그리고 어떻게 해결할 수 있을까요?

리뷰 감사합니다.

Random 객체를 직접 생성하지 않고 의존성 주입 방식으로 Random을 private final Random random; 으로 주입받아 사용하는 방식으로 구현하려 했으나, static문제로 로직이 이리저리 꼬였습니다.
아마 충분한 학습을 하지 않고 구현하려 해서 문제가 터진 것 같아요.

이 부분에 가장 시간을 많이 투자했는데, Java에 대해 더 공부할 필요가 있는 것 같아요.

말씀해주신대로 직접 생성하고 사용하면 성능과 테스트환경에서의 문제가 발생할 수 있다는 문제점은 덕분에 알게 되었는데요.
죄송하지만, DI 방식으로 구현하는 것은 추후에 진행해야할 것 같아요.

꼭 구현해보겠습니다.
감사합니다. :D

충분한 학습 없이 구현하려고 해서 문제가 터졌다고 하셨지만 오히려 이런 과정을 겪으면서 학습하는게 가장 좋은 방법이라고 생각해요 :)

직접 생성하는 방식과 DI 방식의 차이를 고민해 보신 것만으로도 성장이 있었을 것 같네요!

추후에 다시 도전하신다고 하셨으니 그때 또 함께 고민해보면 좋을 것 같습니다. 앞으로도 좋은 질문과 고민 기대할게요!
화이팅입니다 🔥

dd-jiyun · 2025-02-12T09:45:11Z

src/main/java/service/RacingGameService.java

+        List<Integer> distances = cars.stream()
+                .map(Car::getMoveDistance)
+                .toList();
+
+        return distances.stream()
+                .max(Integer::compareTo)
+                .orElseThrow(() -> new IllegalArgumentException("carList is empty"));


Suggested change

List<Integer> distances = cars.stream()

.map(Car::getMoveDistance)

.toList();

return distances.stream()

.max(Integer::compareTo)

.orElseThrow(() -> new IllegalArgumentException("carList is empty"));

return cars.stream()

.map(Car::getMoveDistance)

.max(Integer::compareTo)

.orElseThrow(() -> new IllegalArgumentException("carList is empty"));

List를 생성하고 다시 Stream을 사용해서 최댓값을 구하는데 위와 같이 한 번의 연산으로도 처리할 수 있습니다!

이 부분 또한, 예외 메세지를 더 자세하게 적어주면 좋을 것 같아요 🤭

리뷰 감사합니다.

stream에 익숙하지 않아 연습 겸 Java언어로 문자열을 다루는 알고리즘 문제를 풀어보려합니다.

예외메시지 또한 더욱 더 구체적으로 의미를 전달할 수 있게 작성해보겠습니다.

감사합니다. ^^

dd-jiyun · 2025-02-12T09:48:55Z

src/main/java/service/RacingGameService.java

+    public void printWinners(int LongestMoveDistance, List<Car> cars) {
+        List<String> winnerNames = cars.stream()
+                .filter(car -> car.getMoveDistance() == LongestMoveDistance)
+                .map(Car::getName)
+                .toList();
+
+        StringBuilder output = new StringBuilder(winnerNames.get(0));
+
+        for (int i = 1; i < winnerNames.size(); i++) {
+            output.append(", ").append(winnerNames.get(i));
+        }
+
+        System.out.println(output + "가 최종 우승했습니다.");
+    }


이 부분에 StringBuilder를 사용하지 않고 Stream을 활용해서 더 간결하게 처리할 수 있어요! Collector에 대해서 학습해 본 후 적용시켜볼까요??

리뷰 감사합니다.

public void printWinners(int longestMoveDistance, List<Car> cars) { String winners = cars.stream() .filter(car -> car.getMoveDistance() == longestMoveDistance) .map(Car::getName) .collect(Collectors.joining(", ")); System.out.println(winners + "가 최종 우승했습니다."); }

말씀해주신대로 적용해보았는데요.
학습할 시간이 충분하지 않은 것 같습니다.
나중에 똑같은 로직을 구현할 때, 이 코드와 같이 구현할 자신은 없어서 위의 코멘트대로collector와 여러가지 Java메서드에 대해 학습해보겠습니다.

정말 감사합니다. !

dd-jiyun

제 리뷰에 상화님의 의견도 궁금해요~ 어떤 공감을 가지고 수정하게 되었는지 답변 부탁드려요 :)

borntocode2 · 2025-02-14T10:44:49Z

제 리뷰에 상화님의 의견도 궁금해요~ 어떤 공감을 가지고 수정하게 되었는지 답변 부탁드려요 :)

지윤님 ! 혹시 저의 코멘트가 보이시지 않는걸까요?
리뷰달아주신 곳에 답변을 작성하였는데요, Pending상태라서 혹시 코멘트가 보이시지 않나 해서요..
아님 저의 코멘트가 불충분했던걸까요?

dd-jiyun · 2025-02-14T10:45:11Z

제 리뷰에 상화님의 의견도 궁금해요~ 어떤 공감을 가지고 수정하게 되었는지 답변 부탁드려요 :)

지윤님 ! 혹시 저의 코멘트가 보이시지 않는걸까요? 리뷰달아주신 곳에 답변을 작성하였는데요, Pending상태라서 혹시 코멘트가 보이시지 않나 해서요.. 아님 저의 코멘트가 불충분했던걸까요?

공감만 있고 답변이 없어요!

borntocode2

.test

borntocode2 · 2025-02-14T08:32:06Z

src/main/java/controller/RacingGameController.java

+
+public class RacingGameController {
+    RacingGameService racingGameService = new RacingGameService();
+    Scanner scanner = new Scanner(System.in);


리뷰 감사합니다.

리뷰 달아주신대로 global 디렉토리에 InputView클래스를 생성하고 AutoCloseablef implement하여, 객체로 생성하고 try-with-resources 구문을 활용해 안전하게 resources를 다루는 방법으로 유연하게 구현해보았습니다. !

Scanner를 필드로 선언하면 실수로 memory leak이 발생할 수 있고, 테스트 환경에서도 구현해주어야겠습니다.

새 클래스를 생성하고 객체를 생성하여 사용하는 방식으로 여러 개의 컨트롤러에서도, 테스트환경에서도, 파일 입력, 네트워크 입력시에도 유연하게 input을 받을 수 있게 구현해보았습니다.

감사합니다 .!

borntocode2 · 2025-02-14T08:32:54Z

src/main/java/controller/RacingGameController.java

+
+        racingGameService.playRacingGame(cars, gameCount);
+
+        int LongestDistance = racingGameService.getLongestMoveDistance(cars);


리뷰 감사합니다.

확인한다고 했는데, 더 꼼꼼하게 확인하는 습관을 기르도록 하겠습니다. !

감사합니다.

borntocode2 · 2025-02-14T08:40:11Z

src/main/java/controller/RacingGameController.java

+    }
+
+    private boolean isIllegalInputName(String inputName) {
+        return inputName == null || inputName.isEmpty() || inputName.equals(" ") || isLongerThan5(inputName);


리뷰 감사합니다.

Q. 혹시 상화님은 어떤 기준으로 상수를 분리하는게 가장 적절할 지에 대해서 생각해보신 적 있으신가요??
-> 이참에 생각해보았습니다. 크게 '불변성'과 '가독성'의 측면으로 생각해보았는데요. 사실, 불변성은 전부터 생각했습니다. 원주율처럼 프로그램 전체를 관장하는, 말 그대로의 '상수'로서의 기능을 하는 측면이라고 생각하는데요. 이번에 '가독성'에 대한 측면을 생각하면서 어떤 기준으로 상수를 분리하는지 조금 더 명확해진 것 같아요. split(",")과, split(DEFAULT_NAME_DELIMITER)와 같이 상수로 그 의미를 표현한다면 더욱 더 이해하기 쉽다고 생각해요. 앞으로도 '가독성'과 '불변성'을 생각하며 상수화 기준으로 삼아볼까 합니다.

감사합니다.

borntocode2 · 2025-02-14T08:42:17Z

src/main/java/controller/RacingGameController.java

+    }
+
+    private boolean isIllegalInputName(String inputName) {
+        return inputName == null || inputName.isEmpty() || inputName.equals(" ") || isLongerThan5(inputName);


리뷰 감사합니다.

알려주신대로 trim메서드를 활용하여 문자열 양 끝의 공백을 제거하고 빈 문자열인지 확인할 수 있게 수정하였는데요.

감사드립니다. !

borntocode2 · 2025-02-14T08:44:09Z

src/main/java/controller/RacingGameController.java

+        if (isIllegalInputName(inputName)) {
+            throw new IllegalArgumentException("name is illegal");


리뷰 감사합니다. !

어디에서 배운건지 예외 메시지를 짧고 강렬하게 작성하고자 하는 이상한 습관이 있었어요.
구체적인 메시지를 전달할 수 있도록 수정하였습니다.

네이밍에 관련해서 예외메시지도 추가하여 구체적이고 의미를 정확히 전달할 수 있게 신경써보겠습니다.

감사합니다. !

borntocode2 · 2025-02-14T09:11:35Z

src/main/java/controller/RacingGameController.java

+    private boolean isLongerThan5(String str) {
+        return str.length() > 5;
+    }


리뷰 감사합니다.

메서드를 분리하는 이유가 '재사용성'의 측면에서 효율을 챙기기 위해 하나의 이유가 된다고 생각하는데요. !

메서드이름이 isLongerThan5인 이유도 '어떠한' 문자열이든간에 5가 넘는지 검사한다라는 뜻을 담고자 하여 메서드를 작성하였어요.

음, 사실 프로그램 요구사항에는 말씀하신대로,

private boolean isInputNameLongerThanMaxNameLength(String inputName)과 같이 사용해도 나중에 재사용할 일이 없으니 구체화의 목적은 달성했다고 생각합니다.

혹시, 만약에 본인의 프로그램이 기능이 추가되어서 inputName을 검사하는 것이 아닌 다른 문자열의 길이를 검사하는 메서드를 작성해야할 일이 생긴다면 본인의 기존 코드인 IsLongerThan메서드 명이나 str과 같은 변수 네이밍이 조금 더 재사용성이 가능한 메서드가 아닐까요?

'재사용성'과 '의미전달'의 두 측면에서 네이밍을 어떻게 짓느냐가 고민입니다. 혹시 지윤님의 생각을 코멘트로 달아주실 수 있을까요? 감사합니다.

borntocode2 · 2025-02-14T09:24:43Z

src/main/java/domain/Car.java

+import java.util.List;
+import java.util.Random;
+
+public class Car {


리뷰 감사합니다.

말씀해주신대로 Car객체에 name을 검증하는 로직으로 구현하였어요 !
Car클래스에 getInstancesByNames메서드에 구현했답니다.

사실 기존 코드에서의 controller에서 Car객체의 이름을 검증하는 이유는 '입력받는 name을 검사한다.' 라는 이유였어요.

지윤님의 리뷰를 토대로 수정하면서 들었던 고민과 생각에 코멘트를 부탁드려도 될까요?
->

입력받은 input을 검사하는 것이니 Car객체에는 어울리지 않다.

하지만 input은 결국 Car객체의 name이 될 테니, Car객체에 넣어야하나?

그럼 Car를 생성하는 생성자에 이름을 검증하는 로직을 구현해야할까?

아, 구분자로 split된 String inputNames를 Car객체로 변환하여 List로 인스턴스화 하는 getInstancesByNames라는 메서드가 있구나.

그럼 getInstancesByNames메서드 안에다가 구현해야할까?

getInstancesByNames이라는 네이밍은 '이름으로 인스턴스를 만든다'라는 의미를 담고 있으니, Car의 name을 검증하는 로직은 메서드가 가지고 있는 단일성을 위배하는 것은 아닐까?

결국 마지막 6번의 고민을 해결하지 못하고 구현하였어요.

코멘트 남겨주시면 정말 감사하겠습니다.

감사합니다.

borntocode2 · 2025-02-14T09:27:04Z

src/main/java/service/RacingGameService.java

+        List<Integer> distances = cars.stream()
+                .map(Car::getMoveDistance)
+                .toList();
+
+        return distances.stream()
+                .max(Integer::compareTo)
+                .orElseThrow(() -> new IllegalArgumentException("carList is empty"));


리뷰 감사합니다.

stream에 익숙하지 않아 연습 겸 Java언어로 문자열을 다루는 알고리즘 문제를 풀어보려합니다.

예외메시지 또한 더욱 더 구체적으로 의미를 전달할 수 있게 작성해보겠습니다.

감사합니다. ^^

borntocode2 · 2025-02-14T09:29:25Z

src/main/java/service/RacingGameService.java

+    public void printWinners(int LongestMoveDistance, List<Car> cars) {
+        List<String> winnerNames = cars.stream()
+                .filter(car -> car.getMoveDistance() == LongestMoveDistance)
+                .map(Car::getName)
+                .toList();
+
+        StringBuilder output = new StringBuilder(winnerNames.get(0));
+
+        for (int i = 1; i < winnerNames.size(); i++) {
+            output.append(", ").append(winnerNames.get(i));
+        }
+
+        System.out.println(output + "가 최종 우승했습니다.");
+    }


리뷰 감사합니다.

public void printWinners(int longestMoveDistance, List<Car> cars) { String winners = cars.stream() .filter(car -> car.getMoveDistance() == longestMoveDistance) .map(Car::getName) .collect(Collectors.joining(", ")); System.out.println(winners + "가 최종 우승했습니다."); }

말씀해주신대로 적용해보았는데요.
학습할 시간이 충분하지 않은 것 같습니다.
나중에 똑같은 로직을 구현할 때, 이 코드와 같이 구현할 자신은 없어서 위의 코멘트대로collector와 여러가지 Java메서드에 대해 학습해보겠습니다.

정말 감사합니다. !

borntocode2 · 2025-02-14T09:33:43Z

src/main/java/domain/Car.java

+    private int getRandomNumber() {
+        return new Random().nextInt(10);
+    }


리뷰 감사합니다.

Random 객체를 직접 생성하지 않고 의존성 주입 방식으로 Random을 private final Random random; 으로 주입받아 사용하는 방식으로 구현하려 했으나, static문제로 로직이 이리저리 꼬였습니다.
아마 충분한 학습을 하지 않고 구현하려 해서 문제가 터진 것 같아요.

이 부분에 가장 시간을 많이 투자했는데, Java에 대해 더 공부할 필요가 있는 것 같아요.

말씀해주신대로 직접 생성하고 사용하면 성능과 테스트환경에서의 문제가 발생할 수 있다는 문제점은 덕분에 알게 되었는데요.
죄송하지만, DI 방식으로 구현하는 것은 추후에 진행해야할 것 같아요.

꼭 구현해보겠습니다.
감사합니다. :D

borntocode2 · 2025-02-14T10:47:33Z

제 리뷰에 상화님의 의견도 궁금해요~ 어떤 공감을 가지고 수정하게 되었는지 답변 부탁드려요 :)

지윤님 ! 혹시 저의 코멘트가 보이시지 않는걸까요? 리뷰달아주신 곳에 답변을 작성하였는데요, Pending상태라서 혹시 코멘트가 보이시지 않나 해서요.. 아님 저의 코멘트가 불충분했던걸까요?

공감만 있고 답변이 없어요!

아, 죄송합니다. 제가 PR의 리뷰에 익숙하지 않다보니 별도의 요청을 해야하는지 몰랐습니다. 앞으로의 리뷰는 다시 한번 체크하면서 재 리뷰 요청드리겠습니다 .!!

borntocode2 · 2025-02-14T10:53:51Z

제 리뷰에 상화님의 의견도 궁금해요~ 어떤 공감을 가지고 수정하게 되었는지 답변 부탁드려요 :)

지윤님 ! 혹시 저의 코멘트가 보이시지 않는걸까요? 리뷰달아주신 곳에 답변을 작성하였는데요, Pending상태라서 혹시 코멘트가 보이시지 않나 해서요.. 아님 저의 코멘트가 불충분했던걸까요?

공감만 있고 답변이 없어요!

아, 죄송합니다. 제가 PR의 리뷰에 익숙하지 않다보니 별도의 요청을 해야하는지 몰랐습니다. 앞으로의 리뷰는 다시 한번 체크하면서 재 리뷰 요청드리겠습니다 .! ps. 아마도 제가 지윤님 리뷰에 코멘트를 작성한 것이 아닌, 제 코드에 제가 리뷰를 남긴 것 같아요.

dd-jiyun

리팩토링 하면서 열심히 고민하신 게 보이네요! 상화님이 답변 해주신 것 질문에 답변 남겼어요.
열심히 답변해주셔서 감사합니다 🤭

더 궁금하신 것 있으시면 질문해주세요 !!

dd-jiyun · 2025-02-14T11:21:33Z

src/main/java/domain/Car.java

+    private int getRandomNumber() {
+        return new Random().nextInt(10);
+    }


충분한 학습 없이 구현하려고 해서 문제가 터졌다고 하셨지만 오히려 이런 과정을 겪으면서 학습하는게 가장 좋은 방법이라고 생각해요 :)

직접 생성하는 방식과 DI 방식의 차이를 고민해 보신 것만으로도 성장이 있었을 것 같네요!

추후에 다시 도전하신다고 하셨으니 그때 또 함께 고민해보면 좋을 것 같습니다. 앞으로도 좋은 질문과 고민 기대할게요!
화이팅입니다 🔥

dd-jiyun · 2025-02-14T11:43:06Z

src/main/java/domain/Car.java

+import java.util.List;
+import java.util.Random;
+
+public class Car {


상화님이 고민과 생각을 잘 적어주셨네요 👍 SRP 관점에서 역할과 책임을 어떻게 분리할지 고민하신 것 같아요.

Car 객체가 '자동차'라는 도메인을 표현하는 역할을 한다면, 이름 검증 로직이 포함되는게 자연스러울 수 있어요. 결국 Car의 name이 유효한지를 보장하는 것도 Car의 책임이 될 수 있으니까요.

다만, getInstancesByNames는 "이름을 받아 인스턴스를 생성하는 역할"을 가진 메서드인데, 여기서 검증까지 수행한다면 "이름 검증 + 객체 생성"이라는 두 가지 책임을 갖게 됩니다. 이렇게 되면 고민하신 것과 같이 단일 책임 원칙(SRP)을 위배할 가능성이 있어요.

그래서 생성자에서 검증하도록 하면 더 자연스러울 수 있어요!

그럼 getInstancesByNames()는 단순히 List<Car>를 생성하는 역할만 수행하고 검증 책임은 Car 생성자에서 담당하게 됩니다!

상화님만의 역할과 책임에 대한 기준을 가지고 각각의 장단점을 알아본 뒤 리팩토링해보시면 어떨까요?

dd-jiyun · 2025-02-14T11:58:08Z

src/main/java/controller/RacingGameController.java

+    private boolean isLongerThan5(String str) {
+        return str.length() > 5;
+    }


메서드이름이 isLongerThan5인 이유도 '어떠한' 문자열이든간에 5가 넘는지 검사한다라는 뜻을 담고자 하여 메서드를 작성하였어요.

네! 상화님이 말씀하신 것처럼 메서드명이 일반적인 용도로도 활용될 수 있도록 고민하신 점 좋아요 👍🏻
다만, 추후에 길이 기준이 변경될 경우 메서드명까지 수정해야 한다는 점에서 조금 불편함이 생길 수도 있을 것 같아요.
만약 재사용성을 고려해서 일반화하려는 목적이라면, 오히려 더 유연한 설계를 고민해볼 수도 있을 것 같아요!

혹시, 만약에 본인의 프로그램이 기능이 추가되어서 inputName을 검사하는 것이 아닌 다른 문자열의 길이를 검사하는 메서드를 작성해야할 일이 생긴다면 본인의 기존 코드인 IsLongerThan메서드 명이나 str과 같은 변수 네이밍이 조금 더 재사용성이 가능한 메서드가 아닐까요?

이 부분도 좋은 고민이네요! 다만, 현재 자동차 경주 미션을 기준으로 봤을 때, 자동차 이름 외에 다른 문자열 길이를 검사해야 할 필요가 있을지 한 번 더 생각해보시면 좋을 것 같아요.
제 생각에는 현재 요구사항을 벗어나 너무 일반적인 형태로 설계하려 하면, 오히려 코드의 의미가 흐려질 수도 있을 것 같아요.

혹시 상화님이 고민하신 추가적인 기능이 어떤 부분일까요? 공유해주시면 더 다양한 방향으로 함께 이야기 나눌 수 있을 것 같아요 😄

borntocode2 added 2 commits February 11, 2025 17:30

complete

e024b51

uppercase edit

ba6f96f

dd-jiyun suggested changes Feb 12, 2025

View reviewed changes

edit inputView in comment/next-step#1, check scope problem

9558a5b

kjoon418 mentioned this pull request Feb 13, 2025

잘못된 PR 경로로 인해 close했습니다. #83

Closed

5 tasks

borntocode2 added 2 commits February 13, 2025 21:42

[feat/next-step#2] fix before random problem

343d844

edit All except random problem

1808884

borntocode2 requested a review from dd-jiyun February 14, 2025 08:22

dd-jiyun suggested changes Feb 14, 2025

View reviewed changes

borntocode2 commented Feb 14, 2025

View reviewed changes

dd-jiyun approved these changes Feb 14, 2025

View reviewed changes

MVC 리팩토링

b16b8cb

boorownie merged commit b0f3a8e into next-step:borntocode2 Feb 16, 2025


		racingGameService.playRacingGame(cars, gameCount);

		int LongestDistance = racingGameService.getLongestMoveDistance(cars);

		if (isIllegalInputName(inputName)) {
		throw new IllegalArgumentException("name is illegal");

	return inputName == null \|\| inputName.isEmpty() \|\| inputName.equals(" ") \|\| isLongerThan5(inputName);
	return inputName == null \|\| inputName.trim().isEmpty() \|\| isLongerThan5(inputName);

[LBP] 정상화 자동차 경주 미션 제출합니다. #84

[LBP] 정상화 자동차 경주 미션 제출합니다. #84

Conversation

borntocode2 commented Feb 11, 2025 • edited Loading

기능 요구사항

기존 프로그래밍 요구사항

미션 4단계 MVC패턴 리팩토링

dd-jiyun 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

borntocode2 Feb 16, 2025 • 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

dd-jiyun left a comment

Choose a reason for hiding this comment

borntocode2 commented Feb 14, 2025

dd-jiyun commented Feb 14, 2025

borntocode2 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

borntocode2 commented Feb 14, 2025

borntocode2 commented Feb 14, 2025

dd-jiyun left a comment • 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

borntocode2 commented Feb 11, 2025 •

edited

Loading

borntocode2 Feb 16, 2025 •

edited

Loading

dd-jiyun left a comment •

edited

Loading