[우아한테크코스 - 프리코스] 늦은 3주차 로또 게임 회고 😓

 

🔗 나의 Pull Request Link

뒤늦게 올리는 3주차 회고록… 😓

 

😎 코드리뷰를 통해 성장해보자

---

프리코스를 함께 참여하고 있는 동료들에게 2주차 미션에 대한 코드리뷰를 주고 받았다.

코드리뷰 과정에서 내가 다른 코드들을 보며 느낀점과 내 코드에 대한 리뷰를 통해 아쉬운 부분과 배운 부분을 이번 미션에서 적용해보고자 했다.

 

 

1️⃣ 코드 컨벤션을 잘 지키자.

이전 미션에서 코드 컨벤션을 제대로 확인을 안한 내 자신의 코드를 발견했다..!

if(...) print(...);

 

작은 컨벤션이라도 지킬 수 있도록 꼼꼼하게 내 코드를 작성하고 검토를 할 때에도 더 신경써야겠다.

 

if(...) {
  print(...);
}

 

 

2️⃣ 정적 팩토리 메소드를 사용할 때 더 적절히 활용해보자.

public static RacingGameService from(final String name) {
     return new RacingGameService(name);
}

이렇게 지난 미션에서 위 코드처럼 정적 팩토리 메소드를 사용해보았다.

이에 관해 아래와 같은 리뷰를 받을 수 있었다.

 

“ 위 코드에서 단순히 String의 속성을 가지는 Service가 생기는 것도 아니라, String을 통해서 Car를 만들어낸 다음, 리스트로 구축하는 로직이 실행되는데 이런 메소드를 from이라고 하는게 맞는지 의문입니다 ”

 

리뷰를 받고 다시 한번 내 코드를 보니 정적 팩토리 메소드에 대한 장점을 전혀 살리지 않고 그냥 사용한 코드로만 보이게 되었다.

이를 통해 좀 더 정적 팩토리 메소드를 사용할 경우에는 장점을 더 살리기 위해 더 적절한 파라미터를 사용하고 메소드의 네이밍에 신경쓰고자 했다.

 

 

3️⃣ stream을 최대한 이용해보자.

다른 분들의 코드를 보면서 stream과 람다식을 통해 코드를 간결하게 하고 이를 통해 가독성이 매우 높아지는 것을 볼 수 있었다. 나도 stream을 적용할 수 있는 상황이라면 이를 적절히 사용해 코드를 짧게 유지해 가독성을 높여야겠다라고 생각하게 되었다.

 

 

4️⃣ 한 메서드에서 2가지 이상의 역할을 가지지 않도록 조심하자.

public List<Result> moveAllCars() {
    return racingCarList.stream()
            .map(car -> car.moveOrStop(RandomUtil.generateRandomValue(MIN_RANDOM_VALUE, MAX_RANDOM_VALUE)))
            .toList();
}

해당 메소드에서는 자동차를 움직이는 일, 자동차의 상태를 리턴하는 일을 처리하고 있다.

한 메소드에서 한 가지 이상의 역할만을 담당하게 작성할 수 있도록 꼼꼼하게 코드를 봐야겠다.

 

🤔 고민

---

1️⃣ 같은 로또 번호를 반복해서 사용할 수 없나?

이번 미션에서 로또에 사용되는 숫자를 VO 를 통해 표현했다.

이때, 여러개의 로또를 발행할 때, 같은 번호의 로또 번호를 발행할 때마다 새로운 VO 객체를 통해 처리하는 것은 매우 비효울적이라고 생각했다.

로또 번호는 단순히 값 객체로서 값을 나타내는 역할만을 담당하기 때문에 미리 1 ~ 45번까지의 로또 번호 객체를 생성해두고 이를 재사용하면서 로또를 만들어주는 것이 더 좋을 것 같다고 생각했고 이를 구현하고자 했다.

 

❗정적 초기화 블록을 사용하자!

