Skip to content

Songhansol#3

Open
SongHanSol1140 wants to merge 4 commits intoCODE-CLEANERS:mainfrom
SongHanSol1140:songhansol
Open

Songhansol#3
SongHanSol1140 wants to merge 4 commits intoCODE-CLEANERS:mainfrom
SongHanSol1140:songhansol

Conversation

@SongHanSol1140
Copy link

숫자 야구게임 풀 리퀘스트

Copy link
Collaborator

@ca1af ca1af left a comment

Choose a reason for hiding this comment

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

잘 했는데...테스트 코드도 작성 해 보자.

throw new IllegalArgumentException("숫자는 1-9 사이여야 합니다.");
}
}
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

숫자가 3이여야 한다는 것은 정책 아닌가? 이 숫자가 여러 번 등장하는데, 정책이 바뀔 때 마다 여러 번 등장하는 해당 숫자가 변경되어야 한다.

상수로 처리하거나, 로직을 변경하는 게 좋을 듯 하다

Comment on lines +40 to +46
public static BaseBallNumbers parseIntegerList(String input) {
List<Integer> numbers = new ArrayList<>();
for (char c : input.toCharArray()) {
numbers.add(Character.getNumericValue(c));
}
return new BaseBallNumbers(numbers);
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

  • 스태틱 메서드는 클래스 상단에 위치해야 한다. (생성자 위, 프로퍼티 아래)
  • 유저의 인펏으로만 생성되는 인스턴스라면, 생성자는 private이어도 무관 할 것이다.
  • 반드시 유저의 인펏으로만 생성된다면, 생성자 패러미터 자체를 Input 으로 변경해도 무방 할 것이다.

inputHandler.close();
} catch (IllegalArgumentException error) {
System.out.println(error.getMessage());
System.exit(0);
Copy link
Collaborator

Choose a reason for hiding this comment

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

이건 왜?

Comment on lines +19 to +22
// 자릿수 체크
if (input.length() != 3) {
throw new IllegalArgumentException("3자리 숫자를 입력하세요.");
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

여기도 3이라는 매직넘버가 등장한다.

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