[Java] 3주차 java-lotto 코드 리뷰

https://github.com/Japring-Study/java-lotto/pull/1

코드 리뷰 정리

1. Method를 더 잘게 나누자 (작으면 작을수록 좋다)
   • 코드 다시 보고 나눌 수 있는 함수 적어보기
   • indent는 2 초과로 하지 않기
     ex)

     • validate도 하나하나 나눠보기

       private void validateNumbers(List<Integer> numbers) {
               if (numbers.size() != 6) {
                   throw new IllegalArgumentException("[ERROR] 당첨 번호는 6개여야 합니다.");
               }
               if (numbers.stream().distinct().count() != 6) {
                   throw new IllegalArgumentException("[ERROR] 당첨 번호에 중복된 숫자가 있습니다.");
               }
               if (numbers.stream().anyMatch(n -> n < 1 || n > 45)) {
                   throw new IllegalArgumentException("[ERROR] 로또 번호는 1부터 45 사이의 숫자여야 합니다.");
               }
           }
     

     • printWinningStatistics 함수를 String Formatter를 사용하여 분리하기 (개인취향)

       public enum Format {
           REWARD_RATE_FORMAT("#,##0.0"),
           STATISTICS_RESULT_FORMAT("%s (%,d원) - %d개\n");
             
           private final String format;
             
           Format(String format) {
               this.format = format;
           }
             
           public String toString() {
               return this.format;
           }
       }
     

     src/main/java/lotto/util/StringFormatter.java
     https://github.com/Japring-Study/java-lotto/pull/3/files#diff-aedffb0cd79e6ba0de031470a7fef5e472ceacd6f6bc9e1cd6c6617afc20e495

2. input과 output의 역할을 정확히 분리 (유틸성)
   • InputView, OutputView가 입출력만을 담당하는게 좋다.
   • InputView, OutputView가 다른 class를 의존하는 것보다, 다른 class에서 InputView, OutputView를 의존하면 더 좋을 것 같습니다!

   ⇒ printWinningStatistics 와 printLotto 에서 Lotto나 LottoResult class를 파라미터로 받고 함수 내에서 사용

3. view에서는 로직이 들어가는 것은 옳지 않다 
   • view는 화면에만 집중
     ex)

        private void validateNumbers(List<Integer> numbers) {
           if (numbers.size() != 6) {
               throw new IllegalArgumentException("[ERROR] 당첨 번호는 6개여야 합니다.");
           }
           if (numbers.stream().distinct().count() != 6) {
               throw new IllegalArgumentException("[ERROR] 당첨 번호에 중복된 숫자가 있습니다.");
           }
           if (numbers.stream().anyMatch(n -> n < 1 || n > 45)) {
               throw new IllegalArgumentException("[ERROR] 로또 번호는 1부터 45 사이의 숫자여야 합니다.");
           }
       }
     

     if 안의 조건들도 로직으로 포함 빼서 따로 관리 필요

       private List<Integer> parseNumbers(String input) {
           return Arrays.stream(input.split(","))
                   .map(String::trim) //스페이스 제거
                   .map(Integer::parseInt) //정수 변환
                   .collect(Collectors.toList());
       }
     

     다른 부분으로 빼는 게 좋을 듯

   ⇒ LottoValidator, LottoParser 생성, 함수에 static을 붙여서 이동

   • static을 붙이는 이유! : 해당 메서드가 클래스의 인스턴스가 아닌 클래스 자체에 속하도록 하기 위해서입니다.
     static 키워드를 사용하는 경우, 인스턴스를 생성하지 않고도 클래스 이름으로 메서드를 직접 호출할 수 있습니다.
4. MVC 패턴으로 갈 경우 Service는 맞지 않다, Layered Architecture 로 갈 경우 Respository를 나눠라
   • MVC 패턴 (Java original POJO 느낌)
   • Layered Architecture (Java Spring 느낌)

   차이점의 경우 길어질 거 같아 따로 정리함: MVC Pattern vs Layered Architecture