미리 재사용할 로또 번호 값 객체들을 만들어 놓기 위해 정적 초기화 블록을 사용하기로 했다.

 

정적 초기화 블록이란❓

• 클래스가 로딩될 때 호출되어 각 클래스당 딱 1번 실행
• 객체가 생성되기 이전에 수행되어 인스턴스 멤버에 접근할 수 없다 ❌

 

🧑🏻‍💻 적용해보기

public class LottoNumber {

    private static final int MIN_LOTTO_NUMBER = 1;
    private static final int MAX_LOTTO_NUMBER = 45;
    private static Map<Integer, LottoNumber> lottoNumberCacheFactory = new HashMap<>();
    private Integer number;

    static {
        IntStream.range(MIN_LOTTO_NUMBER, MAX_LOTTO_NUMBER)
                .forEach(i -> lottoNumberCacheFactory.put(i, new LottoNumber(i)));
    }

    ...
}

- 정적 초기화 블록을 사용해 Map 자료구조에 로또번호로 사용될 LottoNumber 객체를 미리 만들어 저장해두었다.

 

👍 정적 초기화 블록을 사용했을 때의 내가 생각한 장점

 

🥇 정적 초기화 블록을 사용해서 클래스가 로드될 때 한 번만 실행되어 외부에서 의도치 않게 변경을 하는 경우를 방지할 수 있다.

 

🥈 한 번 초기화할 때 초기에 비용을 지출해서 이 후 객체를 사용할 때 초기화 비용이 필요 없다.

 

🥉 미리 필요한 객체들을 만들어 놓고 사용해서 필요한 객체에 빠른 엑세스가 가능해 성능이 향상

 

2️⃣ 로또 번호의 당첨 여부 확인에 대한 책임은 누가 담당해야 할까?

내가 뽑은 로또와 당첨 번호사이의 맞은 로또 번호의 개수를 판별해 당첨 여부를 확인하는 책임은 누구에게 있을까?

이런 고민을 하면서 당첨 여부를 확인하는 코드를 여러번 수정하는 일을 반복했다.

 

처음에는 로또 번호에서 당첨 번호를 가져다 당첨 여부를 확인했다. 하지만, 최종적으로는 당첨 번호 클래스에 로또 번호들을 넘겨주어 해당 로또들의 당첨 여부를 확인하도록 했다.

 

이유는 로또 도메인에서는 로또 번호들을 생성만 하고 일급 컬렉션처럼 동작하는 것을 기대했기 때문에 당첨 번호와 비교하는 연산을 추가하게 된다면 많은 책임을 가지게 되어 이는 원하는 것처럼 동작을 하지 않을 것이라고 생각했다.

 

public MatchResult matchLotto(Lotto lotto) {
    final AtomicInteger count = new AtomicInteger(0);
    final AtomicBoolean isMatchBonusBall = new AtomicBoolean(false);

    compareLotto(lotto, count, isMatchBonusBall);
    return new MatchResult(count.get(), isMatchBonusBall.get());
}

• Lotto를 파라미터로 받아 일치하는 로또 번호의 수를 구하도록 했다.

 

⭐ 3주차를 돌아보며

---

- 미션에 대한 피드백, 동료들과의 코드 리뷰를 통해서 좀 더 신경써야 할 부분이 많다는 것을 알게되었고 코드를 작성할 때도 간단한 컨벤션이라도 꼼꼼하게 확인을 해야겠다는 생각을 했다.

 

- 요구사항을 코드에 반영하는 것이 정말 쉽지 않다는 것을 많이 느꼈고 이를 해결하기 위해 요구사항에 대한 Flow를 다양한 관점에서 더 생각해보는 시간이 충분히 필요하다고 느꼈다.

 

- 이런 점들을 개선하고 다른 분들의 코드를 보며 내 코드와 비교해보고 더 좋은 방법이 어떤 것이 있을 지 더 생각해보는 시간을 가지고자 한다.