[2단계 - 로또 구현] 케빈(박진홍) 미션 제출합니다. #282

xlffm3 · 2021-02-21T07:41:50Z

안녕하세요!

지난 번 Step1에서의 피드백은 다음과 같습니다.

LottoTicket 일급 컬렉션이 내부 컬렉션 반환 시 Set을 List로 변환해 반환하던 것을 수정. 5ff494d
get보다 더 나은 메서드 네이밍 -> calculate 등의 동사를 활용했습니다. 662b547

이번 Step2의 요구사항 중에서 사용자가 입력한 값에 대한 예외 처리를 철저히 하라는 내용이 있었는데요!

구매 금액, 보너스 볼 입력값이 유효하지 않은 경우 (음수이거나, 1000원 미만이거나, 1~45 범위가 아니거나) 도메인 단에서 예외가 발생합니다. View에서 해당 입력 String을 Integer로 변환하는 부분(Integer.parseInt(scanner.nextLine());)에 대해서도 별도의 예외 처리가 필요할까요?

try {
 return Integer.parseInt(scanner.nextLine());
} catch (NumberFormatException e) {
  throw new IllegalArgumentException("입력 문자열은 정수여야합니다.");
}

와 같이 예외 번역을 해줄 수 있을 것 같은데, NumberFormatException 예외 클래스명만으로도 충분히 무슨 예외 상황인지 잘 표현되는 것 같다는 생각도 들어서요!

비슷하게 수동으로 구매할 티켓의 개수를 입력받을 때의 코드는 다음과 같은데요.

    public static List<ManualTicketNumbers> inputManualTicketNumbers() {
        int manualTicketCounts = nextIntWithInstructionMessage(INPUT_MANUAL_TICKET_COUNTS_MESSAGE);
        if (manualTicketCounts > ZERO) {
            System.out.println(INPUT_MANUAL_TICKET_NUMBERS_MESSAGE);
        }
        return Stream.generate(InputView::nextLineWithSplit)
                .map(ManualTicketNumbers::from)
                .limit(manualTicketCounts)
                .collect(Collectors.toList());
    }

수동 구매 티켓(manualTicketCounts)이 음수인 경우 limit() 메서드 자체에서 IllegalArgumentException을 호출하는데, 그 이전에 별도의 유효성 체크 검증을 거쳐야 할지 궁금합니다.

    public static List<ManualTicketNumbers> inputManualTicketNumbers() {
        int manualTicketCounts = nextIntWithInstructionMessage(INPUT_MANUAL_TICKET_COUNTS_MESSAGE);
        if (manualTicketCounts < 0) throw new IllegalArgumentException("수동 구매 티켓의 개수는 0 이상의 정수여야합니다.");
        .... //이후 로직
    }

적절한 메서드 명을 작성하는게 아직도 많이 어렵네요 ! ㅠㅠ
지난번 Step1과의 차이라고 한다면...

로또 티켓 1장의 가격을 LottoTicket 객체의 영역으로 두기보다는, 로또 티켓을 판매하는 외부 LottoMachine(혹은 LottoStore 등?)이 가지고 있는것이 자연스럽다고 생각했습니다. 따라서

구입 금액(PurchasingPrice)과 수동으로 구매할 로또 번호(List<ManaulTicketNumbers>)를 입력받으면,
LottoMachine에게 건내주고 LottoMachine이 내부적으로 로또 티켓의 가격을 활용하여

현재 금액으로 요청받은 티켓을 구매할 수 있는지
수동 티켓을 구매하고 남은 돈으로 몇 장의 자동 티켓을 구매할 수 있는지

등을 계산해서 로또 티켓을 생성해 반환하도록 했습니다!

- 로또 티켓의 가격은 외부의 로또 머신이 정한다. - LottoTickets가 생성하는 티켓의 개수를 내부에서 계산하지 않고 외부에서 주입받도록 한다.

- 금액을 검사한 다음 그에 맞는 수동 및 자동 티켓을 생성해 반환한다

- 수동과 자동으로 각각 몇 장 출력한다.

티켓들은 복수개도 가능하기 때문에 복수형으로 통일한다

로또 티켓은 자신의 가격을 모르도록 하고, 티켓의 가격을 결정하는 외부 기계가 구매 금액을 계산하도록 변경한다.

