Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4단계 - 자동차 경주(우승자) , 3단계 피드백 내용 수정 #5735

Open
wants to merge 23 commits into
base: jeonghyeonkwon
Choose a base branch
from

Conversation

jeonghyeonkwon
Copy link

안녕하세요!

4단계 과제 및 3단계 피드백 해주신 내용 보완해서 제출 합니다.

꼼꼼히 검토해주셔서 감사드립니다!

그리고 3단계 말씀해주신 내용 중
"다음 단계를 진행하실 때는 JavaRacingCar에서 ResultView를 없애보는건 어떨까요? 결국 해당 클래스도 게임을 실행하는 도메인 클래스로 보여 요구사항인 ui로직을 분리한다에 위배되는 것으로 보이네요 "
의 UI를 어떻게 분리하면 좋을 지에 대한 방법들을 생각해 봤는데 아직 감이 잘 잡히지 않아서요ㅠㅠ

방법 1. JavaRacingCar의 리턴하는 결과를 반환하는 메소드를 ResultView에 넣어 분리

  • Round라는 클래스를 생성해서 JavaRacingCar의 List 를 반환하는 메소드 생성 해서 ResultView에 주입
  • 각 데이터들만 리턴 받아 다음 객체에 전달하는 방식( main 메소드에서 각각 다음 단계 실행 )
public static void main(String[] args){
  InfoDto dto = new InputView().getInfo();
  JavaRacingCar javaRacingCar = new JavaRacingCar(dto);
  // ... javaRacingCar의 모든 경기 종료 후

  List<Round> round = javaRacingCar.getRoundList();
  ResultView resultView = new ResultView(round);
}

방법 2. JavaRacingCar -> Round -> ResultView 각각의 내부에서 호출해서 JavaRacingCar에서 ResultView 분리

  • Round 클래스 내부에서 ResultView 호출
class JavaRacingCar{

  private void start(){
     for(int i =0; i< this.tryCount; i++){
         Round round = new Round(...);   
     }
  }
}

class Round{
    void endRound(...){    
       ResultView.printEndRound(...)
   }
}

에 비슷한 방법들 뿐이라 말씀 부탁드립니다ㅠㅠ

- execution() 메소드 제거 생성 시 실행 하도록
- isMove 관련 테스트 코드 수정(3, 4, 9 엣지 케이스만)
- validateNegativeNumber메소드 static 제거
- 이번 주제에서 Step 관련 내용 없어 Enum step 제거
- 생성자에 차 대수, 시도 횟수 받는 로직으로 변경 (테스트 코드 포함 수정)
- 차 대수, 시도 횟수 관련 메소드 private로 변경
- readme 요구사항 리스트 수정
- 구분자 상수화
- split, 이름 검증 메소드 분리
- 기능 구현 리스트 차 객체 검증 리스트 추가
- 랜덤 값에 따른 차 이동 검증
- 생성된 차 객체 이름, 생성 시 위치, 이동 검증
- split, toArray 메소드 분리
- initCars 실행 코드 수정
- fix: 이름 길이 범위(이상 -> 초과)
- 메인 컨트롤러에서 Dto로 정보 받는 로직으로 수(테스트 코드 포함)
- InputView 파일 생성
- JavaRacingCar 생성자 ResultView 객체 추가하여 받도록 로직 수정
- ResultView static 메소드 제거
Copy link
Member

@jinyoungchoi95 jinyoungchoi95 left a comment

Choose a reason for hiding this comment

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

안녕하세요 정현님 😄

4단계 잘 진행해주셨네요 👍 몇가지 코멘트 남겨두었는데 확인부탁드려요 :)

궁금하거나 고민이 되는 부분이 있으시다면 언제든 pr 코멘트 또는 dm으로 요청 부탁드립니다.
감사합니다 🙇‍♂️

Copy link
Member

Choose a reason for hiding this comment

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

미션내에서 혹시나 의도하신바가 아니라면 각 단계를 분리하겨 구현하는 것이 아닌 하나의 코드로 단계마다 기능을 추가하며 미션을 진행해주시면 됩니다.

Copy link
Member

Choose a reason for hiding this comment

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

코멘트에 남겨주신 방법 중 1번처럼 해주시면 됩니다. 자동차경주라는 모든 실행 흐름(도메인 코드)에 view가 들어가지 않도록 구현하는 것이 미션의 목적으로 봐주시면 될 것 같아요 :)

Comment on lines +18 to +28
public RacingCar go() {
location += DEFAULT_MOVE_VALUE;
return this;
}

public boolean isMove(int value) {
if (MIN_STANDARDS_VALUE <= value && value <= MAX_STANDARDS_VALUE) {
return true;
}
return false;
}
Copy link
Member

Choose a reason for hiding this comment

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

움직임 여부를 외부에서 isMove를 호출하는 형태가 되어있어 캡슐화가 되어있지 않은 것 같아요. 외부 호출에서 isMove()를 누락하면 의도한 동작이 나오지 않는거죠. isMovego내부로 넣도록 변경이 필요해보이네요 :)

Comment on lines +24 to +27
if (MIN_STANDARDS_VALUE <= value && value <= MAX_STANDARDS_VALUE) {
return true;
}
return false;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (MIN_STANDARDS_VALUE <= value && value <= MAX_STANDARDS_VALUE) {
return true;
}
return false;
return MIN_STANDARDS_VALUE <= value && value <= MAX_STANDARDS_VALUE

if 문이 별도로 필요가 없어보여요 :)

import java.util.List;
import java.util.Random;

public class RacingUtil {
Copy link
Member

Choose a reason for hiding this comment

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

util 클래스의 경우 객체 생성이 불필요하기 때문에 생성자는 private으로 막아주는 것이 좋습니다 😄

@@ -0,0 +1,20 @@
package javaracingcarwinner.dto;

public class RacingInfoDto {
Copy link
Member

Choose a reason for hiding this comment

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

클린코드 2장 의미있는 이름

  • 의미 있게 구분하라
    • Info 나 Data 는 a, an, the 와 마찬가지로 의미가 불분명한 불용어다.

info보다는 더 명확한 네이밍을 사용해보는건 어떨까요?

Comment on lines +13 to +18
public class JavaRacingCar {
private static final String SPLIT_DELIMITER = ",";
private static final int MAX_NAME_LENGTH = 5;

private final List<RacingCar> cars = new ArrayList<>();
private final ResultView resultView;
Copy link
Member

Choose a reason for hiding this comment

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

위에서 말씀드린것처럼 해당 클래스는 라운드에 맞게 자동차를 움직이고 우승자를 결정하는 도메인 클래스이므로 해당 클래스 내에 있는 View 로직은 모두 제거해주세요 :)

Comment on lines +56 to +58
private int random() {
return RacingUtil.random();
}
Copy link
Member

Choose a reason for hiding this comment

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

여기서 random을 사용해도 될 것 같은데 불필요한 래핑이 되는 것 같네요 🤔

Comment on lines +75 to +79
private void validateName(String name) {
if (name.length() > MAX_NAME_LENGTH) {
throw new IllegalArgumentException("5자 초과");
}
}
Copy link
Member

Choose a reason for hiding this comment

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

자동차의 이름이 5자로 제한되는 것은 RacingCar가 가져야하는 책임이 아닐까요?

import java.util.List;
import java.util.stream.Collectors;

public class JavaRacingCar {
Copy link
Member

Choose a reason for hiding this comment

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

해당 클래스의 역할은 JavaRacingCar보다는 RacingGame에 더 가까워보이네요 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants