-
Notifications
You must be signed in to change notification settings - Fork 1.1k
2주차 - Step4 로또(수동) 리뷰 요청드립니다. #262
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
4단계까지 고생하셨습니다. 👍
간단한 피드백 남겼습니다. 고민해 보시고 반영 부탁드립니다~!!
final int numberOfManualTickets = InputView.askNumberOfManualTicket(); | ||
final int numberOfAutoTickets = LottoSeller.countAutoTickets(inputMoney, numberOfManualTickets); | ||
|
||
List<List<Integer>> manualTickets = InputView.askNumbersForManualTickets(numberOfManualTickets); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
수동 로또를 별도의 List<List>자료구조로 만드셨네요. 좀 아쉽네요. 가독성이 떨어지는 형태입니다.
기존에 LottoNumber, LottoTicket을 활용하여 동일하게 사용되도록 해야합니다. InputView를 통해 받은 값들을 로또 도메인과 결합되지 않기 위해 이렇게 하신듯 하네요. 중간에 매개체를 두어 InputView에서 받은 값을 LottoNumber로 변환하고 모두 변환이 완료되면 manualLottoTicket을 반환하는 형태가 맞을 것 같습니다.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
다른 접근 방식으로는 LottoTicketGenerator를 인터페이스화 시키고 자동/수동에 대한 구현체를 만드는 것입니다.
그리고 LottoTickets을 생성할때 이를 파라미터로 주입받아 동적으로 생성하는 방식인데요. 현재 구조에서는 수정할 부분이 많을 듯 합니다. 고민해 보시고 합리적인 방식으로 수정 부탁드립니다!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
중간에 매개체를 두어 InputView에서 받은 값을 LottoNumber로 변환하고 모두 변환이 완료되면 manualLottoTicket을 반환하는 형태가 맞을 것 같습니다.
라고 피드백을 주셨는데요,
Application 클래스 중간에 LottoSeller가 inputView에서 받은 수동로또 값을 LottoTicketGenerator에 위임해서 LottoNumber로 변환하여 기존 로또와 똑같이 LottoTicket으로 생성해서 반환하고 있습니다.
LottoTickets lottoTickets = LottoSeller.issueLottoTicket(manualTickets, numberOfAutoTickets)
말씀주신 중간 매개체가 LottoSeller와 LottoGenerator인 것 같은데 제가 혹시 잘못 이해하고 있으면 말씀주시면 감사하겠습니다ㅜㅠ
그리고 조언주신 것처럼 List<List>는 쓰면서도 찝찝했습니다..!! InputView에서 사용자가 입력하는 String 값을 받자마자 Int로 변환해서 저장하려다가 쓰게 된 것인데, InputView에서는 List<String;>으로 받은 다음에 LottoSeller에서 파싱하는 것으로 수정해봤습니다.
|
||
import java.util.List; | ||
|
||
public class LottoApplication { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
전체코드가 다 변경되었네요ㅠㅠ
들여쓰기가 다시 된 것일까요?? 코드 전체를 다시 다 봐야되서...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ㅠㅠ 패키지를 스텝별로 새로 만들면서 해서 그런 것 같습니다..
단계별 코드를 한 곳에 저장해두려고 그렇게 만들었는데 리뷰하시기에 불편하셨을 것 같네요..
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
로또 미션 진행 하시느라 고생 많으셨습니다. 👍
Merge할게요!
안녕하세요!
늦었지만 로또 4단계 리뷰 요청드립니다!
지난 리뷰에서 피드백 주신 부분을 반영했으며,
어제 수업시간에 배운 내용을 적용해봤습니닷.
바쁘시겠지만 많은 지적과 조언 부탁드립니다 :)