[2단계 - 로또 구현] 에어(김준서) 미션 제출합니다.

[KJunseo]

안녕하세요 재연링!! 에어입니다 ㅎㅎ 😄
말씀해주신 피드백과 수동 구매 기능을 추가하면서 구조가 많이 바뀐 것 같아요. 변화, 추가되는 요구사항에 잘 대처할 수 있게 코드를 짜야하는데 아직 설계 능력이 부족하다는 생각이 많이 들었네요 ㅎㅎ..
지금 코드도 좋은 구조라는 확신은 없지만 저번에 말씀해주신 것 처럼 일단 제 생각대로 구현해보고 피드백을 받아보고 수정해보기로 했습니다!
피드백 주신 것 이외에도 수정사항이 많았는데 그 중 오늘 수업 때 이야기 나온 캐싱도 한 번 적용해봤어요! 제대로 적용했는지도 한 번 봐주세요!
많은 피드백 부탁드려요~~

---

궁금증

Q. 인스턴스 변수의 초기화를 한 곳에서 하는 것이 불가능한 경우면 설계가 잘못되었다고 볼 수 있을까요?

Q. 이번에 캐싱을 적용해보면서 현재 Number 클래스에서는 1~45의 Number를 미리 생성할 때 생성자에서 validation을 거치고, 외부에서 Number 값을 가져오게 하려할때 valueOf(정적 팩토리 메소드) 내부에서도 동일한 validation을 거친 후에 꺼내주는데 뭔가 중복된 느낌이 드네요ㅠㅠ 잘못된 구조로 설계한 것 일까요?

Q. 정적 팩토리 메소드를 사용할 경우는 생성자는 꼭 private으로 둬야하나요? public으로 놔두고 둘 다 사용해도 되나요?

Q. 수동 로또 생성 기능이 추가되기 전에는 Tickets의 내부에서 로또 생성 전략을 받아서 Ticket을 생성하는 식으로 구현했었는데, 수동 기능이 추가되면서 사용자 입력을 받아야되기 때문에 Tickets 도메인에서 입력을 받는 것이 좋지 않은 것 같아 기존의 자동 생성 로직도 일관성을 위해 컨트롤러로 뺐습니다. 이렇게 Tickets를 생성하는 로직을 컨트롤러에서 처리해도 괜찮을까요?

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

    // 자동 로또 생성
    private Tickets autoTicketGenerate(LottoCount count) {
        List<Ticket> tickets = new ArrayList<>();
        while (count.isGreaterThanZero()) {
            count = count.decreaseOne();
            tickets.add(new Ticket(new RandomNumbersGenerator().generate()));
        }
        return new Tickets(tickets);
    }

    // 수동 로또 생성
    private Tickets manualTicketGenerate(LottoCount count) {
        try {
            OutputView.enterManualTicketNumber();
            List<Ticket> tickets = new ArrayList<>();
            ticketGenerate(count, tickets);
            return new Tickets(tickets);
        } catch (RuntimeException e) {
            OutputView.printError(e);
            return manualTicketGenerate(count);
        }
    }

    private void ticketGenerate(LottoCount count, List<Ticket> tickets) {
        while (count.isGreaterThanZero()) {
            count = count.decreaseOne();
            tickets.add(new Ticket(new ManualNumbersGenerator(InputView.inputNumbers()).generate()));
        }
    }

Q. 생성자를 포함한 구조가 많이 바뀌다보니 테스트 코드도 수정이 있었는데 이렇게 구조를 바꾸고 테스트 코드를 바꿔주니 끼워맞춘듯한? 느낌이 들어서 올바르게 테스트를 하고 있는지 잘 모르겠어요..!

Q. 현재는 아래와 같이 되어있는데

public static final int ZERO = 0;
public static final int ONE_COUNT = 1;

이런 값도 상수처리 하는 것이 의미가 있을까요?

Q. 검증시 반환 값이 존재해도 되는 걸까요? 현재 반환 값이 존재하는 validate이 여러개 있습니다. void validate으로 하고 싶지만 이렇게 한 이유는 검증 때 parseInt를 이미 했는데, 검증이 끝나고 또 parseInt를 하는 것이 비효율적이라고 생각해서입니다!

    private Number(String value) {
        this.number = validate(value);
    }

    private static int validate(String value) {
        int number = validateNumber(value);
        validateNumberInRange(number);
        return number;
    }

   public static int validateNumber(String value) {
        try {
            return Integer.parseInt(value);
        } catch (NumberFormatException e) {
            throw new IllegalArgumentException(ERROR_MESSAGE_INVALID_INPUT);
        }
    }

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

현재는 이런 로직입니다.

