Conversation
- Random 클래스의 ints 메서드 사용 - 추후 연산을 위해 목표값을 배열로 생성
- 잘못된 예외 처리 위치 수정 - 스타일 수정 (`-` -> `##`)
- 예외 처리는 검사 기능에서
- 이전 스트라이크 기록이 그대로 남아있는 에러에 대한 fix
- 문자열은 Message 클래스로 분리 - 검사 기능은 Validator 클래스로 분리
isStrike() -> isball()
# Conflicts: # README.md
UiHyeon-Kim
left a comment
There was a problem hiding this comment.
2주 차도 고생하셨습니다~
PR에 올려주신 내용에 대해 답변해 드리자면,
MVC 에서 뷰 클래스는 입출력과 간단한 검증 정도의 역할을 할 수 있다고 생각합니다.
즉, 뷰에서 입력과 출력, 간단한 검증을 맡고 컨트롤러가 이 값을 받아 모델로 전달하는 것이 옳습니다.
그렇다면 모델에서 생성된 값도 컨트롤러가 받아 뷰로 전달하는 게 옳겠죠?
MVC 작성 방식에 대해서는 뷰 클래스에 따로 작성해 뒀으니 읽어보시면 도움이 될 것 같아요.
| public class BaseballGameController { | ||
| private final BaseballGame game = new BaseballGame(); | ||
| private final InputView inputView = new InputView(); | ||
| private final OutputView outputView = new OutputView(); |
There was a problem hiding this comment.
컨트롤러에서 필드에 다른 클래스 객체를 생성한 이유가 있나요?
현재 방식이 잘못된 방식인 건 아니고, 본인만의 의도가 있었다면 상관은 없습니다!
| FAIL, | ||
| SUCCESS, | ||
| RESTART | ||
| } |
There was a problem hiding this comment.
게임 상태를 enum 클래스로 구분한 게 좋네요~
|
|
||
| Validator.inputValidate(input); | ||
|
|
||
| baseballGame.createUserNumbers(input); |
There was a problem hiding this comment.
MVC 패턴에서는 뷰와 모델이 직접적으로 상호작용 하면 안됩니다.
아키텍처 패턴을 적용한 이유가 각 레이어의 역할을 구분하기 위해서인데,
현재 코드에서는 뷰가 모델에 직접적으로 의존하게 되어서 결합도가 높아지게 됩니다.
이처럼 작성하면 생기는 문제점은
- 테스트 코드를 작성하기 힘들다.
- 관심사 분리가 안된다.
- 뷰를 변경할 시 모델도 같이 변경해야할 수 있고, 모델 변경시 뷰를 같이 변경해야 할 수도 있다.
지금 생각나는 건 이정도가 있고, input을 컨트롤러에서 받고 컨트롤러에서 모델로 전달하는 방향을 고려해 보세요!
| } else { | ||
| game.setStatus(GameStatus.FAIL); | ||
| System.out.println(String.format(Message.MESSAGE_BALL_STRIKE_FORMATTED, ball, strike)); | ||
| } |
There was a problem hiding this comment.
볼과 스트라이크 모두 0인 것은 FAIL로 판단한 건 좋다고 생각하는데,
볼이나 스크라이크 하나만 0인 것도 FAIL로 하니 바로 파악하기가 조금 어려웠습니다.
여기서 성공과 실패라는 틀을 깨고 조금 더 파악하기 쉬운 상태를 만들어보는 건 어떨까요?
제 개인적인 생각이라 한 번 고민만 해보시는 것을 추천드립니다..
| int option = scanner.nextInt(); | ||
| if (option == 1) { | ||
| game.createTargetNumber(1, 9); | ||
| game.setStatus(GameStatus.RESTART); // 의미가 있나? |
There was a problem hiding this comment.
15번 라인부터 여기까지도 마찬가지로 뷰와 모델이 직접적으로 연결되어 유지보수성이 많이 떨어지게 됩니다.
MVC 패턴의 각 계층 역할을 아래 적어드리니 한 번 참고해 보시면 좋겠습니다~
- 모델은 컨트롤러나 뷰에 의존하면 안된다.
- 모델 내부에 컨트롤러 및 뷰와 관련된 코드가 있으면 안된다.
- 뷰는 모델에만 의존해야 하고, 컨트롤러에는 의존하면 안된다.
- 뷰 내부에 모델의 코드만 있을 수 있고, 컨트롤러의 코드가 있으면 안된다.
- 뷰가 모델로부터 데이터를 받을 때는 사용자마다 다르게 보여주어야 하는 데이터에 한해서만 받아야 한다.
- 컨트롤러는 모델과 뷰에 의존해도 된다.
- 컨트롤러 내부에는 모델과 뷰의 코드가 있을 수 있다.
- 뷰가 모델로부터 데이터를 받을 때는 반드시 컨트롤러에서 받아야 한다.
| game.reset(); // targetNumber의 상태 초기화 및 이전 userNumber free | ||
| inputView.getUserNumber(game); | ||
| game.match(); | ||
| outputView.printResult(game); //todo: ball, strike 연산을 view클래스에서? |
There was a problem hiding this comment.
OutputView 에서 하는 것은 큰 연산이 아닌 조건 분기라서 크게 상관은 없다고 생각합니다.
다만 신경 쓰이신다면 조금 깔끔하게 수정은 해볼 수 있을 것 같네요
🤔🪑
InputView에서getUserNumber()?뷰 클래스에서 값을 입력받아 게임 클래스에 전달 (두 가지 책임을 가지고 있다 ? DI ?)
뷰 클래스로 단순 입력만 return하고 이후 게임 클래스에서
create~()호출이 옳은 방향일까?trim 과정은
Validator에서 진행 ?그 외 todo 주석으로 위치가 이상하거나 리팩토링에 대해 남겨놨습니다.
테스트코드는 랜덤값에 대해 어떻게 작성해야될 지 모르겠네요.
결국 테스트 코드 내에서 target이라는 랜덤값에 대해 고정값을 주고
입력값을 테스트 코드에 넣어 결과를 확인하는 방법을?
그렇다면 프로그램이 잘 작동하는지에 대한 로직 검증은
인간이 테스트를 생각해내서 검증해내야되는건지..
🙏
적어도 3일은 코드를 보고 리팩토링하는 경험을 가지고자 했는데
기간이 3주는 되야지 3일을 보네여.. 😅
근데 테스트코드 작성이랑 리팩토링을 제대로 못했..