[Review] 로또 3단계 - 코드 리뷰 반영 (2022/02/27)

 

코드 리뷰

이번 리뷰는 Dion께서 해주셨다!

1. 일급 컬렉션의 상속 사용?

우선 난 일급 컬렉션에 상속을 사용하면 안된다는 생각은 하였지만, 좀 구체적인 근거가 없는것 같아 팀원과 함께 질문하였다.

저희 팀은 1급 객체로 LottoTicket을 만들었습니다.

여기서 궁금한 점은, WinningNumbers 객체를 만들면서 1급 객체인 LottoTicket을 상속하는 방법이 떠올랐습니다.왜냐하면 기존의 1급 객체 에서의 코드를 재사용 하면 중복을 최소화 할 수 있기 때문이었습니다.
하지만 1급 객체는 독립적인 단위로 사용하는 것이다 보니, 상속을 하여 재사용 한다는 것 자체가 확실하지 않았습니다.

이에 대한 리뷰어님의 의견이 궁금합니다!

이에 대한 답변은 다음과 같았다.

다음 글을 읽어보면서 다시한번 생각하게 되었다.

코드 재사용 - 상속과 합성에 관하여 (with 카드 결제 금액 계산 애플리케이션)

 

2. LottoNumber 의 동일성 확인

기존에는 number.isSameNumber(another) 와 같이 따로 isSameNumber() 라는 메서드를 구현했었다.

다음 코드를 살펴보자.

public boolean isSameNumber(LottoNumber lottoNumber) {
    return this.number == lottoNumber.number;
}

 

기존의 equals가 있었지만 이를 사요안 이유는 의미 전달 때문이였다.

단순 equal 보다는 isSameNumber가 더 의미전달이 명확하지 않을까? 

 

오늘 다시 생각해보니 equals 만으로도 의미는 명확한 것 같다. 따로 만들 필요는 없는것 같다.

 

3. CheckWinningNumber 를 Stream으로 개선하기

- 변경 전

private int checkWinningNumber(WinningNumbers winningNumbers, LottoNumber myNumber) {
    int result = 0;
    for (int i = 0; i < winningNumbers.getSize(); i++) {
        result += check(winningNumbers.getNumberOfIndex(i), myNumber);
    }
    return result;
}

private int check(LottoNumber numberOfIndex, LottoNumber myNumber) {
    if (myNumber.isSameNumber(numberOfIndex)) {
        return 1;
    }
    return 0;
}

 

- 변경 후

우선 checkWinningNumber()를 stream으로 개선하기 전, check 부터 boolean 반환으로 변경하면 다음과 같다.

private boolean check(LottoNumber numberOfIndex, LottoNumber myNumber) {
    if (myNumber.equals(numberOfIndex)) {
        return true;
    }
    return false;
}

이제 본격적으로 checkWinningNumber을 변견해 보자.

private int checkWinningNumber(WinningNumbers winningNumbers, LottoNumber myNumber) {
    return (int)IntStream.range(0, winningNumbers.getSize())
            .filter(i -> check(winningNumbers.getNumberOfIndex(i), myNumber))
            .count();
}

Stream 으로 변경해 보았다!

 

4. List 대 ArrayList

사실 이번 과제에서 ArrayList를 사용한 이유는, 단순하게 과제의 요구사항이 "ArrayList를 사용하라" 였기 때문입니다.

다음 글을 읽어보며 다시한번 둘간의 차이를 생각해 보았다.

Polymorphism: Why use "List list = new ArrayList" instead of "ArrayList list = new ArrayList"?

결론만 말하면 구현체가 아닌 Interface에 의존해야 한다는 것 이다.

생각해보니 DIP 원칙은 지켜야 한다는점을 생각 못하고 짠 코드인것 같다. 단순하게 요구사항만 준수하려 하였다.

 

5. Seller의 역할

이에 대한 나의 생각은 다음과 같다.

돈은 구매자인 Person이 관리해야 하는 것 아닌가?

꼭 Seller까지 알아야 할까??

 

6. 결과 반환하기

결과의 반환을 사용한 이유는 다음과 같다!

우리 팀이 배열을 사용한 이유는! 각 index 별로 알맞은 값을 추가해주면, 예를 들어

 

0번 index 에 5개 => 0개 맞은 사람이 5명
1번 index 에 2개 => 1개 맞은 사람이 2명
...
6번 index에 1개 => 6개 모두 맞은 사람이 1명

이런 식으로 간단하게 표현할 수 있다 생각하였기 때문입니다.

혹, 리뷰어님 께서 다른 자료구조를 생각한 것 이라면? 어떤 이유가 있는 것 일까?
배열로 의미를 잘 전달할 수 있다 생각했는데, 더 나아가 따로 Result라는 Class로 만들어야 한 것 일까?

 

결과를 담아내는 독립적인 Result 라는 Class를 만들어도 좋을것 같다라는 생각은 든다!