[로또 - 1단계] 안석환 미션 제출합니다

[seokhwan-an]

미션을 진행하면서 고민한 점

로또 생성방안

맨 처음에는 1 ~ 45의 숫자 뭉치에서 랜덤한 숫자를 뽑는 방안을 떠올렸으나 숫자를 뽑을 때마다 같은 숫자를 뽑을 가능성이 높아지게 된다는 단점이 존재해 Collection.shuffle을 이용해서 로또를 추출하는 방안을 채택했습니다.
하지만 로또 여러개를 생성하는 과정에서 고민있었습니다.
<처음에 구현 방식>

private static final List<Integer> NUMBERS = Arrays.asList(
            1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
            11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
            21, 22, 23, 24, 25, 26, 27, 28, 29, 30,
            31, 32, 33, 34, 35, 36, 37, 38, 39, 40,
            41, 42, 43, 44, 45);

public Lottos generateRandomLotto(final Money money) {
    final List<Lotto> lottos = new ArrayList<>();
    for (int i = 0; i < money.getPurchaseQuantity(); i++) {
       Collections.shuffle(NUMBERS);
        lottos.add(new Lotto(NUMBERS.subList(0, 6));
    }
    return new Lottos(lottos);
}

처음에 구현한 방식에서 한 로또를 생성할 때 마다 NUMBERS를 Shuffle을 하고 앞에 6개로 로또로 선정을 했는데 모든 로또가 같은 결과물을 반환하였습니다.

그래서 shuffle 메소드를 디버깅 해보았습니다.

public static void shuffle(List<?> list) {
    Random rnd = r;
    if (rnd == null)
        r = rnd = new Random(); // harmless race.
    shuffle(list, rnd);
}

public static void shuffle(List<?> list, Random rnd) {
    int size = list.size();
    if (size < SHUFFLE_THRESHOLD || list instanceof RandomAccess) {
        for (int i=size; i>1; i--)
            swap(list, i-1, rnd.nextInt(i)); // 이부분이 실행됩니다.
    } else {
        Object[] arr = list.toArray();

        // Shuffle array
        for (int i=size; i>1; i--)
            swap(arr, i-1, rnd.nextInt(i));

        // Dump array back into list
        // instead of using a raw type here, it's possible to capture
        // the wildcard but it will require a call to a supplementary
        // private method
        ListIterator it = list.listIterator();
        for (Object e : arr) {
            it.next();
            it.set(e);
        }
    }
}

shuffle은 오버로딩으로 구현되어 있으며 결론적으로 아래 shuffle 메소드의 swap(list, i - 1, rnd.nextInt(i)) 부분이 실행이 됩니다. 여기서 rnd는 Random 객체이고 nextInt은 무작위 숫자를 반환하는 것인데 왜 반복문 마다 같은 shuffle 결과물을 반환하는지 의문이 듭니다. (혹시 제가 잘못 디버깅 했다면 의견 부탁드립니다.)

그래서 여러 로또를 생성하는 방식을 한번 shuffle 해놓고 뽑는 시작위치를 Random으로 추출해 각기 다른 로또가 나타나게 했습니다.
<변환 방식>

public Lottos generateRandomLotto(final Money money) {
    final List<Lotto> lottos = new ArrayList<>();
    Collections.shuffle(NUMBERS);
    Random random = new Random();
    for (int i = 0; i < money.getPurchaseQuantity(); i++) {
        final int randomNumber = random.nextInt(0, 40);
        lottos.add(new Lotto(NUMBERS.subList(randomNumber, randomNumber + 6)));
    }
    return new Lottos(lottos);
}

다만 이렇게 하면 한번 shuffle된 같은 숫자 뭉치에서 시작위치만 랜덤으로 추출하는 것이기에 많은 량의 로또를 구매하는 경우 중복된 로또가 나타날 확률이 증가할 것 같아 좋은 방법은 아닐 것 같습니다.

혹시 로또를 생성하는 좋은 방안이 있다면 리뷰 부탁드립니다! 이외에도 잘못된 부분이 있다면 편하게 리뷰 부탁드립니다!🙇‍♂️

[JoungMinJu]

석환님 1단계 구현하시느라 수고 많으셨습니다!!! 깔끔하게 작성해주신 덕분에 이해하기 매우 수월했던 것 같습니다 ^___^

질문주신 부분에 대한 답변과, 개인적으로 궁금한 점 몇 자 첨부했습니다 :)
확인해주시면 감사드리겠습니다 ! !

[JoungMinJu]

NUMBERS가 final이어서 suffle을 해도 정상적으로 동작하지 않는 것� 같아보입니다..! number를 복사한 list를 활용하신다면 원하는 로직이 구현될 것으로 보여요!!

[seokhwan-an]

저도 질문을 드리고 잘못된 부분을 찾아보았는데

private static final List<Integer> NUMBERS = Arrays.asList(
            1, 2, 3, 4, 5, 6, 7, 8, 9, 10,
            11, 12, 13, 14, 15, 16, 17, 18, 19, 20,
            21, 22, 23, 24, 25, 26, 27, 28, 29, 30,
            31, 32, 33, 34, 35, 36, 37, 38, 39, 40,
            41, 42, 43, 44, 45);

