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

[자동차 경주 게임] 김형준 미션 제출합니다, #525

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

rvlwldev
Copy link

커밋 내용 유실

Copy link
Member

@Choi-JJunho Choi-JJunho left a comment

Choose a reason for hiding this comment

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

잘봤습니다!
개인적으로 우려됐던 부분을 크게 분류해서 말씀드리자면

  • 여러 타입을 가지는 Enum
  • 너무 많은 역할을 가지는 TypeParser
  • Validation을 입력부에서만 수행하는 부분

이라는 부분이 있었습니다.

  • 여러 타입을 가지는 Enum의 경우

해당 Enum의 정합성이 떨어지는 우려가 있었습니다.

  • 너무 많은 역할을 가지는 TypeParser

역할의 분리가 흩어지는것 같아 흐름을 읽기 어려운것같습니다 ㅠㅠ

  • Validation을 입력부에서만 수행하는 부분

저는 개인적으로 도메인을 구성할때 해당 도메인을 어디에서나 사용해도 동일한 동작을 수행할 수 있어야한다는 것을 고려하고 있어 validation 또한 어느곳에서 호출해도 동일하게 동작해야한다고 생각합니다.

해당 부분을 고려했을 때 validation을 도메인단에서 수행하는건 어떨까요?

src/main/java/racingcar/Message/Rule.java Show resolved Hide resolved
src/main/java/racingcar/Util/TypeParser.java Show resolved Hide resolved
src/main/java/racingcar/View/Relay.java Show resolved Hide resolved
src/main/java/racingcar/Util/TypeParser.java Show resolved Hide resolved

public class Validation {

public void validInputCars(String userInput) {

Choose a reason for hiding this comment

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

validate를 객체 생성할때 체크하는 걸로 바꾸면 좋을 것 같아요

Copy link
Author

Choose a reason for hiding this comment

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

잘 이해가 안됐어요 ㅠㅠ

지금은 Input에서 Validation 를 상속받아서 차량목록을 입력받고 그 문자열의 예외를 확인하는데요
List 를 받아서 예외를 확인하는게 좋을것 같다는 말씀이신가요??

Copy link

@JaegeonYu JaegeonYu left a comment

Choose a reason for hiding this comment

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

고생하셧습니다

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.

3 participants