Skip to content

Conversation

@yangseungin
Copy link

안녕하세요 4단계 PR입니다.
세부 구체적인 요구사항이 없어서 프로그램 요구사항에 대해 문의드립니다.

Q1
예외 처리를 통해 에러가 발생하지 않도록 한다.
이항목의 경우 구입금액이나 수동 로또 수 입력 등에서 공백을 입력하는 경우 예외를 던지고 프로그램이 중단되게 되는데 중단되지 않도록 하라는 말씀이실까요? 공백입력의 경우 다시 입력받도록 하였는데 음수 입력이나 수동 로또를 제대로 입력하지 않는 케이스에 대해서는 다시 입력받도록 처리를 하지 않은 상태입니다.

Q2
수동로또를 위한 ManualLottos를 추가하였는데 LottoTickets를 보면 기존의 Lottos가 수동+자동 이고 ManualLottos로 수동을 가지고 있는 형태라 헷갈릴 수도 있을 것 같은데 LottoTickets의 필드로 AutoLottos, ManualLottos이렇게 가지고 있는게 명확할 수도 있을 것 같은데 어떻게 생각하시나요?

Q3 책임분리관점에서 Inputview의 inputManualLottos메서드에도 문제가 있을까요?
InputView는 ui입력만 담당하는데 ManualLottos라는 도메인객체를 직접 생성하고있어서 위배된다는 느낌이 드네요
그렇다고 List<List>를 반환하는것도 좋은것 같진 않네요

Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

3단계까지 객체 설계가 잘 되어 있다보니 4단계에 그리 큰 변경을 하지 않은 상태에서 기능 추가가 가능했던 것 같네요. 💯
LottoTickets의 역할에 대해 피드백 남겼으니 한번 고민해 보면 어떨까요?

Q1
예외 처리를 통해 에러가 발생하지 않도록 한다.
이항목의 경우 구입금액이나 수동 로또 수 입력 등에서 공백을 입력하는 경우 예외를 던지고 프로그램이 중단되게 되는데 중단되지 않도록 하라는 말씀이실까요? 공백입력의 경우 다시 입력받도록 하였는데 음수 입력이나 수동 로또를 제대로 입력하지 않는 케이스에 대해서는 다시 입력받도록 처리를 하지 않은 상태입니다.
A1.
저도 문구가 애매한 것 같아 '예외 처리를 통해 프로그램이 중단되지 않고 다시 입력할 수 있도록 구현한다.'로 수정했어요. 이 프로그램을 사용하는 사용자 관점에서 고민해 보라는 의미였어요. 어느 수준으로 개발할 것인지는 승인님이 선택해 구현해도 됩니다.

Q2
수동로또를 위한 ManualLottos를 추가하였는데 LottoTickets를 보면 기존의 Lottos가 수동+자동 이고 ManualLottos로 수동을 가지고 있는 형태라 헷갈릴 수도 있을 것 같은데 LottoTickets의 필드로 AutoLottos, ManualLottos이렇게 가지고 있는게 명확할 수도 있을 것 같은데 어떻게 생각하시나요?
A2.
저도 코드 보면서 같은 생각을 해 피드백 남겨봤어요.
현재와 같이 List를 가진다면 둘을 하나로 합쳐 구현하는 것이 더 나을 수도 있을 것 같아요.

Q3 책임분리관점에서 Inputview의 inputManualLottos메서드에도 문제가 있을까요?
InputView는 ui입력만 담당하는데 ManualLottos라는 도메인객체를 직접 생성하고있어서 위배된다는 느낌이 드네요
그렇다고 List를 반환하는것도 좋은것 같진 않네요
A3. 피드백에 남겨봤어요.
Lotto가 아닌 String이라면 View에서 ManualLottos를 생성해 반환하는 것도 좋은 선택이지 않을까 생각해 봤어요.

import java.util.List;
import java.util.Map;

public class LottoTickets {
Copy link
Contributor

Choose a reason for hiding this comment

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

이 객체가 담당하는 책임이 많아지고 있다.
Lotto 목록을 관리하면서 매칭 책임과 Lotto 목록을 생성하는 책임으로 두 가지 이상을 담당하고 있는 것 같다.
Lotto 목록을 생성하는 책임을 LottosFactory와 같은 다른 객체를 생성해 분리하면 어떨까?

'LottoTickets tickets = LottosFactory.createLottos(int purchaseAmount, List manualLottos)'

import java.util.Optional;

public class ManualLottos {
private final List<Lotto> manualLottos;
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
private final List<Lotto> manualLottos;
private final List<String> manualLottos;

위와 같이 사용자가 입력한 값을 String으로 저장하는 역할만 담당해도 되지 않을까?
List로 구현하니 LottoTickets과의 다른 역할이 없을 수도 있겠다.

}

System.out.println("수동으로 구매할 번호를 입력해 주세요.");
List<Lotto> manualLottos = new ArrayList<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
List<Lotto> manualLottos = new ArrayList<>();
List<String> manualLottos = new ArrayList<>();

앞에서 피드백한 바와 같이 String으로 저장하고 반환하는 것은 어떨까?

@yangseungin
Copy link
Author

오늘 다른일이 있어서 늦게시작해서 많은 생각을 하지못하고 수정해서 리뷰요청드리네요
LottoTickets의 manualLottos가 수동 입력문자열만 가지고 있다보니 lottos가 수동+자동 로또를 모두 가지고 있는 구조가 맞는거 같아요

Copy link
Contributor

@javajigi javajigi left a comment

Choose a reason for hiding this comment

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

피드백 반영하느라 고생했네요. 👍

바로 merge하려다 혹시라도 추가 연습해보고 싶은 부분이 있을 수 있어 request changes로 남겨봅니다.
혹시라도 추가 리팩터링해보고 싶은 부분 있으면 도전해 보시고요.
없으면 바로 리뷰 요청해도 됩니다.

아니면 로또 미션은 진행 상태로 두고 "수강신청 - 레거시 코드 리팩터링" 미션 진행하다 일정 시간이 지난 후 다시 리팩터링 도전해 보는 것도 좋은 연습 방법일 수 있습니다. AI와 함께 로또 미션의 객체 설계에 대한 피드백을 받아보는 것도 좋은 방법일 것 같아요.

import java.util.ArrayList;
import java.util.List;

public class LottoTicketsFactory {
Copy link
Contributor

Choose a reason for hiding this comment

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

이 객체의 복잡도가 높음
LottoTickets와 ManualLottos를 분리하지 말고 하나의 객체로 리팩터링할 방법은 없을까?
Lotto의 생성자에 String을 받아 List로 Lotto.from(String text)와 같은 정적 팩터리 메서드도 추가한다면...
로또 생성과 관련한 여러 측면에 고민을 추가로 해보면 어떨까?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants