Skip to content
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

Merged
merged 42 commits into from
Feb 28, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
42 commits
Select commit Hold shift + click to select a range
0a2d8f7
fix: 결과 출력 형식 수정
KJunseo Feb 21, 2021
2b19b28
refactor: LottoCount 인스턴스 변수 초기화를 하나의 생성자에서 처리
KJunseo Feb 21, 2021
cb3b024
refactor: 에러 메세지 상수처리
KJunseo Feb 21, 2021
fdb8bde
refactor: 메소드명 변경
KJunseo Feb 21, 2021
d72a003
refactor: WinnerTicket splitNumbers 스트림 단순화
KJunseo Feb 21, 2021
64b0854
refactor: 사용하지 않는 코드 삭제
KJunseo Feb 21, 2021
72a38cf
refactor: Ticket 생성자 하나로 만들기
KJunseo Feb 21, 2021
284b1e9
refactor: HashMap 대신 EnumMap 사용
KJunseo Feb 21, 2021
a6f900f
refactor: 메소드 명 변경
KJunseo Feb 21, 2021
d5deba3
style: 사용하지 않는 import 제거
KJunseo Feb 21, 2021
e94ac14
test: 구매 금액에 따른 로또 티켓 장수 확인 테스트 추가
KJunseo Feb 21, 2021
67c4d69
docs: 수동 구매 요구사항 추가
KJunseo Feb 22, 2021
4086029
feat & refactor: 수동 구매 로또 수 입력 숫자인지 검증 & 이를 위한 리팩토링
KJunseo Feb 22, 2021
e32098c
feat: 수동 로또 티켓 구매 검증 추가
KJunseo Feb 22, 2021
e3bfbd3
feat: 수동 티켓 생성 추가 및 검증
KJunseo Feb 22, 2021
9a7e31f
feat: 수동 구매 후 기존 LottoCount 업데이트
KJunseo Feb 22, 2021
0b79006
feat: 컨트롤러에 연결
KJunseo Feb 22, 2021
e63076a
refactor: TicketValidation private 생성자추가 및 인덴트 줄이기
KJunseo Feb 22, 2021
cf03a1f
refactor: stream 사용하여 인덴트 줄이기
KJunseo Feb 22, 2021
f5c8cf8
feat: 수동 생성 로직 정렬 기능 추가
KJunseo Feb 22, 2021
34ac0db
refactor: Ranking 가져오는 로직 stream 사용하여 단순화
KJunseo Feb 22, 2021
7ca5541
refactor: 쓰레드 세이프를 위해 static 제거
KJunseo Feb 22, 2021
9b7020d
refactor: 당첨금 총액을 계산하는 로직을 PrizeMoney에서 Statistics로 변경
KJunseo Feb 22, 2021
654cd8d
refactor: 로또 번호 캐싱해보기
KJunseo Feb 23, 2021
ff04462
refactor: Tickets 생성 로직 수정
KJunseo Feb 23, 2021
f34b0f6
style: 상수 정리
KJunseo Feb 23, 2021
a0af6dd
test: Number 테스트 추가
KJunseo Feb 23, 2021
e3b2c59
refactor: 컨트롤러 정리
KJunseo Feb 23, 2021
7726718
refactor: 검증 도메인 단에서 처리
KJunseo Feb 23, 2021
8127bbf
refactor: prizeMoney의 멤버변수로 int대신 Money사용
KJunseo Feb 23, 2021
ad560e2
refactor: 상태값에 의해 예외가 발생하는 부분 IllegalStateException 으로 변경
KJunseo Feb 23, 2021
fde84eb
refactor: private 생성자 추가
KJunseo Feb 25, 2021
a22b87c
style: 하드코딩 상수처리
KJunseo Feb 25, 2021
e844496
refactor: 로또 숫자 캐싱시 정적 팩토리 메소드 적용
KJunseo Feb 25, 2021
e48a6d2
refactor: 수익률 계산 기능 Statistics에서 처리
KJunseo Feb 25, 2021
c6b4f50
refactor: 로또 개수, 당첨금 총액, 수익률 구하는 로직 적절한 자료형으로 변경
KJunseo Feb 25, 2021
0cea17a
fix: 수익률이 1000원 이하일 경우 error 발생 버그 fix
KJunseo Feb 25, 2021
c800e8f
refactor: 비즈니스 로직 서비스단으로 분리
KJunseo Feb 25, 2021
8cefe2c
style: 상수 정리
KJunseo Feb 25, 2021
2f0f106
feat: 수동 로또 구입 개수가 구매할 수 있는 양보다 많은 경우 예외 잡기
KJunseo Feb 25, 2021
cb44841
refactor: LottoCount 주생성자 변경
KJunseo Feb 26, 2021
3df637c
refactor: 자동 로또 생성 전략 주입받아 사용하기
KJunseo Feb 27, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion src/main/java/lotto/Application.java
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();
}
}
10 changes: 8 additions & 2 deletions src/main/java/lotto/game/LottoController.java
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,12 @@
import lotto.view.OutputView;

