1단계 미션 제출합니다. #4
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Java17, Junit5, AssertJ 의존성 추가 - .gitignore 업데이트
전반적인 구조 설계
ksy90101
requested changes
May 23, 2024
| import java.util.HashMap; | ||
| import java.util.Map; | ||
|
|
||
| public class CommandFactory { |
There was a problem hiding this comment.
이건 Command에 넣으면 되지 않을까요?
혹시 따로 Factory를 만드신 이유가 있나요?
|
|
||
| public class CommandFactory { | ||
|
|
||
| private final Map<String, Command> commandMap; |
There was a problem hiding this comment.
Map이라는 단어보단 복수형으로 쓰는게 좋을거 같습니다.
Comment on lines
+24
to
+29
| Map.of( | ||
| "1", new PlayGameCommand( | ||
| new ConsoleBaseballGame( | ||
| inputView, outputView, new RandomBallNumbersGenerator())), | ||
| "9", new ExitApplicationCommand(this) | ||
| ) |
There was a problem hiding this comment.
- 이 부분을 따로 분리하면 어떨까요?
- 차후 여기서 1과 9에 대해서 알기가 어려운거 같아요. 상수화를 하면 어떨까요?
Comment on lines
+15
to
+16
| private BallNumbers targetNumbers; | ||
| private boolean isRunning; |
There was a problem hiding this comment.
ConsoleBaseBallGame에서 상태를 가지고 있는게 이상한거 같아요.
ConsoleBaseBallGame은 어떤 역할을 하는건가요?
Comment on lines
+18
to
+21
| checkArgument( | ||
| number >= MIN_BALL_NUMBER && number <= MAX_BALL_NUMBER, | ||
| "BallNumber은 " + MIN_BALL_NUMBER + "~" + MAX_BALL_NUMBER + "사이의 숫자여야 합니다." | ||
| ); |
There was a problem hiding this comment.
다른 생성자 혹은 다른 정적 팩터리 메서드를 만든다면 이 부분은 또 작성해야하지 않나요?
이럴땐 생성자에서 차라리 하면 어떨까요?
| public final class Validator { | ||
|
|
||
| private Validator() { | ||
| // 유틸리티 클래스 |
| // 유틸리티 클래스 | ||
| } | ||
|
|
||
| public static void checkArgument(boolean expression, String message) { |
| BallNumber created = BallNumber.of(number); | ||
|
|
||
| // then | ||
| assertNotNull(created); |
There was a problem hiding this comment.
assertJ를 적극적으로 활용해보면 어떨까요?
| .withMessage("야구 숫자는 1~9사이의 서로 다른 3자리 숫자여야 합니다."); | ||
| } | ||
|
|
||
| @DisplayName("BallNumbers가 가지고 있는 숫자를 추측하여, 추측 결과를 확인한다. (Case. 3스트라이크)") |
There was a problem hiding this comment.
이런경우 ParameterziedTest를 사용해볼수 있지 않을까요?
| BallNumbers generated = randomBallNumbersGenerator.generate(); | ||
|
|
||
| // then | ||
| assertThat(generated).isNotNull(); |
There was a problem hiding this comment.
이게 과연 랜덤으로 생성된 값을 반환했다고 볼수 있을까요?
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.