[Step5] 로또(수동) #1698
[Step5] 로또(수동) #1698
Conversation
src/main/java/lottery/LottoNo.java
Outdated
| import java.util.Comparator; | ||
| import java.util.Objects; | ||
|
|
||
| public class LottoNo implements Comparable<LottoNo> { |
There was a problem hiding this comment.
중요한 내용은 아니지만 LottoNumbers와 항상 같이 쓰일 것 같은데 이름을 맞춰주면 어떨까요?
LottoNo, LottoNos
LottoNumber LottoNumbers
There was a problem hiding this comment.
이름이 일관성이 떨어지네요 ㅎㅎ 코드 수정하겠습니다. 피드백 감사합니다.
src/main/java/lottery/Tickets.java
Outdated
| return value.stream().filter(a -> a.getType().equals(TicketType.MANUAL)).mapToInt(a -> 1).sum(); | ||
| } | ||
|
|
||
| public int sizeOfAutoTickets() { | ||
| return value.stream().filter(a -> a.getType().equals(TicketType.AUTO)).mapToInt(a -> 1).sum(); |
There was a problem hiding this comment.
요건 팀마다, 개인마다 컨벤션이 다르겠지만
이렇게 긴 메소드 체이닝의 경우 적절하게 엔터를 통해 분리해주시는게 가독성 향상에 도움이 될 수 있겠네요~
There was a problem hiding this comment.
맞습니다. 어떤 stream은 엔터로 분리했는데, 이부분은 하지 않았네요. : ) 네, 수정하겠습니다!
| - [X] 사용자 입력값을 예외처리한다. | ||
| - [X] 수동 입력시 출력 내용 변경 | ||
|
|
||
| - [X] int 값 로또 번호를 LottoNo 으로 변경한다. |
There was a problem hiding this comment.
머지 전에 전체적인 코드를 한번 훑어봤는데 아래 부분에 궁금한 점이 있어 코멘트 남깁니다.
파일 체인지론 잡히지 않아서 링크를 걸었어요.
위 부분에서 아래와 같이 검증해주는 부분이 있는데
private void setMatchTypeMap(Map<LotteryMatchType, Integer> matchTypeMap) {
if (Objects.isNull(matchTypeMap) || matchTypeMap.isEmpty()){
throw new IllegalArgumentException(NOT_FOUND_LOTTERY_MATCH_TYPE);
}
this.matchTypeMap = matchTypeMap;
}
Map에 데이터가 하나도 없는 경우도 아래와 같이 출력되어야 하지 않을까요?
위와 같이 처리하면 예외가 발생할 것 같아서요.
3개 일치 (5000원)- 0개
4개 일치 (50000원)- 0개
5개 일치 (1500000원)- 0개
5개 일치, 보너스 볼 일치 (30000000원)- 0개
6개 일치 (2000000000원)- 0개
There was a problem hiding this comment.
맞네요 👍 테스트 케이스가 빈약하네요. 테스트 코드 추가 후, 코드 변경하겠습니다ㅎ
|
간단한 부분이라 확인 후 바로 요청 주시면 머지하도록 할게요~ 고생 많으셨습니다. |
|
제가 작성한 코드에 대해서 되돌아볼 수 있었던 좋은 시간이였습니다. |
|
리뷰 감사합니다 : ) |
안녕하세요. 리뷰어님, 5단계 로또(수동) PR입니다.
수동기능이 들어가면서 Ticket 이 가져야할 책임으로서 Type을 하나 추가했습니다.
더불어, 로또번호가 int 였던 부분을 LottoNo 으로 변경했습니다.
테스트 코드를 많이 짜보지 않았던 탓인지, fixture 작업을 하는데, 생각보다 많은 시간이 소모되었네요;