public class LottoController {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Controller에 로직이 많아보이네요!
Controller는 View와 도메인의 연결만을 담당하도록 로직을 분리해보면 좋겠어요

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

질문에 대한 답은 여기 남겨둘게요!

Q1. 인스턴스 변수의 초기화를 한 곳에서 하는 것이 불가능한 경우면 설계가 잘못되었다고 볼 수 있을까요?
A1. 아닙니다 상황에 따라 초기화가 한 곳에서 불가능할 경우가 있습니다
대부분의 경우 정적 팩토리 메서드와 같은 방법으로 처리할 수 있을텐데 에어의 경우도 그런 경우인지는 확인이 필요하겠네요!
어떠한 경우인지 알 수 있을까요?

Q2. 이번에 캐싱을 적용해보면서 현재 Number 클래스에서는 1~45의 Number를 미리 생성할 때 생성자에서 validation을 거치고, 외부에서 Number 값을 가져오게 하려할때 valueOf(정적 팩토리 메소드) 내부에서도 동일한 validation을 거친 후에 꺼내주는데 뭔가 중복된 느낌이 드네요ㅠㅠ 잘못된 구조로 설계한 것 일까요?
A2. 생성자는 private이기 때문에 어차피 내부에서 밖에 접근할 수 없습ㄴ디다.
그렇다면 캐싱 시에도 정적 팩토리 메서드를 통해 생성하면 중복을 제거할 수 있지 않을까요!?

Q3. 정적 팩토리 메소드를 사용할 경우는 생성자는 꼭 private으로 둬야하나요? public으로 놔두고 둘 다 사용해도 되나요?
A3. 정적 팩토리 메서드를 사용할 경우 관례적으로 private 처리를 합니다!
어떠한 이유 때문일지 고민해보아요 :)

Q4. 수동 로또 생성 기능이 추가되기 전에는 Tickets의 내부에서 로또 생성 전략을 받아서 Ticket을 생성하는 식으로 구현했었는데, 수동 기능이 추가되면서 사용자 입력을 받아야되기 때문에 Tickets 도메인에서 입력을 받는 것이 좋지 않은 것 같아 기존의 자동 생성 로직도 일관성을 위해 컨트롤러로 뺐습니다. 이렇게 Tickets를 생성하는 로직을 컨트롤러에서 처리해도 괜찮을까요?
A4. 해당 메서드의 구현은 나쁘지 않지만 전체적으로 Controller에 로직이 많아보이네요!
Controller는 View와 도메인의 연결만을 담당하도록 로직을 분리해보면 좋겠어요

Q5. 또 자동 생성과, 수동 생성 메서드 구조가 매우 흡사하여 합쳐보려고 했는데 실패했습니다. 수행하는 로또 생성 전략만 다른 것 같아서 전략을 파라미터로 전달하는 식으로 구현해보려고 했는데 수동 생성시 Input값을 한 번 밖에 받지 못하더라구요.. 중복을 해결할 수 있을까요? 있다면 힌트 부탁드려요!
A5. 수동 생성 시 모든 번호를 받음 -> 로또들을 생성
하는 인터페이스로 분리한다면 두 구조를 합칠 수 있을 것 같습니다!