xlffm3 · 2021-02-21T08:30:34Z

학습 로그

[TDD] 점진적인 리팩토링 - 4

내용

요구사항 변화로 인해 기존의 메서드가 파라미터 혹은 리턴 타입이 변경되는 경우 최대한 기존 테스트들이 깨지지 않도록 점진적으로 리팩토링한다.
변경된 메서드를 추가한 다음, 변경 이전의 메서드를 사용하는 코드들을 IDE를 통해 탐색하여 변경된 메서드를 사용하도록 코드를 수정한다.
- 이후 변겨 이전의 메서드들을 사용하는 코드가 사라지면 변경 이전의 코드를 삭제한 다음, 변경된 메서드의 이름을 변경 이전의 메서드로 변경한다.
새로운 기능을 생성하는 것 보다 기존의 코드를 변경하면서 TDD를 적용하는 것이 무척 어렵다고 느꼈다.

[JDK] Comparator - 2

내용

Stream API에서 sorted()를 적용할 때, Comparator에 람다식을 적용하기 보다 인터페이스의 디폴트 메서드를 적용.
- comparing, comparingInt, thenComparing 등.

[JDK] Stream의 concat - 1

내용

두 리스트를 하나로 병합할 때 list의 addAll 외에도 Stream들을 concat하는 기능을 확인하고 사용해보았다.

hyunssooo

안녕하세요 케빈!
빠르게 step2까지 진행해주셨네요!👍👏
몇 가지 코멘트 남겼어요! 참고 하시고 고민하시면 좋겠습니다!
궁금한 점이나 고민점이 있으시다면 언제든 편하게 dm 주세요!
감사합니다!

hyunssooo · 2021-02-22T12:07:01Z

src/main/java/lotto/controller/LottoController.java

+    public void run() {
+        LottoMachine lottoMachine = new LottoMachine(new RandomLottoNumberGenerator());
+        LottoTickets lottoTickets = buyLottoTickets(lottoMachine);
+        WinningLottoTicket winningLottoTicket = generateWinningLottoTicket();
+        LottoResult lottoResult = lottoTickets.checkResult(winningLottoTicket);
+        int purchasingPriceTotal = lottoMachine.calculatePurchasingPrice(lottoTickets);
+        double yield = lottoResult.calculateYield(purchasingPriceTotal);
+        OutputView.printLottoResult(lottoResult, yield);
+    }
+
+    private LottoTickets buyLottoTickets(LottoMachine lottoMachine) {
+        PurchasingPrice purchasingPrice = new PurchasingPrice(InputView.inputPurchasingPrice());
+        List<ManualTicketNumbers> manualTicketsNumbers = InputView.inputManualTicketsNumbers();
+        LottoTickets lottoTickets = lottoMachine.issueLottoTickets(purchasingPrice, manualTicketsNumbers);
+        OutputView.printPurchasedLottoTicketCounts(manualTicketsNumbers.size(), lottoTickets.getTicketCounts());
+        OutputView.printAllLottoTicketNumbers(lottoTickets);
+        return lottoTickets;
+    }


개행으로 가독성을 높일 수는 없을까요??🤔

확실히 컨트롤러의 코드가 보기 힘든 구조네요

hyunssooo · 2021-02-22T12:28:36Z

src/main/java/lotto/view/InputView.java

+    public static List<ManualTicketNumbers> inputManualTicketsNumbers() {
+        int manualTicketCounts = nextIntWithInstructionMessage(INPUT_MANUAL_TICKET_COUNTS_MESSAGE);
+        if (manualTicketCounts > ZERO) {
+            System.out.println(INPUT_MANUAL_TICKET_NUMBERS_MESSAGE);
+        }
+        return Stream.generate(InputView::nextLineWithSplit)
+                .map(ManualTicketNumbers::from)
+                .limit(manualTicketCounts)
+                .collect(Collectors.toList());
+    }


ManualTicketNumbers는 도메인 객체로 보이네요! View에서 바로 객체 생성을 하는 것이 맞을까요? 고민해보면 좋을 거 같아요!

DTO 클래스라서 다른 방식을 고려해보겠습니다. :D

hyunssooo · 2021-02-22T13:04:47Z

src/test/java/lotto/domain/LottoMachineTest.java

+
+    private final LottoMachine lottoMachine = new LottoMachine(() -> Arrays.asList(1, 2, 3, 4, 5, 6));