1. 구입 금액을 입력받기
2. 구입 금액을 가지고 살 수 있는 전체 로또 개수를 구하여 LottoCount 객체 생성
3. 수동 로또 개수를 입력받아서 LottoCount 객체 생성
4. 전체 로또 개수에서 수동 로또 개수를 빼줌으로 자동 로또 개수 구하여 LottoCount 객체 생성
5. 수동 LottoCount 객체를 이용해서 수동 Tickets 생성
6. 자동 LottoCount 객체를 이용해서 자동 Tickets 생성

4)번에서 수동 로또 개수로 살 수 있는 최대 개수보다 더 큰 값이 들어오는 경우를 검증해서 예외를 던지는데, 사용자에게 입력을 받는 부분은 3)번이어서 어떤식으로 예외처리를 해줘야 완전 처음으로 안돌아가고 다시 3)번 부터 진행되게 할 수 있을까 고민해봤지만 답을 찾지 못했습니다. 이 부분도 힌트 주실 수 있을까요??

항상 감사드립니다~ 이번에도 잘 부탁드려요!! 😄

[jaeyeonling]

2단계 미션을 잘 구현해주셨습니다 👍
질문에 대한 답변과 추가적인 피드백 남겨두었으니 확인 부탁해요!

[jaeyeonling]

Suggested change

	return new LottoCount(String.valueOf(lottoCount - ONE_COUNT));
	return new LottoCount(lottoCount - ONE_COUNT);

int 생성자도 제공해주면 어떨까요!?

[jaeyeonling]

해당 클래스의 역할은 어떻게 될까요?

[KJunseo]

만들어진 통계를 바탕으로 수익률을 계산하는 책임을 가지고 있는 객체가 따로 필요하다고 생각했는데 통계 객체 자체에서 수익률을 계산해도 될 것 같네요!

[jaeyeonling]

String이라는 자료형이 상금을 표현하기에 적절한 자료형인지 고민해보아요

[jaeyeonling]

1과 45란 숫자 또한 상수를 이용할 수 있을 것 같습니다

[jaeyeonling]

해당 코드에서 로또의 번호를 한번에 입력받은 후 생성하게 변경한다면 어떠한 차이가 있을지 고민해보세요 :)

[jaeyeonling]

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

[jaeyeonling]

수동 생성 시 모든 번호를 받음 -> 로또들을 생성
하는 인터페이스로 분리한다면 두 구조를 합칠 수 있을 것 같습니다!

[jaeyeonling]

Suggested change

	- [ ] 로또 구매
	- [x] 로또 구매

매의 눈 @_@

[jaeyeonling]

String과 같은 문자열이 아닌 응답에 적절한 타입이 어떤게 있을지 고민해보면 좋을 것 같습니다!
설계하고 기능을 구현할 땐 View에 대한 부분은 생각하지 않고 구현 후 View를 붙이는 습관을 들이면 자연스럽게 역할과 협력의 기준으로 설계하실 수 있습니다 :)

[jaeyeonling]

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

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번 작업을 묶어서 메서드로 분리하는 방법이 가장 간단할 것 같네요!

[KJunseo]

A1 -> 이번 미션 같은 경우는 그런 경우가 없었는데 갑자기 궁금해져서 여쭤봤어요!! ㅎㅎ

A3 -> 정적 팩토리 메소드를 사용할 때 생성자를 private 해주는 이유에 대해 생각해봤어요. 제 생각에는 일관성 때문이라는 생각이 들었어요. 생성자로도 인스턴스를 만들 수 있고, 정적 팩토리 메소드로도 인스턴스를 만들 수 있으면 헷갈리고 일관성이 깨질 수 있겠다는 생각을 했어요. 혹시 다른 이유일까요??

A6 -> Tickets의 생성로직을 수정하면서 TicketsTest에서 Tickets를 생성하는 부분들이 다 바뀌었는데 그 중 void ticketsCreate()와 void checkTicketsRanking()의 Tickets를 만드는 부분이 끼워맞춘게 아닌가라는 생각이 살짝 들었어요!

---

추가 궁금증

Q1. LottoCount의 생성자들의 순서가 조금 궁금해요! 아래와 같이 public 사이에 private이 존재해도 컨벤션에 문제가 없을까요? 실제로 값을 초기화하는 생성자를 가장 아래로 내리려다보니 이렇게 됐어요!

public LottoCount(Money money) {
    this(money.divideMoney(Ticket.PRICE));
}

private LottoCount(int value) {
    this(String.valueOf(value));
}

public LottoCount(String value) {
    this.lottoCount = validate(value);
}

