미션4_로또 게임 수동 #12
미션4_로또 게임 수동 #12
Conversation
|
리뷰 부탁 드립니다 🙏🏼 |
- 수동, 자동 로또 티켓으로 나누지 않고 하나로 관리
- LottoGenerator 클래스에 자동/수동 생성 이동 - LottoValidator 클래스 이름 변경
- 가독성을 위해 enum 사용
|
리뷰 부탁 드립니다 🙏🏼 🙏🏼 |
| private final LottoCalculator calculator; | ||
|
|
||
| /** 지난 주 당첨 번호 유효성 검사기 */ | ||
| private final LastWeekLottoValidator validator; |
There was a problem hiding this comment.
흠.. Validator는 개인적으로 생각했을때 해당 어플리케이션의 비지니스 로직으로써 서비스 쪽에 의존하는게
좋다고 생각하는데 의존을 제너레이터에서 변경하신 이유가 있을까요?
There was a problem hiding this comment.
로또 게임에서 사용자에게 입력받아 만드는 LottoTicket의 종류는 두 가지가 있습니다.
- 수동 로또 번호 입력 -> 로또 티켓으로 반환
- 지난주 당첨 번호 입력 -> 로또 티켓으로 반환
LottoService 에 Validator 를 두면, 중복 코드가 발생한다고 생각하여 generator 로 옮겼어요
수동 로또 입력, 지난주 당첨 번호 입력 모두 LottoTicketGenerator 의 manualGenerate 를 사용하여 생성되도록 되어있어요.
그래서 LottoTicketGenerator 로 옮기게 되었어요
There was a problem hiding this comment.
그러기엔 너무 비지니스 규칙에 의존하고 있는 명칭을 쓰고 있는듯 싶은데요?!
중복 번호를 벨리데이션한다 라는 명칭의 메서드가 어울리는 듯 싶습니다
- 수동 로또 티켓이 생성될 때 카운팅하여 변수에 저장
- 로또 티켓을 생성하기 때문에 이름을 적절히 변경
- LottoService 에 정의된 LottoTicketGenerator 를 사용하도록 변경 - Loose Coupling
- LottoValidator -> LottoTicket (MAX_LOTTO_NUMBER, MIN_LOTTO_NUMBER)
- 자동, 수동 로또 구매애 대해 전략 패턴 적용
|
전략 패턴과 지난 번 리뷰에 대한 제 답변 위주로 리뷰 부탁 드립니다 ! 🙏🏼 |
| import lotto.domain.LottoTicketGenerator; | ||
| import lotto.dto.LottoPurchaseDto; | ||
|
|
||
| public interface PurchaseStrategy { |
There was a problem hiding this comment.
오호... 구매행위에 대한 전략을 취하셨군요!
사실 의도한건 로또번호에 대한 전략을 말한거긴 한데 좋습니다
| import lotto.domain.LottoTicketGenerator; | ||
| import lotto.dto.LottoPurchaseDto; | ||
|
|
||
| public class ManualStrategy implements PurchaseStrategy { |
There was a problem hiding this comment.
어쩌피 lottoTicketGenerator는 항상 써야 하니까 필수 파라미터로 두고 메서드에서는 제거 할거 같네요.
public class ManualStrategy implements PurchaseStrategy {
private final LottoTicketGenerator lottoTicketGenerator;
public ManualStrategy(LottoTicketGenerator lottoTicketGenerator) {
this.lottoTicketGenerator = lottoTicketGenerator;
}
@Override
public void buyLottoTicket(LottoConsumer consumer, LottoPurchaseDto purchaseDto) {
List<LottoTicket> lottoTickets = new ArrayList<>();
for (String[] lottoNumber : purchaseDto.getLottoNumbers()) {
lottoTickets.add(lottoTicketGenerator.manualGenerate(lottoNumber));
}
// 로또 티켓 추가
consumer.addManualLottoTickets(lottoTickets);
}
}| public LottoPurchaseDto(int money, List<String[]> lottoNumbers) { | ||
| this.money = money; | ||
| this.lottoNumbers = new ArrayList<>(); | ||
| if (lottoNumbers != null) { |
| */ | ||
| public void buyLotto(LottoConsumer consumer, int money) { | ||
| consumer.buyLotto(money, generator); | ||
| public void setPurchaseStrategy(PurchaseStrategy strategy) { |
There was a problem hiding this comment.
조금 위험한 코드네요
LottoSerivce를 사용자마다 만들지 않으면 해당 부분은 쓰레드 세이프 하지 않은 코드가 될꺼 같아요
차라리 사용하는 메서드에서 아래와 같이 하는 편이 좋을꺼 같아요
public void buyLottoTicket(/*PurchaseStrategy strategy*/ LottoConsumer consumer, LottoPurchaseDto dto) {
|
추가적으로 전략패턴에 대한 테스트 코드는 존재하지 않아요 추가 부탁드려요 |
2 participants
변경 이유
미션 4 - 로또 게임 수동 개발
변경의 종류
변경 내용
기대 효과
주요사항
코드 품질
#테스트 과정
LottoController.java의 main 메서드를 실행합니다.