5. if else 너무 많다

    public static LottoRank getRank(int matchCount, boolean bonusMatch) {
        if (matchCount == 6) return FIRST;
        else if (matchCount == 5 && bonusMatch) return SECOND;
        else if (matchCount == 5) return THIRD;
        else if (matchCount == 4) return FOURTH;
        else if (matchCount == 3) return FIFTH;
        return NONE;
    }
   

   • 해결법으로 : Stream API 사용

       public static LottoRank getRank(int count, boolean bonusMatch) {
           return Arrays.stream(values()) //모든 LottoRank 값을 스트림으로 변환
                   .filter(rank -> rank.matchCount == count)
                   .filter(rank -> rank !=SECOND|| (rank ==SECOND&& bonusMatch))
                   .findFirst()
                   .orElse(NONE);
       }
     

6. get 이라는 네이밍의 역할
   • getter: 단순히 필드값을 가져오는 것
   • getRank -> 네이밍 get을 바꾸기

   ⇒ findMatchingRank

7. 상수 하드 코딩

    List<Lotto> lottoList = lottoService.generateLottoNumber(money / 1000);
   

   • magic number 사용
8. 예외 처리를 같은 클래스 내에서 같이 하게 되면 지저분해 보일 수 있다.
   • 클래스를 따로 분리해서 처리하는 방법을 구상

9. println, print, printf 에 대해서

    System.out.printf("%s - %d개\n", lottoRank.getDescription(), result.getCountForRank(lottoRank));
       
   

   • println, print : 형식 지정이 필요 없는 경우나 간단한 출력 작업
   • printf : 형식 문자열을 사용하여 출력 형식을 지정
     • 숫자의 자릿수, 소수점 자리수, 문자열의 정렬 등을 쉽게 제어

---

다른 사람 코드 리뷰

toString 함수

• 객체를 표현할 수 있는 의미있는 정보(객체가 띄는 상징성)를 도출

BigDecimal이란?

• double이나 float 타입보다 더 정밀한 숫자 표현을 제공
• 부동소수점 계산의 정확도를 유지해야 하는 경우

디미터 법칙 (Law of Demeter)

• 객체 간의 상호작용을 최소화하여 객체의 결합도(Coupling)를 낮추고 코드의 유연성과 유지보수성을 높이는 설계 원칙
  • 결합도가 낮다 == 객체의 내부 구조가 외부로 노출되다

• 디미터 법칙을 위반하는 메서드 체이닝

    String city = company.getCeo().getAddress().getCity(); 
    // 객체의 내부 객체를 통해 접근
  

• 하지만, stream()은 예외

  Stream API 같은 경우에는 동일한 Stream으로 변환하여 반환 할 뿐, 캡슐화는 그대로 유지되므로 문제가 없다.
  만약 여러 .(도트)가 사용되더라도 객체의 내부 구현이 노출되지 않는다면 그것은 디미터의 법칙을 준수하는 코드이다.
  출처: https://mangkyu.tistory.com/147 [MangKyu’s Diary:티스토리]

정적 팩토리 메서드 (of)

별도의 객체 생성의 역할을 하는 클래스 메서드를 통해 간접적으로 객체 생성을 유도
출처: https://inpa.tistory.com/entry/GOF-💠-정적-팩토리-메서드-생성자-대신-사용하자 [Inpa Dev 👨‍💻:티스토리]

• 생성자 호출을 public으로 하는 건 위험하다.
• 간접적으로 하는 게 안전하다.
• 이름을 가질 수 있다 ⇒ 가독성 좋은 코드
• 객체 지향적 프로그래밍

정적 팩토리 메서드 네이밍 컨벤션

• from : 하나의 매개 변수를 받아서 객체를 생성
• of : 여러개의 매개 변수를 받아서 객체를 생성

• getInstance

  instance : 인스턴스를 생성. 이전에 반환했던 것과 같을 수 있음.

• newInstance

  create : 새로운 인스턴스를 생성

• get[OtherType] : 다른 타입의 인스턴스를 생성. 이전에 반환했던 것과 같을 수 있음.

• new[OtherType] : 다른 타입의 새로운 인스턴스를 생성.

  https://tecoble.techcourse.co.kr/post/2020-05-26-static-factory-method/

전략 패턴

• https://victorydntmd.tistory.com/292

구성 요소