코드 리뷰 PR #2103
코드 리뷰 PR #2103
Conversation
src/main/java/lotto/Application.java
Outdated
| InputView inputView = createInputView(); | ||
| OutputView outputView = new OutputView(); | ||
|
|
||
| BuyLottoService buyLottoService = createBuyLottoService(outputView); |
There was a problem hiding this comment.
현재 작성하신 controller와 service를 모두 application클래스에서 생성하도록 구현하신것같은데
우아한테크코스 3주차 공통 피드백을 보시면,
함수(메서드) 라인에 대한 기준 프로그래밍 요구사항을 보면 함수 15라인으로 제한하는 요구사항이 있다. 이 기준은 main() 함수에도 해당된다. 공백 라인도 한 라인에 해당한다. 15라인이 넘어간다면 함수 분리를 위한 고민을 한다.
라고 제시되어있어요. 만약 이후 controller와 service, repository까지 많아지게 되는 프로그램이라면 위 요구사항을 어기게 될것같은데 어떻게 생각하실까요??
There was a problem hiding this comment.
음.. 맞아요 저도 이 부분이 많이 걸렸어요. 하지만 모듈화 하면 할 수록 Application에서 생성해줘야 하는게 많아져서 고민이 됐어요...
다음에는 controller만 의존하게끔 수정해봐야겠어요!!
| } | ||
|
|
||
| public int calculateLottoCount() { | ||
| return price / THOUSAND; |
There was a problem hiding this comment.
calculateLottoCount를 호출할때마다 구입가격에서 1000을 나눌텐데 이렇게 하신 이유가 궁금합니다!!
There was a problem hiding this comment.
로또의 개수를 구하는 함수(calculateLottoCount)로 만들었습니다!
듣고보니 만약 로또 가격이 바뀌게 된다면 나누는 가격을 수정해야 하니 THOSAND 대신 LOTTOPRICE로 바뀌는게 좋겠군요!
| .anyMatch(number -> number.equals(bonusNumber.isSameBonusNumber(bonusNumber))); | ||
| } | ||
|
|
||
| private long countMatchingWinnerNumber(Lotto lotto, WinnerNumber winnerNumber) { |
There was a problem hiding this comment.
int로 선언하셔도 됐을텐데 long으로 선언하신 이유가 궁금합니다!
There was a problem hiding this comment.
countMatchingWinnerNumber() 함수에서 stream을 쓰다보니 return 값이 long이 되었어요!
long이든 int든 크게 영향을 주지 않을 것이라고 생각하여 long으로 선언했습니다!
| return purchasePrice; | ||
| } | ||
|
|
||
| public Statistics calculateMatching() { |
There was a problem hiding this comment.
현재 calculateMatching메소드는 statics도메인에 접근하기 위한 메소드로 보여집니다! service계층이 있는 이유는, 프레젠테이션 계층과 데이터 엑세스 계층 사이를 연결하기 위함이죠!! 따라서 이 부분은 service쪽에서 구현되어야한다고 생각해요!
There was a problem hiding this comment.
맞아요! 이 로직은 service에 배치하는게 더 적절할 것 같군요! 👍
| private final Map<Rank, Integer> result; | ||
|
|
||
| private Statistics() { | ||
| result = new EnumMap<>(Rank.class); |
There was a problem hiding this comment.
재현님 덕분에 EnumMap에 대해서 처음으로 배웠네요!! ㅎㅎEnumMap이 해시충돌을 안일으킨다는데 덕분에 배워갑니다!!
No description provided.