Level1 로또 미션 피드백 정리

로또 미션

 

로또 번호를 캐싱하여 불필요한 인스턴스 생성 방지

private static final Map<Integer, LottoNumber> lottoNumbers = new HashMap<>();;

static {
    for (int number = MINIMUM_LOTTO_NUMBER; number <= MAXIMUM_LOTTO_NUMBER; number++) {
        lottoNumbers.put(number, new LottoNumber(number));
    }
}

이 내용은 리뷰어님이 좋은 패드백을 주셔서 한 번 짚고 넘어가기 위해 넣었다. 로또 번호는 1부터 45까지 존재하고 값 타입이기 때문에 같은 번호의 객체가 2개 이상 생성될 일이 없다. 때문에 1부터 45개를 미리 캐싱해두고 사용하도록 했다. 이렇게 하면 불필요한 객체 생성을 막을 수 있다.

상수 관리 방법

상수를 관리하는 방법에는 상수를 관리할 클래스를 따로 만들고 상수를 모아 정의하는 방법이 있고, 상수를 사용하는 각각의 클래스에 따로 정의하는 방법이 있다. 처음에 각 클래스가 자신한테 의미 있는 상수를 관리하는 방법을 택했었다. 그런데 로또 번호의 최소, 최댓값 1, 45, 로또 가격인 1000이라는 숫자가 여러 로또 관련 도메인에서 중복돼서 사용되었다. 그래서 중복 사용되는 녀석들만 따로 LottoConstant 클래스를 만들어서 관리했었다.

상수를 LottoConstant나 상수를 사용하는 각 클래스에 분산되어서 정의하는 것보다는 LottoConstant에서만 상수를 관리하거나 상수의 의미를 잘 드러낼 수 있는 각 클래스에 정의하는 방법을 결정에서 한 방법으로 사용한다면 추후 관리에도 혼란을 만들지 않을거라 생각이 들어요.

그런데 다음과 같은 피드백을 받았다. 확실히 어떤 상수는 Constant 클래스에서 관리하고 어떤 상수는 어떤 클래스가 가지고 있으면 혼란스러울 수도 있겠다는 생각이 들었다. 이번 미션에서는 LottoConstant에서 로또 관련 상수들을 관리하는 방법을 선택했었다. 다음 미션 때는 각 클래스가 가지는 방법으로 선택하고 여러 곳에서 중복으로 쓰이는 상수들은 public으로 열어서 LottoTicket.PRICE, LottoNumber.MAX 이런 식으로 사용하도록 해봐야겠다. (실제로 Integer 클래스 같은 숫자 값 클래스는 Integer.MAX_VALUE로 자기 자신의 최대 범위를 상수로 정의해 놓았다.)

테스트할 땐 경계값을 잘 활용하자

@ParameterizedTest
@ValueSource(ints = {0, 46})
@DisplayName("로또 번호 범위가 아니면 예외")
void lottoNumberRangeException(int source) {
    assertThatThrownBy(() -> LottoNumber.getInstance(source))
            .isInstanceOf(IllegalArgumentException.class);
}

테스트를 작성할 때 경곗값을 잘 정의하는 것이 중요하다. 로또 번호의 비즈니스 로직이 1부터 45 사이의 정수여야 하기 때문에 예외로 0과 46을 넣어서 테스트했더니 경곗값을 잘 활용했다며 리뷰어님이 칭찬해 주셨다.

객체 역할과 책임 설계

객체에게 역할과 책임을 알맞게 배분하는 일은 항상 어렵고 고민이 되는 포인트이다. 로또 도메인 구조를 설계하면서 이 부분에 대해서도 리뷰어와 얘기를 나눴었다.

리뷰어

지금 LottoTicket과 LottoTicketNumbers가 어떤 책임을 부여받아서 협력하고 있을까요? 그부분부터 정리해봐야 할 거 같아요. 한번 고민해보시고 두 객체의 책임이 비슷하다면 하나를 정리해도 될 거 같아요.

나

LottoTicketNumbers는 로또 번호 6자리를 나타내는 객체입니다. 이 객체는 LottoNumber의 리스트가 온전히 6자리이고 중복되어서는 안된다는 로또 번호 6자리 그 자체에 대한 책임을 맡고 있습니다.
LottoTicket은 LottoTicketNumbers를 의존하며 당첨 번호와 보너스 번호를 통해 자신이 몇 등짜리 로또인지 알려주는 책임을 맡고 있습니다.
LottoTicket이 LottoTicketNumber를 가지지 않고 List<LottoTicket>을 바로 가지고 있고 6자리인 것과 중복이 안된다는 검증을 LottoTicket 자신이 할 수도 있다고 생각하긴 합니다. 그게 로또의 역할이라 해도 어색하지 않습니다.
다만 LottoTicket에 속하지 않으면서 6자리인 것과 중복되어서는 안 된다의 검증을 해야 하는 개념이 있습니다. 바로 당첨 번호입니다.
LottoTicketNumbers를 만들지 않았다면 똑같은 검증을 이 당첨 번호에게도 적용해야 했을 것입니다. 입력받은 당첨 번호를 LottoTicketNumbers객체로 취급함으로써 그러한 검증 과정이 깔끔해졌다고 생각합니다.
(또한 보너스 번호가 당첨 번호와 중복되는지 확인하는 로직도 괜히 controller 안에서나 다른 객체가 해주지 않아도 LottoTicketNumbers가 처리할 수 있습니다.)
제 생각은 이러해서 LottoTicket과 LottoTicketNumbers로 나눈 것이었습니다. 철시의 생각이 듣고 싶습니다!

리뷰어

네넵 생각 말씀해주셔서 감사해요. 당첨번호에 대해서도 동일한 검증이 필요하기 때문에 위 LottoTicketNumbers가 의미가 있었다면 정의하는 것이 의미가 있었다고 생각이 드네요. getter를 두 번 호출하게 되는 거는 일급 컬랙션 LottoTicketNumbers에서 바로 List의 값을 조회할 수 있는 메서드를 지원하도록 만드는 것도 방법이겠네요.

설계의 관한 부분은 정답은 없다고 생각한다. 다만 계속 고민하면서 그때 그 때 최선인 설계를 하면 된다는 생각이 들었다. 하지만 지금 내 설계보다 더 좋은 설계는 분명 있겠지...

AssertJ로 가독성 있는 테스트 코드 짜기

assertJ를 사용하면 스트림처럼 메서드를 이어 붙여서 가독성 좋은 테스트를 작성할 수 있다. 자세한 내용은 링크 참조

테스트 클래스 패키지 구조는 프로덕션 코드와 동일하게 가져가자

현재는 클래스가 많지는 않아서 두드러지지 않을 수 있지만 프로덕션 코드와 동일한 패키지 구조로 테스트 구조를 맞춰놓으면 가독성과 유지보수에도 좀 더 도움이 될거에요~

현재는 패키지 구조가 단순하다. 하지만 패키지가 복잡해지고 테스트 대상 클래스들이 여러 패키지에 산재해 있다면 테스트 클래스도 프로덕션 패키지 구조와 동일하게 가져가는 것이 유지 보수하기 좋다고 한다. 그래서 테스트 클래스들이 전부 도메인 패키지 안의 클래스들에 대한 것이었기에 테스트 클래스들도 test 밑에 domain 패키지를 만들어서 넣어주었다.

---

로또 1단계 피드백

로또 2단계 피드백