-
Notifications
You must be signed in to change notification settings - Fork 248
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
[2단계 - 로또 구현] 에어(김준서) 미션 제출합니다. #297
Changes from 1 commit
0a2d8f7
2b19b28
cb3b024
fdb8bde
d72a003
64b0854
72a38cf
284b1e9
a6f900f
d5deba3
e94ac14
67c4d69
4086029
e32098c
e3bfbd3
9a7e31f
0b79006
e63076a
cf03a1f
f5c8cf8
34ac0db
7ca5541
9b7020d
654cd8d
ff04462
f34b0f6
a0af6dd
e3b2c59
7726718
8127bbf
ad560e2
fde84eb
a22b87c
e844496
e48a6d2
c6b4f50
0cea17a
c800e8f
8cefe2c
2f0f106
cb44841
3df637c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,10 +1,15 @@ | ||
package lotto; | ||
|
||
import lotto.game.LottoController; | ||
import lotto.game.LottoService; | ||
import lotto.ticket.strategy.NumbersGenerator; | ||
import lotto.ticket.strategy.RandomNumbersGenerator; | ||
|
||
public class Application { | ||
public static void main(String[] args) { | ||
LottoController lottoController = new LottoController(); | ||
NumbersGenerator autoTicketGenerator = new RandomNumbersGenerator(); | ||
LottoService lottoService = new LottoService(autoTicketGenerator); | ||
LottoController lottoController = new LottoController(lottoService); | ||
lottoController.run(); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,12 @@ | |
import lotto.view.OutputView; | ||
|
||
public class LottoController { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 질문에 대한 답은 여기 남겨둘게요! Q1. 인스턴스 변수의 초기화를 한 곳에서 하는 것이 불가능한 경우면 설계가 잘못되었다고 볼 수 있을까요? Q2. 이번에 캐싱을 적용해보면서 현재 Number 클래스에서는 1~45의 Number를 미리 생성할 때 생성자에서 validation을 거치고, 외부에서 Number 값을 가져오게 하려할때 valueOf(정적 팩토리 메소드) 내부에서도 동일한 validation을 거친 후에 꺼내주는데 뭔가 중복된 느낌이 드네요ㅠㅠ 잘못된 구조로 설계한 것 일까요? Q3. 정적 팩토리 메소드를 사용할 경우는 생성자는 꼭 private으로 둬야하나요? public으로 놔두고 둘 다 사용해도 되나요? Q4. 수동 로또 생성 기능이 추가되기 전에는 Tickets의 내부에서 로또 생성 전략을 받아서 Ticket을 생성하는 식으로 구현했었는데, 수동 기능이 추가되면서 사용자 입력을 받아야되기 때문에 Tickets 도메인에서 입력을 받는 것이 좋지 않은 것 같아 기존의 자동 생성 로직도 일관성을 위해 컨트롤러로 뺐습니다. 이렇게 Tickets를 생성하는 로직을 컨트롤러에서 처리해도 괜찮을까요? Q5. 또 자동 생성과, 수동 생성 메서드 구조가 매우 흡사하여 합쳐보려고 했는데 실패했습니다. 수행하는 로또 생성 전략만 다른 것 같아서 전략을 파라미터로 전달하는 식으로 구현해보려고 했는데 수동 생성시 Input값을 한 번 밖에 받지 못하더라구요.. 중복을 해결할 수 있을까요? 있다면 힌트 부탁드려요! Q6. 생성자를 포함한 구조가 많이 바뀌다보니 테스트 코드도 수정이 있었는데 이렇게 구조를 바꾸고 테스트 코드를 바꿔주니 끼워맞춘듯한? 느낌이 들어서 올바르게 테스트를 하고 있는지 잘 모르겠어요..! Q7. 현재는 아래와 같이 되어있는데 이런 값도 상수처리 하는 것이 의미가 있을까요? Q8. 검증시 반환 값이 존재해도 되는 걸까요? 현재 반환 값이 존재하는 validate이 여러개 있습니다. void validate으로 하고 싶지만 이렇게 한 이유는 검증 때 parseInt를 이미 했는데, 검증이 끝나고 또 parseInt를 하는 것이 비효율적이라고 생각해서입니다! Q9. 현재 다른 부분들은 컨트롤러에서 try-catch를 사용하여 잘못된 입력이 들어온 경우 다시 입력받도록 되어 있습니다. 하지만 수동 로또 개수로 살 수 있는 최대 개수보다 더 큰 값이 들어오는 경우를 try-catch로 감싸지 못했습니다.
4번 단계에서 예외처리 발생 시 3번 단계를 진행하도록 처리하면 될 것 같습니다 코드로 봤을 땐 단순하게 3, 4번 작업을 묶어서 메서드로 분리하는 방법이 가장 간단할 것 같네요! |
||
private final LottoService lottoService; | ||
|
||
public LottoController(LottoService lottoService) { | ||
this.lottoService = lottoService; | ||
} | ||
|
||
public void run() { | ||
Money money = generateMoney(); | ||
LottoCount lottoCount = possibleLottoCount(money); | ||
|
@@ -53,7 +59,7 @@ private LottoCount manualTicketAmount() { | |
|
||
private Tickets ticketPurchase(LottoCount manualTicketAmount, LottoCount autoTicketAmount) { | ||
Tickets manualTickets = manualTicketGenerate(manualTicketAmount); | ||
Tickets autoTickets = LottoService.buyAutoTickets(autoTicketAmount); | ||
Tickets autoTickets = lottoService.buyAutoTickets(autoTicketAmount); | ||
OutputView.noticeLottoCount(manualTicketAmount, autoTicketAmount); | ||
|
||
Tickets totalTicket = Tickets.joinTicket(manualTickets, autoTickets); | ||
|
@@ -64,7 +70,7 @@ private Tickets ticketPurchase(LottoCount manualTicketAmount, LottoCount autoTic | |
private Tickets manualTicketGenerate(LottoCount count) { | ||
try { | ||
OutputView.enterManualTicketNumber(); | ||
return LottoService.buyManualTickets(InputView.inputNumbers(count)); | ||
return lottoService.buyManualTickets(InputView.inputNumbers(count)); | ||
} catch (RuntimeException e) { | ||
OutputView.printError(e); | ||
return manualTicketGenerate(count); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,41 @@ | ||
package lotto.game; | ||
|
||
import lotto.ticket.Tickets; | ||
import lotto.ticket.strategy.RandomNumbersGenerator; | ||
import org.junit.jupiter.api.BeforeEach; | ||
import org.junit.jupiter.api.DisplayName; | ||
import org.junit.jupiter.api.Test; | ||
|
||
import java.util.Arrays; | ||
import java.util.List; | ||
|
||
import static org.assertj.core.api.AssertionsForClassTypes.assertThat; | ||
|
||
public class LottoServiceTest { | ||
LottoService lottoService; | ||
|
||
@BeforeEach | ||
void setUp() { | ||
lottoService = new LottoService(new RandomNumbersGenerator()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 인터페이스로 분리하여 모킹을 할 수 있게 됐고, 의도한 값이 나올 수 있게 변경됐네요 👍 |
||
} | ||
|
||
@Test | ||
@DisplayName("수동 로또 구매") | ||
void buyManualTicket() { | ||
List<String> ticketNumbers = Arrays.asList( | ||
"1,2,3,4,5,6", | ||
"1,3,5,7,8,10", | ||
"4,8,10,15,17,45" | ||
); | ||
Tickets tickets = lottoService.buyManualTickets(ticketNumbers); | ||
assertThat(tickets.getTickets().size()).isEqualTo(3); | ||
} | ||
|
||
@Test | ||
@DisplayName("자동 로또 구매") | ||
void buyAutoTicket() { | ||
LottoCount lottoCount = new LottoCount("3"); | ||
Tickets tickets = lottoService.buyAutoTickets(lottoCount); | ||
assertThat(tickets.getTickets().size()).isEqualTo(3); | ||
} | ||
} |
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.
Controller에 로직이 많아보이네요!
Controller는 View와 도메인의 연결만을 담당하도록 로직을 분리해보면 좋겠어요