Q2. 마찬가지로 LottoCount의 생성자 관련 질문인데요. Int 생성자를 만드는 게 좋을 것 같다는 피드백을 보고 만들어봤는데 인스턴스 초기화를 한 곳에서 하려고 this()를 사용했습니다. 만들고 보니 동일하게 String.valueOf()를 사용하게 되었는데요. 크게 상관이 없을까요?

private LottoCount(int value) {
    this(String.valueOf(value));
}

Q3. 수동 로또를 한번에 받아서 처리한다면 한줄씩 잘못 입력되자마자 다시 입력받게 하려면 View단에서 try-catch를 사용해야할 것 같은데, 해당 부분만 View단에서 예외처리를 하면 일관성이 없어보이지 않을까라는 생각이 들었어요. 어떻게 생각하시나요??

Q4. 
수동 생성 시 모든 번호를 받음 -> 로또들을 생성
하는 인터페이스로 분리한다면 두 구조를 합칠 수 있을 것 같습니다!

이 피드백을 보고 리팩토링을 해보다보니 중복은 없어졌는데, 아에 별개의 것 처럼 바뀐 것 같아요. 의도하신대로 리팩토링을 한걸까요??

[jaeyeonling]

Suggested change

	public class LottoService {
	public static Tickets buyManualTickets(List<String> manualLottoNumber) {
	List<Ticket> manualTickets = manualLottoNumber.stream()
	.map(ManualNumbersGenerator::new)
	.map(ManualNumbersGenerator::generate)
	.map(Ticket::new)
	.collect(Collectors.toList());
	return new Tickets(manualTickets);
	}
	public static Tickets buyAutoTickets(LottoCount count) {
	List<Ticket> tickets = new ArrayList<>();
	while (count.isGreaterThanZero()) {
	count = count.decreaseOne();
	tickets.add(new Ticket(new RandomNumbersGenerator().generate()));
	}
	return new Tickets(tickets);
	}
	}
	public class LottoService {
	private final NumbersGenerator generator;
	public static Tickets buyManualTickets(List<String> manualLottoNumber) {
	List<Ticket> manualTickets = manualLottoNumber.stream()
	.map(ManualNumbersGenerator::new)
	.map(ManualNumbersGenerator::generate)
	.map(Ticket::new)
	.collect(Collectors.toList());
	return new Tickets(manualTickets);
	}
	public static Tickets buyAutoTickets(LottoCount count) {
	List<Ticket> tickets = new ArrayList<>();
	while (count.isGreaterThanZero()) {
	count = count.decreaseOne();
	tickets.add(new Ticket(generator.generate()));
	}
	return new Tickets(tickets);
	}
	}

인터페이스의 장점이 무엇인지 고민해보고 어떠한 차이가 있을지 고민해보아요!

[KJunseo]

말씀해주신 부트스트래핑 과정을 좀 찾아봤는데 클래스는 컴파일 시점이 아니라 클래스가 처음 사용될 때 JVM 내부로 클래스 로딩을 하는 것 같습니다. 따라서 main에서 먼저 인터페이스 구현체를 생성하여 사용해주면 애플리케이션 시작 시점에 클래스 로딩이 되기 때문에 만약 구현체가 바뀌더라도 Controller와 Service에서는 코드 변경 없이 확장이 가능하게 되는 것이지만 Controller나 Service에서 인터페이스 구현체를 생성한다면 구현체가 바뀌면 Controller와 Service의 코드도 바뀌기 때문에 OCP를 위반한다.(기존 코드가 변경됨)

라는 개념으로 이해했습니다! 말씀해주신 부분이 이 부분 맞을까요??

그렇다면 인터페이스를 사용할 때는, 항상 main에서 구현체를 생성하여 controller에 주입하는 식으로 사용하는 것이 좋다고 이해해도 될까요?

OCP: 개방 폐쇄 원칙

• 기능을 확장 하면서도 기능을 사용하는 기존 코드는 변경되지 않는 것.(확장에는 열려있고 변경에는 닫혀 있어야 한다.)
• 유연함과 관련된 원칙. 기존 기능을 확장하기 위해 기존 코드를 수정해 주어야 한다면, 새로운 기능을 추가하는 것이 힘들어진다.
• 변화가 예상되는 부분을 추상화함으로서 사용자 입장에서 변화를 고정시킨다.
• 변화되는 부분을 추상화하지 못하면 OCP를 지킬 수 없게 된다.

[jaeyeonling]

추가적인 요구사항도 잘 반영해주셨네요 👍
수 많은 피드백 반영하시느라 고생 많으셨습니다
다음 단계도 화이팅입니다 :)

[jaeyeonling]

인터페이스로 분리하여 모킹을 할 수 있게 됐고, 의도한 값이 나올 수 있게 변경됐네요 👍