[내일배움캠프Spring-17일차] 코드 리펙토링

구현까지 완료되어서 튜터님께 코드 리뷰를 받으러 갔다. 오늘은 해당 조언들을 통해서 적절하게 코드를 리펙토링 하는 과정을 가지고자 한다 !

1. 중복된 멤버 변수 선언을 객체로 선언

1) 문제 상황

CartItem.java 에 Menu 객체를 가져와 Name,Price, Description 을 별도로 선언.

2) 코드 리뷰

" 카트 아이템 있는 걸 메뉴아이템 객체 리스트를 받아오는게 좋을 거 같다. "

Why? 메뉴 아이템 객체를 그대로 클래스에 가져와서 이름, 가격, 설명을 복사하는 것이다. 중복된 코드라는 의미이다. 이렇게 하면 setter랑 getter도 복사한 객체를 다시 불러오는 수준이다. 굳이 복사하지 않고 객체를 가져오기만 해도 menuitem의 Getter 와 Setter 를 활용할 수 있다는 것이다.

그러니까 메뉴아이템 객체를 그대로 필드로 선언하면 메뉴 아이템 객체의 함수까지 이용할 수 있는 것이다.

3) 문제 해결

- 이렇게 하니 확실히 코드의 중복은 사라지면서 확장성이 높아지는 장점을 얻을 수 있다.

2. Menu 객체 초기화

1) 문제 상황

Main.java 에 너무 많은 정보를 담아냄.

2) 코드 리뷰

" Kiosk 객체가 만들어질 때 마다 메뉴와 메뉴 아이템들을 생성자에 초기화 해주면 될 거 같다"

Why? Main.java 에는 최대한 많은 정보를 담아두는건 좋은 방향이 아닐 수 있다.

3) 문제 해결

Main 에 있던 모든 객체 생성을 Kiosk.java 의 생성자에 넣을 것이다. 이렇게 하면, 키오스크가 켜질 때 마다 값이 키오스크 안에서 초기값이 세팅 되는 것이다.

그래서 아예 List<Menu>를 만들어서 해당 리스트를 Kiosk 생성자에 넣어둘 것이다.

  private List<Menu> initMenus() {
    // Menu 카테고리 이름 설정
    Menu burger = new Menu("Burger \uD83C\uDF54");
    ...

    /* Burger */
       burger.addMenuItems(new MenuItem("Whopper", 7100, "버거킹의 대표 메뉴로, 두툼한 패티와 신선한 채소가 어우러져 클래식한 맛을 제공하는 와퍼"));
       ...

    /* Drink */
    drink.addMenuItems(new MenuItem("Americano", 1500, "진하고 깔끔한 커피 본연의 맛을 즐길 수 있는 음료"));
    ...

    /* Desert */
    desert.addMenuItems(new MenuItem("French Fries", 2100, "바삭하고 황금빛으로 튀겨진 감자"));
    ...

    return new ArrayList<>(Arrays.asList(drink, burger, desert));
  }

이런식으로 Menu 리스트를 반환하는 init() 함수를 하나 만들어 준다.

  public Kiosk() {
    this.menus = initMenus();
  }

그런 후 생성자에 넣어주면 끝이다.

이렇게 되면 확실히 Main 에서는 Kiosk 의 시작만을 담당하기 때문에 Main 의 역할이 줄어든다는 장점이 있다.

3. Enum 속 기능 함수 제외

1) 문제 상황

Eunm 속 기능 함수 포함, 클래스 안에 Enum 클래스 포함

2) 코드 리뷰

" Enum은 클래스 안에 넣기 보다는 Enum 클래스로 만들어주는것이 좋을 거 같고, Enum 안에 기능적 함수를 두는 걸 비추천한다. "

- Enum은 이넘클래스로 만들어주는게 좋다. Eum은 Enum Class 로 만들던가, 메뉴나 메뉴 아이템, 키오스크아이템 등 다른 클래스 속에 이너클래스 넣던가 하는게 좋을 거 같다고 한다.

Why? Enum은 보통 고정된 값들의 집합을 표현할 때 유용하며, 독립된 enum 클래스로 만드는 것이 명확하게 그 값을 구분할 수 있고, 코드 유지보수 측면에서도 더 직관적이다. 보통 enum은 그 자체로 고유한 의미를 가지고 있기 때문에, 다른 클래스 내에 포함시키는 경우에는 enum이 표현하려는 값들이 해당 클래스의 동작이나 속성과 관련이 깊을 때 사용하는 것이 좋다. 따라서 enum을 독립된 클래스나, 클래스 내의 이너 클래스로 정의하면 코드의 가독성도 좋아지고, 재사용성도 높아진다.

- Enum 속에 별도의 기능함수를 넣어두지 말라

Why? Enum에 기능 함수를 넣게 되면, 그 Enum이 값뿐만 아니라 동작도 포함하게 되어 Enum의 본래 목적을 벗어나게 된다. 이로 인해 Enum이 너무 복잡해지고, 코드가 불필요하게 지나치게 결합되어 유지보수하기 어려워질 수 있다.