Q6. 생성자를 포함한 구조가 많이 바뀌다보니 테스트 코드도 수정이 있었는데 이렇게 구조를 바꾸고 테스트 코드를 바꿔주니 끼워맞춘듯한? 느낌이 들어서 올바르게 테스트를 하고 있는지 잘 모르겠어요..!
A6. 변경된 테스트 중 어색하다고 생각이 드는 부분은 못찾은 것 같은데 기존 테스트 코드가 바뀐 것 중 어색하다고 생각되는 부분이 어딘지 알 수 있을까요?

Q7. 현재는 아래와 같이 되어있는데 이런 값도 상수처리 하는 것이 의미가 있을까요?
A7. 해당 네이밍은 매직넘버와 동일한 정보를 제공하기 때문에 의미가 없을 것 같습니다!
0, 1 값에 집중하기 보다는 어떠한 의미를 가지는지를 고민해보면 좋을 것 같네요 :)

Q8. 검증시 반환 값이 존재해도 되는 걸까요? 현재 반환 값이 존재하는 validate이 여러개 있습니다. void validate으로 하고 싶지만 이렇게 한 이유는 검증 때 parseInt를 이미 했는데, 검증이 끝나고 또 parseInt를 하는 것이 비효율적이라고 생각해서입니다!
A8. 없는걸 추천하는 분들도 있지만 개인적으론 상관이 없다고 생각합니다 ㅎㅎ
스타일에 따라 달라지는 부분이라 뭐가 정답이라고는 말씀드리기 힘드네요 ㅜㅜ

Q9. 현재 다른 부분들은 컨트롤러에서 try-catch를 사용하여 잘못된 입력이 들어온 경우 다시 입력받도록 되어 있습니다. 하지만 수동 로또 개수로 살 수 있는 최대 개수보다 더 큰 값이 들어오는 경우를 try-catch로 감싸지 못했습니다.
이 부분도 힌트 주실 수 있을까요??
A9.

try {
	전체 로또 개수에서 수동 로또 개수를 빼줌으로 자동 로또 개수 구하여 LottoCount 객체 생성
} catch (Exception) {
	수동 로또 개수를 입력받아서 LottoCount 객체 생성
}

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);
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down
13 changes: 9 additions & 4 deletions src/main/java/lotto/game/LottoService.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,20 @@
import lotto.ticket.Ticket;
import lotto.ticket.Tickets;
import lotto.ticket.strategy.ManualNumbersGenerator;
import lotto.ticket.strategy.RandomNumbersGenerator;
import lotto.ticket.strategy.NumbersGenerator;

import java.util.ArrayList;
import java.util.List;
import java.util.stream.Collectors;

public class LottoService {
private final NumbersGenerator autoLottoGenerator;

public static Tickets buyManualTickets(List<String> manualLottoNumber) {
public LottoService(NumbersGenerator generator) {
this.autoLottoGenerator = generator;
}

public Tickets buyManualTickets(List<String> manualLottoNumber) {
List<Ticket> manualTickets = manualLottoNumber.stream()
.map(ManualNumbersGenerator::new)
.map(ManualNumbersGenerator::generate)
Expand All @@ -20,11 +25,11 @@ public static Tickets buyManualTickets(List<String> manualLottoNumber) {
return new Tickets(manualTickets);
}

public static Tickets buyAutoTickets(LottoCount count) {
public Tickets buyAutoTickets(LottoCount count) {
List<Ticket> tickets = new ArrayList<>();
while (count.isGreaterThanZero()) {
count = count.decreaseOne();
tickets.add(new Ticket(new RandomNumbersGenerator().generate()));
tickets.add(new Ticket(autoLottoGenerator.generate()));
}
return new Tickets(tickets);
}
Expand Down
41 changes: 41 additions & 0 deletions src/test/java/lotto/game/LottoServiceTest.java
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());

Choose a reason for hiding this comment

The 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);
}
}