Conversation
작동 순서대로 기능 목록 작성 테스트 케이스 작성 리펙토링 요소 작성 예정
리펙토링 요소 추가(주석 추가)
| throw new IllegalArgumentException(NO_CAR_ERROR.getErrorMessage()); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
여러 상황에 대해서 꼼꼼하게 검증을 진행하신 부분이 정말 좋네요!
글자 수 제한이나, 원하시는 여러 상황이 정규표현식으로도 가능하니 두 가지 모두 해보시는 것도 도움이 될 것 같아요!
하지만, 작성하신 코드를 보니 validateBlank 같은 함수는 여러 곳에서 재활용 가능한 것으로 보이니 코드의 재활용성 측면에서나 검증의 엄밀성에서는 지금 작성해주신 부분이 더 좋은 걸로 보이네요! 저도 배워갑니다! bb
| // Console이 입력을 못 받는 상황을 IllegalArgumentException 으로 통일 | ||
| throw new IllegalArgumentException(ONLY_WHITESPACE_ERROR.getErrorMessage()); | ||
| } | ||
| } |
There was a problem hiding this comment.
try catch를 일관된 예외를 처리하기 위해서 사용하는 것이 좋네요!
저는 보통 상정할 수 없는 예외를 처리할 때만 사용하려고 자제하고 있는데, 이런 식으로 일관된 처리를 보장하는 패턴으로써 사용하는 것이 훨씬 좋아보입니다! 많이 배웠습니다!
There was a problem hiding this comment.
패키지 명에 따라서 해석했을 때는 Cars를 도메인(데이터)로 사용하고자 하신것으로 이해했습니다.
그런데 코드에는 여러가지 비즈니스로직이 섞여있는 것으로 보아 조금 혼동되네요.
- 비즈니스 로직과 데이터 저장이 혼용
이 경우 한 파일안에 다수의 책임이 주어지기 때문에 "단일 책임 원칙"을 고려한다면 수정이 필요할 것 같습니다.
domain의 cars 클래스에서는 데이터의 저장만을 담당하고, 따로 race의 기능들을 담당하는 파일로 분리하면 좋을 것 같아요! - 경주 별 출력도 cars에서 담당하고 있음
mvc 패턴을 적용하려고 하신 것으로 보입니다. 그렇다면 출력은 view에서 담당해주는 게 조금 더 자연스럽다고 생각해요! - 내부 클래스 car
사실 말씀하신 논리에 따르면 car가 Cars에서만 사용되기 때문에 내부클래스로 선언하셨다는 부분에 대해서는 충분히 이해가 가서 저도 합당하다고 생각했습니다. 다만, 가독성이나 재활용성, 테스트 등과 같이 여러 부분을 고려하다보면 해당 부분에서 조금 불편한 점을 느끼시지 않을까 생각합니다. 두 코드를 각각의 클래스로 관리하는 방법도 염두에 두시면 좋을 것 같아요!
덧붙여서 내부에서만 사용하실 거라면 private static으로 해두시는 것이 안전할 것 같습니다!
There was a problem hiding this comment.
- 맞아요 가장 고민이 많은 부분이에요. 1주차에 비해 클래스를 많이 분할했지만, 여전히 특정 클래스(지금은 Cars 클래스)가 많이 무거운 느낌이 많이 있습니다. SRP를 한번 공부해서 적용해보겠습니다:)
- 엇 이 부분은 출력이라기보단 문자열을 view에게 반환하고 출력은 view 가 담당하도록 구현했습니다. 주신 코멘트를 제가 잘 이해한 게 맞는지 모르겠네요😂 혹시 답변이 적절하지 않다면 말씀해주세요!:)
- 제 불편함을 정확히 아셨네요 ㅎㅎ 이후에 Car 클래스를 사용하는 다른 클래스가 있다면 분명 분리하는 게 좋을 것 같아요! 그리고 public이 아니라 private!! 중요한 부분 코멘트 감사합니다:)
| - [x] 빈 문자열("") 또는 1개 이상의 공백(예: " ") 입력 시 예외 발생 `(판단)` | ||
| - [x] 자동차이름 입력값이 공백만으로 이루어진 경우 `(판단)` | ||
| - [x] 시도할횟수 입력값이 공백만으로 이루어진 경우 `(판단)` | ||
| - [x] 자동차 이름이 5자 초과하는 경우 |
There was a problem hiding this comment.
자동차 이름은 5자 이하 가능한거라, 6자 초과하는 경우로 에러처리해야해요!
There was a problem hiding this comment.
이름이 5자 이하까지 허용되기 때문에, 5자를 초과하는 경우(6 이상인 경우) 예외를 처리하도록 했습니다:)
| new InputView(), | ||
| new OutputView() | ||
| ); | ||
| controller.run(); |
There was a problem hiding this comment.
controller의 run()메서드와 ApplicationRunner의 run() 메서드의 이름이 같아 혼동될 수 있을거 같아요.
상속(오버라이딩)이 아닌 경우에는 서로 다른 이름으로 구분해주면 좋을 것 같습니다!
There was a problem hiding this comment.
추가로, Controller가 현재 Service, InputView, OutputView 총 3개의 객체를 생성자 주입받고 있습니다. 이렇게 되면 이 중 하나라도 변경될 경우 Controller 코드까지 영향을 받아 결합도가 높아집니다. 따라서 Controller는 가능한 한 적은 의존성을 가지는 것이 좋습니다. 일반적으로 Service는 생성자 주입을 통해 사용하고, View는 생성자 주입 대신 메서드 호출 방식으로 사용한다고 합니다.
There was a problem hiding this comment.
적당한 메서드 이름이 생각이 안나서 그냥 똑같이 해버렸는데,, 딱 걸렸네요ㅋㅋ
이름 짓는 게 젤 힘든 느낌😂
생성자 주입은 자신있게 설계한 부분인데 결합도나 의존성 부분은 생각도 못했네요.. 역시 끝없는 리펙토링..🥲
궁금한 부분이 있는데,
- 변경된다면 생성자 호출 방식, 다른 View 사용 가능성 등이 생각 나는데 혹시 지영님께서 고려하시는 다른 변경 예시가 있으신가요?
- '일반적으로 Service는 생성자 주입을 통해 사용하고, View는 생성자 주입 대신 메서드 호출 방식으로 사용한다'의 구체적인 이유가 너무 궁금하네요!! 너무너무 좋은 피드백이라 생각이 듭니다.. 이런 내용이 이해되면 뭔가 뻥 뚫리는 느낌이에요 ㅎㅎ
3 participants
java-racingcar-precourse
스스로 판단한 내용은
(판단)으로 표기한다.구현 기능 목록
기능 작동 순서대로 작성
1. 자동차의 이름을 입력받고 검증한다.
(판단)(판단)2. 입력 받은 자동차의 이름을 분리하고 검증한다.
(판단)3. 자동차를 만들어 이름을 부여한다.
4. 시도할 횟수를 입력받고 검증한다.
(판단)(판단)(판단)5. 자동차 경주를 진행하면서 과정을 문자열로 저장한다.
6. 우승자를 결정해 우승자 목록을 문자열로 저장한다.
7. 자동차 경주 진행 과정과 우승자 목록을 출력한다.
테스트 코드
정상 입력 케이스
(판단)(판단)(판단)(판단)예외 발생 입력 케이스
(판단)(판단)(판단)(판단)(판단)(판단)(판단)(판단)리펙토링