3) 문제해결

- 해당 로직을 Kiosk 클래스 속에 넣어두었다.

- Enum 이 이렇게 간단해진 걸 볼 수 있다!

public enum Discount {
  VETERAN(1, "국가유공자", 0.9),
  SOLDIER(2, "군인", 0.95),
  STUDENT(3, "학생", 0.97),
  GENERAL(4, "일반인", 1.0);

  private final String userType;
  private final int type;
  private final Double rate;

  Discount(int type, String userType, Double rate) {
    this.userType = userType;
    this.type = type;
    this.rate = rate;
  }

  public String getUserType() {
    return userType;
  }

  public Double getRate() {
    return rate;
  }

  public int getType() {
    return type;
  }

  public int getPercent() {
    return (int) Math.round((1 - rate) * 100);
  }
}

4. Enum 을 활용해서 출력 작성

1) 문제 상황

하드 코딩 출력

2) 코드 리뷰

"Enum Type 을 가져와서 Stream을 이용하여 출력해봐라"

- Why? 유지보수성, 확장성이 떨어지기 때문이다. Enum 을 가져와서 출력하게 되면, Enum 이 수정될 때 마다 따로 코드를 수정하지 않아도 되기 때문에 더욱 좋은 방향일 것이다.

3) 문제 해결

- Stream 을 활용하여 Enum을 가져오는 것인데 !

AtomicInteger index = new AtomicInteger(1);
Arrays.stream(Discount.values())
       .map(values -> String.format("%d. %s | %d %%", index.getAndIncrement(), values.getUserType(), values.getPercent()))
       .forEach(System.out::println);

여기서 AtomicInteger 는 Stream 내부에서 Index 처럼 사용할 수 있는 친구이다 ! 이렇게 하면 Integer.Range 를 만들지 않아도 index 를 넣을 수 있다.

Discount 의 열거타입 객체를 Stream 으로 가져와서

-> map 을 통해 index, Discount 의 UserType 과 Discount 의 Percent 를 가져와서

-> 각각의 String을 println 해준다.

이렇게 풀면 코드가 더 깔끔하고 Enum 의 유지보수가 쉬워진다.

5. Stream 활용

1) 문제 해결

if 와 for 문이 있어서 코드 가독성이 좋지 않은 것

2) 코드 리뷰

"Stream 을 활용할 수 있으면 활용해볼 것"

Why? 이렇게만 코딩하면 가독성이 좋지 않음. for 문과 if 문이 이렇게 있으면 한눈에 어떠한 로직인지 다른 사람이 보기에는 힘들 수 있다는 것이다. 즉 "한눈에 로직을 해석하기 힘들다"는 말은 직관적인 코드 흐름을 개선하는 것이 좋다는 말이다. Stream 을 활용하면 가독성, 효율성, 함수형 프로그래밍의 장점이 있다.

3) 문제 해결

Optional<Discount> enumDiscount = Arrays.stream(Discount.values()) // Create all listed objects in the list of enumerated objects
            .filter(discount -> discount.getType() == orderDiscount)
            .findAny();

이런식으로 해결할 수 있다 !

6. 예외 흐름 제어

1) 문제 상황

예외를 던지며 흐름을 제어하고자 함

2) 코드 리뷰

" 예외 처리는 특정 예외가 발생했을 때 그거에 대한 처리를 하는 것이지 에러로 흐름을 제어하지는 말라 "

Why? 이렇게 예외를 흐름 제어에 사용하는 것은 코드의 가독성과 유지보수성을 떨어뜨린다. 프로그램이 예외를 "예상한 상황"으로 다루게 되면 실제로 예외가 발생했을 때 그 예외가 진짜로 중요하고 치명적인 오류를 나타낼 때 구별하기 어려워진다.

지금와서 다시 보니 왜 이렇게 이중으로 처리되는 걸 담아냈는지 나도 잘 이해가 안 간다.. 생각조차 못했다. 코드리뷰를 하면서 깨달았고, 조언을 받아보니 이해가 됐다. 이제야 이 문제점이 와 닿는다. 이후부터는 모든 Try 문을 수정하는 코드 리펙토링을 진행하였다.

3) 문제 해결

- try 문이 낭비 되었던 부분을 수정하고, while 문 역시 수정하였다.

✅ 오늘의 회고

오늘은 진짜.. 아침에 받았던 코드 리뷰를 다 수정했어서 이제는 끝이겠지 했다.

그런데 질문 하나 하러가면서 코드리뷰를 한 번 더 진행했는데 여기서 수정해야할 사항이 또 우수수.. 나왔다 ㅠㅡㅠ..

정말 끝이겠지.. 싶지만 내일 코드리뷰 받으면 또 우수수 쏟아질 것이다 ㅎㅡㅎ!! 하하하 💀💀

그래서 오늘 하루종일 리팩토링 하는 시간을 가진 거 같다. 일단은 오늘은 ! 이 정도면 만족한다.

내일 Readme 쓰고 한 번 더 코드리뷰를 받고, Level 별로 다시 한번 점검 해보아야 겠다.