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

Open
wants to merge 33 commits into
base: oownahcohc
Choose a base branch
from

Conversation

oownahcohc
Copy link

집중한 부분

  • MVC 패턴을 지키면서 코드 작성
  • 객체지향적인 코드 작성
  • 원시값 wrapping 을 위한 VO 클래스 활용
  • 불변 컬렉션과 더 나은 상태, 행위 관리를 위한 일급 컬렉션 활용

마음에 안드는 점

  • 현재 잘못된 값 입력 시 재입력을 위해 try-catch 를 사용중인데 다른 방법이 있을지 궁금(고민중)

@oownahcohc
Copy link
Author

  • Name VO 에서 공백 validate 로직 수정
  • size 테스트 메서드 hasSize() 로 수정
  • 콘솔 출력 확인 (System.println.out) 삭제

Copy link

@ch0ijihwan ch0ijihwan left a comment

Choose a reason for hiding this comment

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

리뷰 남겨드립니다.

수고하셨습니다~👍
처음 한거라고 믿기지가 않습니다.

그리고 전반적으로 코멘트 드리고 싶은 것 적어둡니다!

  • final 키워드 사용하시는 것 통일성있게 해주시면 좋을 것 같습니다. 적응이 힘드시다면 인텔리제이 설정으로 매서드 추출 단축키 사용시 기본값으로 final이 명시되게 해주는 설정이 있어요~
  • 그리고 vo 형태로 객체를 감싸려고 시도하신 것은 잘하신 것 같아요. 하지만 현재 vo들은 반쪽짜리 vo라고 생각됩니다. vo의 특징에 대해 좀 더 찾아보시고 전반적으로 리팩토링 해주세요~~
  • 테스트 코드 작성시 @DisplayName 을 좀 더 자세하게 적어주시면 좋을 것 같습니다. 저는 레이싱카를 한번 해서 대강 이해가 되지만, 그렇지 않은 사람들이 봤을 때 이해가 안될 수도 있다고 생각합니다. 어떤 값이 주어지고, 어떤결과가 나오는지 명확하게 작성해주세요!

그리고

현재 잘못된 값 입력 시 재입력을 위해 try-catch 를 사용중인데 다른 방법이 있을지 궁금(고민중)

이 부분에 대해서는 의견을 남겨드리자면, 잘못된 값을 입력받은 경우 예외를 발생하는데 이를 처리하기위한 try-catch 는 무조건적으로 필요하다고 생각합니다.
그리고 try-catch 를 사용하는 것이 나쁜 것은 아닙니다. 이런 고민을 하신 이유가 있을까요??


리뷰와 별개로 궁금한 부분들 질문 달아두었는데, 답변 달아주시면 좋겠습니다~

리뷰가 이해가 안되시거나 직접적으로 물어보고 싶으시면 코멘트남기시거나 슬랙 dm 보내주세요!

src/main/java/gmbs/model/car/vo/Position.java Show resolved Hide resolved
src/main/java/gmbs/model/car/vo/Position.java Show resolved Hide resolved
src/test/java/gmbs/model/car/vo/PositionTest.java Outdated Show resolved Hide resolved
src/main/java/gmbs/model/car/collection/Cars.java Outdated Show resolved Hide resolved
src/test/java/gmbs/model/game/RacingGameTest.java Outdated Show resolved Hide resolved
src/test/java/gmbs/model/game/RacingGameTest.java Outdated Show resolved Hide resolved
src/test/java/gmbs/model/game/RacingGameTest.java Outdated Show resolved Hide resolved
@oownahcohc
Copy link
Author

이 부분에 대해서는 의견을 남겨드리자면, 잘못된 값을 입력받은 경우 예외를 발생하는데 이를 처리하기위한 try-catch 는 무조건적으로 필요하다고 생각합니다.
그리고 try-catch 를 사용하는 것이 나쁜 것은 아닙니다. 이런 고민을 하신 이유가 있을까요??

try-catch 는 단순히 가독성 측면에서 많이 손해를 본다고 생각해서 였습니다 ㅎㅎ

Copy link

@ch0ijihwan ch0ijihwan left a comment

Choose a reason for hiding this comment

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

try-catch 는 단순히 가독성 측면에서 많이 손해를 본다고 생각해서 였습니다 ㅎㅎ

아마 이렇게 생각하시는 이유가 클린 코드를 보셨기 떄문이라고 생각합니다..!
저도 동일한 생각을 했었습니다.

그런데 try-catch 자체가 가독성을 낮추는게 아니라, 무분별하게 사용하는 것이 가독성을 낮추는 것입니다..!

src/main/java/gmbs/model/car/vo/Position.java Show resolved Hide resolved
src/test/java/gmbs/model/car/CarTest.java Show resolved Hide resolved
src/test/java/gmbs/model/car/collection/CarsTest.java Outdated Show resolved Hide resolved
src/test/java/gmbs/model/car/collection/CarsTest.java Outdated Show resolved Hide resolved
src/test/java/gmbs/model/car/vo/NameTest.java Show resolved Hide resolved
src/test/java/gmbs/model/car/vo/NameTest.java Show resolved Hide resolved
Copy link

@ch0ijihwan ch0ijihwan left a comment

Choose a reason for hiding this comment

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

일단 저번 리뷰 반영이 다 되어있지 않아서, 그외에 보이는 것들만 리뷰해드립니다.

그리고 이퀄즈, 해시코드를 재정의 해준 클래스들을 확인해보았는데, vo라고 패키징해둔 클래스들이 vo 가 아니라고 생각되네요!

vo 에 대해 공부 및 정리 해보시고 리팩토링 후 연락주세요~
궁금한점 있으면 언제든지 연락주세요

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.

None yet

2 participants