[Java] 리펙토링 전 문제 점을 코드 리뷰 하면서 찾아보자.

 

개요

코드를 나름 열심히 생각하고 만들면서 나중에 확정성하고, 코드를 보기 편하게 만들었다고 생각을 하였다

하지만 코드를 들고 가서 많은 지적을 들었는데,

오늘은 이 지적을 받아드리고 적용을 하기 위해 내용을 정리를 해볼까 한다.

 

과제 내용은 키오스크 CLI 로 출력하는 과제이다.

 

순서는 코드의 흐름 순서대로 진행을 하도록 하겠다.

 

고칠 점.1

public class Main {
    public static final int END_NUMBER = -1;

    public static void main(String[] args) {

        // 데이터 가져오기
        MenuList<Menu> menus = MenuData.getMenus();

        Kiosk<Menu> kiosk = new Kiosk(menus);
        kiosk.start();
    }
}

 

Main 에서 사용이 안되는 static final 은 사용하는 곳에 옮기도록 하자.

 

연관이 되는 곳에 내용이 있어야지 외부에서 쓰인다고 Main 에 두는 것이 아니다. ( 게다가 외부에서 한곳에만 사용이 되었다. )

 

 

 

고칠 점.2 

public class Kiosk<T extends Menu> {

    private final MenuList<T> menus;
    private final MenuList<T> shop;
    private final InputUtil iu;
    private Discount discount;

    // 기본 생성자
    public Kiosk(MenuList<T> menus) {
        this.menus = menus;
        this.shop = new MenuList<>();
        this.iu = new InputUtil(new Scanner(System.in));
    }

 /* 생략 */
 }

 

MenuList 을 같은 기능을 한다고 해서 같은 클래스 이름을 사용하면 안된다.

다시 설명을 하면 하나의 클래스로 두개의 목적을 다루고 있는 것.

 

이 문제에서 MenuList 는 Menu 를 담고 있는 리스트 인데

사용자 장바구니가 shop이라는 이름으로 같은 MenuList 을 가지고 있는 점은 잘못된 것이다.

 

같은 기능을 사용을 하더라도 이름이 다른 별도의 클래스를 만드는 것이 맞다.

 

 

 

고칠 점.3

public class InputUtil {

    private final Scanner sc;

    public InputUtil(Scanner sc) {
        this.sc = sc;
    }

    public int inputNum(int first, int size) {
        // Depth 를 3 이하로 하면 좋다는데
        // 무조건 3이하로 해야하는가?
        int num = -1;
        while (true) {
            try {
                num = sc.nextInt();
                if (first <= num && num <= size) break;
                else throw new NumberFormatException("메뉴에 없습니다.");
            } catch (NumberFormatException e) { // 넘치는 숫자 입력
                System.out.println(e.getMessage());
            } catch (InputMismatchException e) { // 문자 입력
                System.out.println("잘못된 입력입니다.");
                sc.next();
            }
        }

        return num - 1;
    }
}

 

이 클래스 안에 있는 메소드는 Kiosk 클래스에서만 사용이 되는데 따로 클래스를 분리를 할 필요가 없다.

 

그리고 유틸은 한 기능을 위해서 나누는 것이 아니다.

여러 가지를 모을 수 있어야 Util로 나눈다.

 

 

고칠 점.4

public enum MenuType {
    BURGER(0, "Burger"),
    DRINKS(1, "Drinks"),
    DESSERTS(2, "Desserts");

    private final int numberType;
    private final String stringType;

    MenuType(int numberType, String stringType) {
        this.numberType = numberType;
        this.stringType = stringType;
    }

    public static MenuType getTypeNum(int num) {
        return Arrays.stream(values())
                .filter(item -> item.getNumberType() == num)
                .findAny().orElse(null);
    }

    public static List<String> typeList() {
        return Arrays.stream(values())
                .map(MenuType::getStringType)
                .toList();
    }

    public static int size() {
        return MenuType.values().length;
    }

    public String getStringType() {
        return stringType;
    }

    public int getNumberType() {
        return numberType;
    }
}

 

변수 명에 Type이라고 붙이는 것은 다른 사람이 사용할 때 이해하기 어려울 수 있다.

보통 Type은 바뀔 수 있는 형태를 말하는 것이다.

ex) numberType -> 넘버 종류이다.

그러므로 리턴을 해준다고 하면 number 혹은 num OR no ( Number ) 줄인 말을 사용하는 것이 좋다.

 

추가

MenuType 클래스를 왜 쓰는지 다시 생각을 해보라고 하셨다.

일단 Menu는 추상클래스고 밑에 Burger, Drinks, Desserts 종류가 또 상속을 받고 있다.

하지만 "이것을 MenuType을 가지고 찾을 필요가 있는가" 에 대해서 말해주셨다.

차라리 추상클래스에 공통 내용을 더 추가를 해서 찾으면 되지 않는가 에 대해서 지적을 해주셨다.

 

 

고칠 점.5

 

 

추상 클래스와 밑에 상속받는 자식 클래스들이 패키지가 나누어져있는 것이 이상하다.

패키지 안에 같이 있어야 된다. 하나로 합쳐라

 

+ 패키지 이름은 첫번째가 대문자가 오면 안된다.

[Java] (Project, Package, Class, Method) Naming 규칙

 

 

추가로 알려주신 점

고차함수 배워서 적용해보기.

질문을 확실하게 정해놓고 가져오기

 

 

느낀 점

아 오늘 과제로 탈탈 제대로 털리니까

정신은 아픈데 바꿀점을 오히려 명확하게 알게 되어서 좋았다.

 

이제 내일은 이 내용들을 바탕으로 전부 리펙토링을 진행을 해보아야겠다...