인터페이스를 이용해서 테스트를 잘 작성해주셨어요!
하지만 인터페이스는 테스트만 잘 작성하기 위해서 사용하는 것은 아닙니다!
프로덕션 코드에서 인터페이스에 의존하고 여러 구현체들을 활용할 수 있어요!

hyunssooo · 2021-02-22T13:27:36Z

src/main/java/lotto/domain/ManualTicketNumbers.java

+public class ManualTicketNumbers {
+
+    private final List<Integer> manualTicketNumbers;
+
+    private ManualTicketNumbers(List<Integer> manualTicketNumbers) {
+        this.manualTicketNumbers = manualTicketNumbers;
+    }


이 클래스의 역할은 무엇일까요??

사실 Domain이라기 보다는... DTO 성격이 큰 클래스입니다.
사용자가 2장의 수동 로또 번호를 입력하면

1, 2, 3, 4, 5, 6
11, 12, 13, 14, 15, 16

도메인에서 이용할 수 있도록 각각을 split해서 List<Integer>로 만든 다음, 2개의 List<Integer>를 List로 collect를 해주어야 하는데요.
최종적으로 List<List<Integer>> 형태가 되는데 너무 복잡해보여서 List<Integer>를 ManualTicketNumbers로 만든 다음 List<ManualTicketNumbers>를 제공하도록 했습니다.

이 부분에 대해서는 더 나은 방법을 고민해봐야겠습니다... 😥

dto라면 패키지를 분리하는 것도 좋을 거 같아요 :)

hyunssooo · 2021-02-22T13:27:45Z

src/main/java/lotto/domain/LottoMachine.java

+    public LottoTickets issueLottoTickets(PurchasingPrice purchasingPrice, List<ManualTicketNumbers> manualTicketsNumbers) {
+        int purchasableTicketCounts = purchasingPrice.calculatePurchasableTicketCounts(LOTTO_TICKET_COST);
+        int manualTicketCounts = manualTicketsNumbers.size();
+        validatePurchasingPrice(purchasableTicketCounts, manualTicketCounts);
+        int automaticTicketCounts = purchasableTicketCounts - manualTicketCounts;
+        LottoTickets manualLottoTickets = LottoTickets.generateManual(manualTicketsNumbers);
+        LottoTickets automaticLottoTickets = LottoTickets.generateAutomatic(automaticTicketCounts, lottoNumberGenerator);
+        return manualLottoTickets.concat(automaticLottoTickets);
+    }


2개의 LottoTickets 객체를 생성해서 새로운 LottoTickets 객체를 생성해주네요!
1개의 LottoTickets 객체를 생성하는 방법은 없을까요??

LottoNumberGenerator 인터페이스가 있는데 ManualLottoNumberGenerator 이라는 구현체를 만들어 보는건 어떨까요??

제가 만든 Generator를 잘 활용해볼 생각은 못했네요... 😅😅

음... 자동티켓을 생성할 때는 자동티켓 개수와 자동번호생성기(인터페이스)가 필요하고
수동티켓을 생성할 때는 수동티켓 입력 번호 리스트가 필요한데요.

하나의 메서드가 자동과 수동에 필요한 인수들을 받아 내부적으로 1개 LottoTicket 객체만 생성하게끔 할 수 있긴 한데
파라미터의 수가 많아지다보니 자동과 수동을 나눈 뒤 concat하는 방식을 취했었습니다.

피드백 주신 내용을 바탕으로 리팩토링해보겠습니다. :D

- 자동 및 수동을 외부에서 합쳤던 기존의 기능을 LottoTickets이 한 번에 자동 및 수동 티켓들을 합쳐서 반환하는 방향으로 바꾼다. - 사용하지 않는 concat 메서드는 삭제한다.

- 기존의 dto 관련 클래스를 삭제한다. - 수동 입력과 관련된 클래스들을 도메인 패키지에서 작성한다. - String 배열을 통해 객체를 생성한다.

xlffm3 · 2021-02-23T07:41:23Z

안녕하세요 지노!

남겨주신 피드백을 바탕으로 많은 고민을 했었는데요...