public Lottos generateRandomLotto(final Money money) {
    final List<Lotto> lottos = new ArrayList<>();
    for (int i = 0; i < money.getPurchaseQuantity(); i++) {
       Collections.shuffle(NUMBERS);
        lottos.add(new Lotto(NUMBERS.subList(0, 6)); // 문제가 된 부분
    }
    return new Lottos(lottos);
}

현재 Lotto를 생성할 때 Numbers의 sublist를 이용합니다. 이 때 Lotto는 Numbers의 참조값을 가지게 되는데 이 부분이 문제가 되었습니다.

이렇기에 shuffle이 이루어질 때마다 모든 로또(Lotto1, Lotto2, Lotto3)가 영향을 받게 되는 것이었습니다.
현재는 이 부분은 방어적 복사를 통해 참조를 끊어냈습니다

[JoungMinJu]

아 대박 이해했습니다!
덕분에 더 알고갑니다 🥹 수고많으셨습니다!

[JoungMinJu]

저는 로또 생성시 random.nextInt를 사용하여 1~45 숫자 6개를 뽑아내고, 요건에 쓰인대로 suffle을 사용했었는데
생각해보니 석환님이 구현하신 로직이 suffle을 사용하는 용도에는 더 적합할 것 같네요🥹
처음 의도대로 구현하신다면

다만 이렇게 하면 한번 shuffle된 같은 숫자 뭉치에서 시작위치만 랜덤으로 추출하는 것이기에 많은 량의 로또를 구매하는 경우 중복된 로또가 나타날 확률이 증가할 것 같아 좋은 방법은 아닐 것 같습니다.

이 문제도 해결될 것 같아 더 좋은 방법으로 보입니다!

[seokhwan-an]

처음 의도한 방향으로 수정했습니다!

[JoungMinJu]

저는 코드 구현시에 Money(구매금액)을 따로 객체로 빼려다가, Money 객체가 로또의 가격을 관리하고 있는 것이 ..? 뭔가 이상하다고 생각해서 객체로 생성하지 않았었습니다! 비즈니스 로직으로서 구매금액-로또개수 관련 로직을 관리하게 구현했었는데, money 객체가 로또의 가격을 관리??하고 있는 부분에 대해선 석환님은 어떻게 생각하시는지 의견이 궁금합니다! !

[seokhwan-an]

이 부분은 이름이 추상적이어서 생긴 오해인 것 같습니다.
제가 구현한 Money 객체는 단순하게 돈을 의미하는 것이 아닌 로또 구입 비용을 관리하는 객체를 의미하는 것이었습니다. 그래서 로또 구매 개수를 산출하는 메소드나 1000원 단위로 관리하는 것을 자연스럽게 여겼던 것 같습니다. 객체 명칭을 조금더 명확하게 수정하겠습니다!

[JoungMinJu]

Money 객체에 단 리뷰와 마찬가지로 저는 이렇게 되면 "lottos가 관리하는 구매한 로또 개수", "money가 관리하는 구매한 로또 개수"가 같은 의미인데 관리 지점이 두 개가 생기게 되어 문제가 되지 않을까..! 라고 생각했었습니다!

근데 지금 코드를 쭉 읽어보니, 결국 money의 로또 개수를 기반으로 lottos를 만드는 것이기 때문에 로직 상으로는 별 문제가 없을 것 같긴 한데........ 여튼 저는 저렇게 생각했었는데 석환님의 생각은 어떠신지 궁금합니다!!

[seokhwan-an]

관리 지점이 두개라고 보기보다는 구매한 로또 개수를 파악할 수 있는 출처가 Lottos와 Money라고 보면 좋을 것 같습니다. (Money는 이제 LottoPurchaseMoney로 변경될 예정입니다.) 그 이유는 민주님이 리뷰달아주신 것처럼 Lottos를 통해서 얻은 로또 개수와 Money를 통해서 얻는 로또 개수가 다르지 않기 때문입니다.

[JoungMinJu]

요거 StringJoiner 쓰면 더 간편해지지않을까요!?

[seokhwan-an]

오 한번 이용해서 수정해보겠습니다!

[JoungMinJu]

테스트코드 관련해서 궁금한 점이 있습니다!
subList로 무조건적으로 로또 숫자 개수는 6개가 나오도록 구현해놓은 상황인데, 이런 케이스에 대해서도 보통 테스트케이스를 작성하시는지 궁금합니다! !
저는 외부적인 요인(사용자의 입력)이 있는 경우만 주로 테스트 케이스로 작성하는 편이라서요🥹

[seokhwan-an]

저는 제가 작성한 검증 로직에 대해서는 모두 테스트를 작성하는 편입니다. 그 이유는 먼저 제가 작성한 코드가 모두 잘 동작하는지 파악하는데 큰 도움이 되는 것이 첫번째 이유이고 두번째는 이유로는 테스트 코드가 다른 개발자가 볼 때 그 사람이 구현한 기능을 파악하는데 도움이 된다고 생각했습니다. 두번째 이유는 제가 개인적으로 겪은 경험인데 다른 분의 코드를 리뷰할 때 테스트 코드를 바탕으로 구현한 로직을 보면 그 사람의 의도와 목적을 더 쉽게 파악하는데 도움이 되었던 것 같습니다! (또한 구현한 로직이 명확하게 파악되었습니다.)

[JoungMinJu]

잘 반영해 주신 것 같아 approve 하겠습니다! 수고 많으셨습니다 :)