[LBP] 현정빈 자동차 경주 미션 1,2,3 단계 제출합니다. #88
Conversation
JeongBeanHyun
changed the title
There was a problem hiding this comment.
안녕하세요 정빈 👋
이번 미션도 고생하셨습니다! 말씀해주신 부분을 중점적으로 많이 봤어요.
추가적으로 궁금하신 부분이 있다면 편하게 질문해주세요 !
현재 코드에서는 따로 controller를 만들지 않고, main에서 RacingGame, InputView, ResultView를 직접 호출하고 있습니다. controller를 따로 만들지 않고 Main에서 게임을 진행하면 어떤 문제가 발생하는 걸까요?
물론 정답은 없지만, Main 클래스는 애플리케이션 실행 시작의 역할만 담당하는 것이 이상적이에요. 지금처럼 Main에서 여러 객체를 생성하는 건 책임이 너무 많아집니다. 그리고 게임의 로직이 변경되었을 때 Main 클래스를 변경해야하는 것은 코드 유지보수도 어려워지고 "캡슐화"를 위반하게 돼요.
가급적 Controller를 사용하는 것이 좋겠죠?
controller의 책임 범위를 어디까지로 생각하면 좋을지 궁금합니다. controller는 흐름만 관리하고, 입력 검증과 자동차 생성은 InputView에서 담당하도록 설정하는 것이 맞을까요?
코드에 정답은 없기 때문에 정빈님만의 기준을 세우시는 게 좋을 거 같습니다.
저의 생각을 살짝 공유드리자면,
- 흐름제어: Controller
- 입/출력: View
- 검증 : Car
- 게임 관련 로직 : RacingGameService
이렇게 구성할 것 같아요.
어떻게 하면 각 클래스가 하나의 책임을 가질 수 있게 설계할 수 있을 지 고민해보시면 좋을 것 같아요!
단일 책임 원칙을 더 잘 지키기 위해 가장 중요한 기준이 무엇일까요?
클래스를 나누는 데 책임의 범위를 명확히 해서 각 클래스가 맡는 역할이 분명하고, 그 역할에 맞는 책임만 가지도록 하는 것이라고 생각해요.
src/main/java/Car.java
Outdated
|
|
||
| private final String name; | ||
| private int position; | ||
| private static final int MOVE_TRIGGER = 4; |
There was a problem hiding this comment.
상수화 👍🏻
추가로 참고하시면 좋은 컨벤션 블로그 공유드립니다!
메서드 및 필드 선언 순서
There was a problem hiding this comment.
자료 공유 감사합니다! 참고하여 코드 작성하도록 하겠습니다!
src/main/java/InputView.java
Outdated
| public void validateInput(String carName) throws Exception { | ||
| if (carName.length() > 5) { | ||
| throw new Exception("자동차 이름은 5자 이하여야합니다."); | ||
| } | ||
| } |
There was a problem hiding this comment.
현재 validateName이라는 이름만 보면 이름 전체를 검증하는 것인지, 길이를 검증하는 것인지 명확하지 않을 수 있어요.
역할을 잘 나타낼 수 있는 네이밍을 고려해보는 것은 어떨까요?
메서드 이름으로 어떤 역할을 하는 지 알 수 있다면 주석이 필요하지 않겠죠?
There was a problem hiding this comment.
validateName을 validateNameLength로 수정하였습니다. 메서드의 역할을 직관적으로 전달하기 위해 네이밍에 더욱 신경쓰겠습니다!
네이밍을 할 때 항상 어떻게 작성해야할지 고민이 많은데요,
-
메서드의 역할을 직관적으로 전달하면서도
-
너무 길지 않게 간결한 표현을 유지하는 것
이 두 가지를 균형 있게 고려하려고 노력하고 있습니다.
메서드 이름이 길어지면 가독성이 떨어질 것 같아 최대한 간결하게 작성하려고 하는데,
그렇다 보니 오히려 의미가 명확하지 않게 전달되는 경우가 많은 것 같습니다.
혹시 지윤 리뷰어님께서는 네이밍을 정할 때 참고하는 원칙이나 사이트가 있을까요?
그리고 메서드 이름을 작성할 때 몇 단어까지가 적절한지에 대한 기준이 있다면 공유해주실 수 있을까요?
더 좋은 네이밍을 고민하는 데 많은 도움이 될 것 같습니다! 감사합니다!
There was a problem hiding this comment.
혹시 지윤 리뷰어님께서는 네이밍을 정할 때 참고하는 원칙이나 사이트가 있을까요?
그리고 메서드 이름을 작성할 때 몇 단어까지가 적절한지에 대한 기준이 있다면 공유해주실 수 있을까요?
저는 네이밍을 정할 때 직관적인 메서드명을 가장 중요하게 생각해요! 정빈님께서 우려하신 것처럼, 메서드명이 길어지면 가독성이 떨어질 수도 있지만, 오히려 메서드가 너무 많은 일을 하고 있지는 않은지 점검해볼 기회라고 생각해요!
만약 메서드명이 길어진다면, 저는 먼저 해당 메서드의 역할을 다시 한번 점검하고, 꼭 필요한 단어만 남기려고 합니다. 그럼에도 적절한 네이밍이 떠오르지 않는다면, 여러 표현을 검색해 보거나 AI에게 "이 메서드를 잘 표현할 수 있는 이름"을 추천받기도 해요.
추가로, 네이밍을 정할 때는 일관성도 중요한 요소라고 생각해요. 같은 역할을 하는 메서드들이 유사한 네이밍 패턴을 따른다면, 코드의 가독성이 훨씬 좋아지니까요!
There was a problem hiding this comment.
네 답변 감사합니다! 참고하여 네이밍에 대해 더 고민해보겠습니다!
src/main/java/InputView.java
Outdated
| import java.util.List; | ||
| import java.util.Scanner; | ||
|
|
||
| public class InputView { |
There was a problem hiding this comment.
InputView에서 입력과 이름 검증, 그리고 Car 객체 생성의 역할을 하고 있어요.
다음 주차에 나오는 MVC 패턴에 대해서 학습해보신 후에 리팩토링하면 더 깔끔하게 역할을 분리할 수 있을 것 같아요.
추가로, 이름 검증을 view에서 하신 이유가 있을까요?
There was a problem hiding this comment.
검증을 어디에서 하는 것이 좋을지 고민하다가, 입력과 함께 검증하는 방식이 익숙하여 InputView에서 작성하게 되었습니다.
그러나 MVC 패턴을 학습한 후, View는 사용자 입력을 받아 전달하는 역할만 수행해야 하며, 데이터의 검증 로직은 Controller나 Domain에서 처리하는 것이 적절하다는 것을 알게 되었습니다.
-
View → 입력과 출력 담당(가공 없이 Controller로 전달)
-
Controller → 입력된 데이터를 검증하고, Model에 전달
-
Model → 검증된 데이터를 바탕으로 비즈니스 로직 실행
따라서, 입력값의 유효성 검사는 InputView가 아니라 Controller에서 수행하는 것이 더 적절하다고 판단하였습니다.
리팩토링에서 수정하였습니다! 좋은 피드백 감사합니다!
src/main/java/InputView.java
Outdated
| } | ||
|
|
||
| public void validateInput(String carName) throws Exception { | ||
| if (carName.length() > 5) { |
There was a problem hiding this comment.
길이를 나타내는 "5"도 상수화하면 어떨까요?
그리고 정빈님만의 상수화 기준이 있으신지 궁금합니다!
There was a problem hiding this comment.
네 길이를 나타내는 "5"도 상수화하였습니다.
저는 자주 사용되는 값을 상수화 하는 것 같습니다. 그러나 길이를 나타내는 "5" 역시도 중요한 조건이기 때문에 상수화가 적절한 것 같습니다.
상수화는 가독성 향상, 유지보수 편리, 코드 일관성 유지에 좋은 방법이라고 생각합니다.
좋은 피드백 감사합니다!
src/main/java/InputView.java
Outdated
| public int inputRaceRounds() { | ||
| System.out.println("시도할 회수는 몇회인가요?"); | ||
| int rounds = scanner.nextInt(); | ||
| System.out.println(); | ||
| return rounds; | ||
| } |
There was a problem hiding this comment.
네 참고하여 시도 횟수의 입력 검증을 추가하였습니다.
검증을 추가하면서 다음 두 가지 사항을 고려하였습니다.
- 숫자 입력 검증 -> 숫자가 아닌 값이 입력될 경우
- 횟수 범위 검증 -> 0이하의 값이 입력될 경우
추가적으로 고려할 입력 검증이 있다면 피드백 부탁드립니다!
public int promptForValidNumber() {
while (!scanner.hasNextInt()) {
System.out.println("잘못된 입력입니다. 숫자를 다시 입력해주세요.");
scanner.next();
}
return scanner.nextInt();
}public int ensurePositiveRounds(int rounds) {
if (rounds <= 0) {
throw new IllegalArgumentException("시도할 회수는 1이상이어야 합니다.");
}
return rounds;
}
src/main/java/InputView.java
Outdated
| this.racingGame = racingGame; | ||
| } | ||
|
|
||
| public void inputCarNames() throws Exception { |
There was a problem hiding this comment.
메서드에 throws Exception을 추가하여 예외 처리를 적용해보려 하였습니다.
그러나 모든 예외를 Exception으로 처리하면, 어떤 종류의 예외가 발생했는지 명확하지 않다는 문제점이 있다는 것을 생각하지 못하였습니다.
예외 처리에 관해 더 공부하고 적용하도록 하겠습니다!
src/main/java/InputView.java
Outdated
|
|
||
| public void validateInput(String carName) throws Exception { | ||
| if (carName.length() > 5) { | ||
| throw new Exception("자동차 이름은 5자 이하여야합니다."); |
There was a problem hiding this comment.
Exception은 Java 예외 계층에서 최상위에 속해요. 너무 포괄적이라 예외 발생 원인을 명확하게 파악하기 어려울 수 있어요.
이와 같이 자동차 이름 길이 초과되었을 때에 더 구체적인 예외를 사용하는 것은 어떨까요?
There was a problem hiding this comment.
네 구체적인 예외를 사용하는 것이 적절하다고 생각하여 수정하여 리팩토링을 진행하였습니다.
기존에는 Exception을 사용하여 예외를 처리했지만,
**"자동차 이름 길이 초과"**라는 특정 예외 상황을 명확하게 표현하기 위해 IllegalArgumentException을 사용하는 것으로 변경하였습니다.
public void validateInput(String carName) {
if (carName.length() > NAME_LENGTH) {
throw new IllegalArgumentException("자동차 이름은 5자 이하여야합니다.");
}
}
src/main/java/RacingGame.java
Outdated
| import java.util.*; | ||
| import java.util.stream.Collectors; | ||
|
|
||
| class RacingGame { |
There was a problem hiding this comment.
검수 과정에서 실수로 제거한 것 같습니다. 다음 과제에서는 꼼꼼하게 확인 후에 제출하도록 하겠습니다. 감사합니다!
<접근 제어자>
-
메서드가 외부에서 호출될 필요가 있는지? (public vs private)
-
같은 패키지 내에서만 접근할 수 있도록 제한하는 것이 적절한지? (default, protected)
-
내부적으로만 사용되는 메서드라면 private으로 접근을 최소화하는 것이 좋다는 점을 항상 고려하겠습니다.
src/main/java/RacingGame.java
Outdated
| } | ||
|
|
||
| public void gameStart(int rounds) { | ||
| Random random = new Random(); |
There was a problem hiding this comment.
메서드 내에서 Random 객체를 생성하면 어떤 장단점이 있을까요?
There was a problem hiding this comment.
네 Random 객체를 메서드 안에서 생성한 이유는, 난수가 필요한 곳에서 바로 만들면 코드가 이해하기 쉬울 것 같았기 때문입니다.
하지만, 메서드가 호출될 때마다 새로운 Random 객체가 생성되면 메모리 낭비가 발생할 수 있다는 문제점을 생각하지 못했습니다.
그래서 Random 객체를 한 번만 생성하고, 여러 곳에서 재사용하는 방식으로 수정했습니다.
src/main/java/RacingGame.java
Outdated
| public List<Car> getWinner() { | ||
| int maxPosition = cars.stream().mapToInt(Car::getPosition).max().orElse(0); | ||
| winnerCars = cars.stream().filter(car -> car.getPosition() == maxPosition).collect(Collectors.toList()); | ||
| return winnerCars; | ||
| } |
There was a problem hiding this comment.
| public List<Car> getWinner() { | |
| int maxPosition = cars.stream().mapToInt(Car::getPosition).max().orElse(0); | |
| winnerCars = cars.stream().filter(car -> car.getPosition() == maxPosition).collect(Collectors.toList()); | |
| return winnerCars; | |
| } | |
| public List<Car> getWinner() { | |
| int maxPosition = cars.stream() | |
| .mapToInt(Car::getPosition) | |
| .max() | |
| .orElse(0); | |
| return cars.stream() | |
| .filter(car -> car.getPosition() == maxPosition) | |
| .collect(Collectors.toList()); | |
| } |
이와 같이 수정하면 단계별로 표현되어서 더 읽기 쉬울 것 같아요 :)
There was a problem hiding this comment.
추가로 반환되는 List<Car>는 가변 객체인가요 불변 객체인가요?
There was a problem hiding this comment.
네 감사합니다! 단계별로 수정하였습니다!
현재 코드에서는 가변 객체가 반환되고 있습니다. 가변 리스트는 추가, 삭제, 수정하는 것이 가능하기에 데이터의 무결성이 깨질 위험이 있는 것 같습니다.
예기치 못하게 우승자의 리스트가 바뀌는 상황을 방지하기 위해서 불변 리스트로 반환해주는 개선이 필요해 보입니다.
가변 객체와 불변 객체를 생각해보며 리팩토링 진행하였습니다. 좋은 피드백 감사합니다!
| public void move(int randomValue) { | ||
| if (randomValue >= MOVE_TRIGGER) { | ||
| public void move() { | ||
| int random = randomValue.nextInt(10); |
There was a problem hiding this comment.
현재 Random 객체 이름이 randomValue인데, 오히려 생성된 난수를 의미하는 변수가 randomValue가 되는게 더 명확할 거 같다고 생각하는데 어떠세요?
There was a problem hiding this comment.
네 현재 randomValue라는 변수명이 난수를 생성하는 객체를 의미하고 있지만, 실제로 난수를 저장하는 변수로 사용하면 더 직관적일 것 같습니다.
그리고 랜덤 객체는 randomGenerator로 수정하여 '난수를 생성하는 객체'라고 표현하는 것이 좋을 것 같습니다! 좋은 피드백 감사합니다!
| } | ||
| } | ||
|
|
||
| private void printRoundResults() { |
There was a problem hiding this comment.
printRoundResults()는 라운드 결과를 출력하는 메서드인데, 현재 ResultView가 있음에도 Service에서 처리하신 이유가 있을까요?
혹시 ResultView로 분리하는 것에 대해 어떻게 생각하시나요?
There was a problem hiding this comment.
라운드 결과 출력이 단순 출력보다는 게임의 핵심이라는 생각에 Service에 작성하였습니다. 그러나 MVC 패턴으로 생각했을 때, printRoundResults()는 ResultView에서 처리하는 것이 적절한 방법인 것 같습니다.
저도 리팩토링 과정에서 printRoundResults()를 어디에 작성할지 고민이 있었는데, 지윤 리뷰어님 피드백이 올바른 것 같습니다. 좋은 피드백 감사합니다!
| public void carList(List<String> carNames) { | ||
| cars = carNames.stream() | ||
| .map(Car::new) | ||
| .collect(Collectors.toList()); |
There was a problem hiding this comment.
현재 carList() 메서드는 List<String>을 받아 cars 리스트를 새롭게 할당하는 방식이네요! 이 경우 리스트의 불변성이 유지될까요, 아니면 가변적일까요?
참고하면 좋을 자료 공유드립니다!
일급컬렉션
There was a problem hiding this comment.
현재 carList() 메서드는 List을 받아 cars 리스트를 새롭게 할당하는 방식입니다.
이 경우 리스트의 불변성이 유지되지 않고 가변적인 것 같습니다...! collect(Collectors.toList())는 기본적으로 ArrayList를 반환하기 때문에 리스트가 변경될 수 있습니다.
리스트를 불변으로 만들려면
- Collections.unmodifiableList() 사용
- List.copyOf() 사용
- 일급 컬렉션 활용
의 방법이 있습니다. 특히 일급 컬렉션 활용 방법은 단순히 리스트를 불변으로 만드는 것뿐만 아니라, 컬렉션의 검증, 검색 등의 기능을 하나의 클래스에서 처리할 수 있어 코드의 유지보수성과 확장성을 높이는 장점이 있습니다.
그동안 List 같은 컬렉션을 직접 다루는 것이 자연스럽다고 생각했는데, 별도의 클래스로 감싸면 관리가 훨씬 편리하고, 코드의 책임이 분리된다는 점이 인상적이었습니다! 좋은 피드백 감사합니다!!
|
|
||
| private List<Car> cars = new ArrayList<>(); | ||
| private List<Car> winnerCars = new ArrayList<>(); | ||
| private int rounds; |
There was a problem hiding this comment.
처음에는 rounds를 여러 메서드에서 사용할 것이라고 예상했지만,
실제로는 gameStart() 내부에서만 사용되므로 필드가 아닌 지역 변수로 사용하는 것이 더 적절한 것 같습니다.
불필요한 필드를 줄이면 코드의 명확성이 높아지고 유지보수가 쉬워지므로, rounds 필드는 제거하는 것이 바람직한 것 같습니다.
소중한 시간을 내어 도와주셔서 진심으로 감사드립니다!
2주차
안녕하세요, 이지윤 리뷰어님! 😊
2주차 1,2,3 단계 과제 제출합니다. 편하신 시간대에 리뷰해주시면 정말 감사하겠습니다!
이번 과제를 진행하면서 단일 책임 원칙(SRP)과 MVC 패턴을 명확하게 구분하는 것에 대해 고민이 많았습니다.
현재 코드에서 특히 SRP를 위반한 부분이 있다면 피드백 부탁드립니다!
<질문>
현재 코드에서는 따로 controller를 만들지 않고, main에서 RacingGame, InputView, ResultView를 직접 호출하고 있습니다. controller를 따로 만들지 않고 Main에서 게임을 진행하면 어떤 문제가 발생하는 걸까요?
controller의 책임 범위를 어디까지로 생각하면 좋을지 궁금합니다. controller는 흐름만 관리하고, 입력 검증과 자동차 생성은 InputView에서 담당하도록 설정하는 것이 맞을까요?
단일 책임 원칙을 더 잘 지키기 위해 가장 중요한 기준이 무엇일까요?