ManualTicketNumbers라는 클래스의 역할에 대해.
- 처음에는 별도의 비즈니스 로직이 없어 dto라 생각해 패키지도 나누고 클래스명도 변경했는데요.
- 문제는 로또를 생성할 때 많은 클래스가 해당 DTO 객체에 의존하게 됩니다.
  - public static LottoTickets generate(List<TicketNumberDto> ticketNumberDtos, ...) 등.
- 도메인 객체들의 비즈니스 로직이 여러 레이어들 사이에서 데이터를 전달하는 목적으로 사용되는 DTO에 의존하는 것이 부자연스럽다고 생각되어 다시 domain으로 분리했습니다.

ManualTicket(사용자가 입력한 수동 로또 티켓 1장의 로또 번호들의 컬렉션(List<Integer>))과
ManualTickets(n장의 수동 로또 티켓의 컬렉션(List<ManualTicket>))를 사용했으며, 해당 객체를 View가 아닌 Controller에서 생성하도록 변경했습니다. :D

그리고 LottoNumberGenerator를 응용해서 ManualLottoNumberGenerator를 사용해보라는 피드백 또한 여러 방면으로 시도를 해보았는요. LottoTickets 객체를 외부에서 concat하지 않고 자동과 수동을 한 번에 생성하려면

public static LottoTickets generate(int autoTicketCounts, List<String[]> manualTickets, LottoNumberGenerator randomGenerator, LottoNumberGenerator manualGenerator)와 같이 파라미터의 수가 너무 많아지는 문제가 있었습니다.

또한 현재 인터페이스에 정의된 추상 메서드는 public List<Integer> generate()인데, 해당 메서드로는 수동 티켓을 다루기 어려운 상황입니다. 별도의 추상 메서드 public List<Integer> generateManual(String[])를 추가하면 되겠지만,,, 다형성의 이점을 잘 살리지 못하는 경우인 것 같아 해당 피드백을 반영하지 못했습니다. 😥

hyunssooo

안녕하세요 케빈!
step2까지 진행하신다고 수고 많으셨어요! 👏👏👏
고민하셨던 부분에 관해서는 제 생각 남겼습니다!
읽어보시고 참고만 해주세요!! 감사합니다!

hyunssooo · 2021-02-25T04:25:01Z

src/main/java/lotto/domain/LottoMachine.java

+
+    private final LottoNumberGenerator lottoNumberGenerator;
+


이 부분이 List<LottoNumberGenerator> 이라면 자동과 수동을 한번에 처리 가능할 거 같아요!

public class LottoMachine { private final List<LottoNumberGenerator> generators; .. } public interface LottoNumberGenerator { List<Integer> generate(); } public class ManualLottoNumberGenerator implements LottoNumberGenerator { private final List<Integer> numbers; public RandomLottoNumberGenerator(List<Integer> numbers) { this.numbers = numbers; } @Override public List<Integer> generate() { .. } }

현재는 LottoNumberGenerator.generate() 가 List<Integer> 를 반환하기 때문에 의미 없어 보일 수 있지만
List<LottoNumber> 혹은 LottoTicket 을 반환한다면 충분히 의미가 있을 거 같아요!

이런 방법도 있겠군요...! 단순히 Integer list만을 반환해야 한다는 생각때문에 다른 방법은 제대로 고려해보질 못했습니다. 😂 generator 인터페이스가 integer를 넘어서 lotto 관련 도메인 객체들을 반환해도 괜찮을지 공부하면서 고민해봐야겠네요. 이번 미션 리뷰하시느라 고생많으셨습니다!

케빈이랑 함께 고민한 시간 즐거웠어요!😃

xlffm3 added 16 commits February 21, 2021 13:46

docs: README 수동 구매 요구사항 정리

3ce3584

feat: 수동 구매 티켓 번호에 대한 일급 컬렉션 생성

1ed07a8

feat: InputView 수동 구매 로또 수 및 번호 입력 기능 구현

5478196

refactor: LottoTickets 자동 티켓 생성 메서드 파라미터 타입 변경

49dc0e8

- 로또 티켓의 가격은 외부의 로또 머신이 정한다. - LottoTickets가 생성하는 티켓의 개수를 내부에서 계산하지 않고 외부에서 주입받도록 한다.

refactor: 테스트 메서드 삭제

5a11ae3

feat: LottoTickets 수동 생성 기능 구현

f295bdf

feat: 두 개의 로또 티켓들을 연결하는 기능 구현

51051a2

feat: LottoMachine을 통해 수동과 자동 티켓 구매하는 기능 구현

effbe24

- 금액을 검사한 다음 그에 맞는 수동 및 자동 티켓을 생성해 반환한다

refactor: OutputView 구매 내역 출력 메서드 파라미터 변경

b435b6c

- 수동과 자동으로 각각 몇 장 출력한다.

refactor: 수동 입력 티켓 번호 변수명을 단수에서 복수 형태로 변경

21e9ea6

티켓들은 복수개도 가능하기 때문에 복수형으로 통일한다

refactor: LottoMachine 유효성 검증 로직 메서드 추출

bb5c677

refactor: 로또 티켓에 든 비용 계산의 주체를 Ticket에서 Machine으로 변경

662b547

로또 티켓은 자신의 가격을 모르도록 하고, 티켓의 가격을 결정하는 외부 기계가 구매 금액을 계산하도록 변경한다.

refactor: 1줄을 읽어 split하는 기능을 별도 메서드로 분리

2050817

refactor: LottoTicket 내부 컬렉션 Set 반환하도록 수정

5ff494d

refactor: 구매 가능 티켓 개수 계산시 나눗셈 예외처리 추가

94c9ff3

feat; LottoController 구현

07fa523

style: ticket -> tickets 복수형태 통일

216fb7a

hyunssooo suggested changes Feb 22, 2021

View reviewed changes

xlffm3 added 8 commits February 23, 2021 13:04

style: 컨트롤러 코드 개행 추가

46b01ed

refactor: ManualTicketNumbers 도메인 클래스를 DTO 클래스로 변경

39476e6

feat: 수동 입력 티켓에 대한 일급 컬렉션 생성

40c3d14

- 기존의 dto 관련 클래스를 삭제한다. - 수동 입력과 관련된 클래스들을 도메인 패키지에서 작성한다. - String 배열을 통해 객체를 생성한다.

refactor: LottoTickets 생성 메서드 파라미터 변경

67c302a

refactor: LottoMachine의 로또 생성 메서드 파라미터 변경

f35dc7b

feat: ManualTickets 사이즈 계산 기능 구현

f1a6fdd

refactor: Controller에서 List가 아닌 ManualTickets 사용하도록 변경

7be7421

hyunssooo approved these changes Feb 25, 2021

View reviewed changes

hyunssooo merged commit bb55978 into woowacourse:xlffm3 Feb 25, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[2단계 - 로또 구현] 케빈(박진홍) 미션 제출합니다. #282

[2단계 - 로또 구현] 케빈(박진홍) 미션 제출합니다. #282

xlffm3 commented Feb 21, 2021 •

edited

Loading

xlffm3 commented Feb 21, 2021

hyunssooo left a comment

hyunssooo Feb 22, 2021

xlffm3 Feb 23, 2021

hyunssooo Feb 22, 2021

xlffm3 Feb 23, 2021

hyunssooo Feb 22, 2021

hyunssooo Feb 22, 2021

xlffm3 Feb 23, 2021 •

edited

Loading

hyunssooo Feb 23, 2021

hyunssooo Feb 22, 2021

xlffm3 Feb 23, 2021

xlffm3 commented Feb 23, 2021 •

edited

Loading

hyunssooo left a comment

hyunssooo Feb 25, 2021

xlffm3 Feb 28, 2021

hyunssooo Feb 28, 2021


		private final LottoMachine lottoMachine = new LottoMachine(() -> Arrays.asList(1, 2, 3, 4, 5, 6));

[2단계 - 로또 구현] 케빈(박진홍) 미션 제출합니다. #282

[2단계 - 로또 구현] 케빈(박진홍) 미션 제출합니다. #282

Conversation

xlffm3 commented Feb 21, 2021 • edited Loading

xlffm3 commented Feb 21, 2021

학습 로그

[TDD] 점진적인 리팩토링 - 4

내용

[JDK] Comparator - 2

내용

[JDK] Stream의 concat - 1

내용

hyunssooo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xlffm3 Feb 23, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xlffm3 commented Feb 23, 2021 • edited Loading

hyunssooo left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

xlffm3 commented Feb 21, 2021 •

edited

Loading

xlffm3 Feb 23, 2021 •

edited

Loading

xlffm3 commented Feb 23, 2021 •

edited

Loading