피드백 내용

1.2.3.

1. calculate method의 operator는 string으로 받지 말고 class, 혹은 enum class로 받으면 어떨까요?

• 변경 전:

/* 사칙연산 계산 부분 메소드 매개변수로 요소를 받아 계산 후 리턴
     throws 키워드를 통해 직접 예외를 처리하지않고 발생지에서 처리요청
     열거 객체를 통한 사칙연산 구분 */
    /* CalculatorManager 가 가지는 구현객체를 통해 객체를 추상클래스타입 변수로 갈아끼워 호출명은 동일하지만
     * 계산연산이 바뀌도록 다형성 부여 */
    public double calculate(T firstNum, T secondNum, String operator) throws ArithmeticException {
        operatorType = OperatorType.find(operator);
        switch (operatorType) {
            case ADD:
                cm.setCalculate(new AddCalculator());
                break;
            case SUB:
                cm.setCalculate(new SubCalculator());
                break;
            case MULT:
                cm.setCalculate(new MultCalculator());
                break;
            case DIV:
                cm.setCalculate(new DivCalculator());
                break;
            case REM:
                cm.setCalculate(new RemCalculator());
                break;
        }
        // 연산 과정 및 결과 저장 메서드 호출.
        result = cm.getCalculate().calculate(firstNum, secondNum);
        saveCalculationProcess(firstNum, secondNum, result, operator);
        return result;
    }

• calculate() 메소드가 String 타입으로 연산자를 받아오면서 switch를 통해 eunm 객체를 리턴받아 스위칭하기에 calculate() 메소드와 eunm 클래스를 동시에 수정해야하는 의존성 문제 발생.
• ArithmeticCalculator의 변경 없이도 연산자를 추가할 수 있는 구조로 변경필요.

• 변경 후

/* 사칙연산 계산 부분 메소드 매개변수로 요소를 받아 계산 후 리턴
     throws 키워드를 통해 직접 예외를 처리하지않고 발생지에서 처리요청*/
    // AbstractCalculator 객체를 받음으로써 calculate()는 오퍼레이터를 몰라도 연산이 가능
    public double calculate(T firstNum, T secondNum, AbstractCalculator cm) throws ArithmeticException {
        // 연산 과정 및 결과 저장 메서드 호출.
        result = cm.calculate(firstNum, secondNum);
        saveCalculationProcess(firstNum, secondNum, result, cm.getOperator());
        return result;
    }

switch (OperatorType.find(operator)) {
            case ADD:
                cm.setCalculate(new AddCalculator());
                break;
            case SUB:
                cm.setCalculate(new SubCalculator());
                break;
            case MULT:
                cm.setCalculate(new MultCalculator());
                break;
            case DIV:
                cm.setCalculate(new DivCalculator());
                break;
            case REM:
                cm.setCalculate(new RemCalculator());
                break;
        }
        /* Calculator 객체를 통한 calculate() 메소드를 통해 계산.
        정수인지 실수인지에 대한 타입분간.
        입력값중 음수가 있는지 판단.*/
        if (fisrtdoublecnt != 0 && seconddoublecnt != 0) {
            if (calc.negativeIntegerChecker(firstNumDouble, secondNumDouble)) {
                System.out.println("양수만 입력해주세요.");
            }
            System.out.println(firstNumDouble + " " + operator + " " + secondNumDouble + " = " + calc.calculate(firstNumDouble, secondNumDouble, cm.getCalculate()));

• AbstractCalculator 의 객체를 직접적으로 받으면서 calculate 메소드는 숫자와 객체만을 가지고 연산자가 무엇인지에 상관없이 계산에대한 동작을 보장함.
• 또한 Inner eunm Class 였던 OperatorType Class 르 외부 Class 로 이동. OperatorType을 외부에서도 호출가능하게 끔 변경.
• 하지만 결국 switch()의 장소가 ServiceManager Class 로 이동했을 뿐 Eunm 과 switch()의 의존관계에 대한 해결은 방법을 모르겠다. (답은 추석이후에..)
• (작성일 : 240919) : 피드백 상담 결과 다형성을 이용한 추가적인 책임의 분리가 맞았음.
• switch()의 의존관계에 대한 해결은 현재 구조가 아닌 Enum Class 에서 필드값으로 함수를 람다식으로 넘겨서 결과를 리턴받는식으로 하면 해결이 가능은 함.
  아니면 람다의 메소드를 넣는것도 가능하다!

public enum Operator {
    PLUS("+", (num1, num2) -> num1 + num2),
    MINUS("-", (num1, num2) -> num1 - num2),
    MULTIPLY("*", (num1, num2) -> num1 * num2),
    DIVIDE("/", (num1, num2) -> num1 / num2);
    
    
    or
    
    
public enum Operator {
    PLUS("+") {
        @Override
        public int calculate(int num1, int num2) {
            return num1 + num2;
        }
    },

참조 : https://velog.io/@ljinsk3/Enum%EC%9C%BC%EB%A1%9C-%EB%8B%A4%ED%98%95%EC%84%B1%EC%9D%84-%EA%B5%AC%ED%98%84%ED%95%98%EB%8A%94-%EB%B0%A9%EB%B2%95

2. emptyListChecker 등의 이름은 객체를 의미하는 것처럼 보여요.method의 이름은 동사로 짓는게 좋습니다.getIsEmptyList 등의 이름은 어떨까요?

• 변경 전 :

    // 빈 List 요청 확인 메소드
    private boolean emptyListChecker() {
        return firstNumbers.isEmpty();
    }

    // 요청 index 가 size 를 넘지 않았는지 체크메소드
    private boolean listIndexChecker(int index) {
        return firstNumbers.size() < index;
    }

    //입력값 중 음수가 있는지 확인하는 메소드
    public boolean negativeIntegerChecker(T firstNum, T secondNum) {
        return firstNum.doubleValue() < 0 || secondNum.doubleValue() < 0;
    }

• 변경 후 :

    // 빈 List 요청 확인 메소드
    private boolean isEmptyList() {
        return firstNumbers.isEmpty();
    }

    // 요청 index 가 size 를 넘지 않았는지 체크메소드
    private boolean isIndexOutOfBounds(int index) {
        return firstNumbers.size() < index;
    }

    //입력값 중 음수가 있는지 확인하는 메소드
    public boolean isNegativeNumber(T firstNum, T secondNum) {
        return firstNum.doubleValue() < 0 || secondNum.doubleValue() < 0;
    }

• 메소드명에 대한 코딩 컨벤션을 지키지 못한 부분.
• 패키지명 : 소문자
• 클래스명 : 단어의 첫 글자를 대문자로 시작하는 대문자 카멜표기법(Upper camel case)을 사용
• 클래스명과 인터페이스명은 명사로 짓는다. 테스트클래스는 클래스명이 TEST로 끝난다.
• 메서드명 : 첫 글자를 대문자로 작성하는 소문자 카멜표기법(Lower camel case)를 사용 동사로 짓는다.

3. 디렉토리 구조는 좀 더 정리해보면 좋을 것 같아요. 한 폴더에 너무 많은 클래스들이 들어가면 원하는 파일을 찾는게 힘들 수 있어서, 유사한 파일들을 묶어서 구조화시키면 더 좋을 것 같아요.

• 변경 전 :

  

• 변경 후 :

  

• 작은 프로젝트이기에 구분이 힘들지않지만 큰프로젝트일수록 패키지의 구조, 특히 Spring의 경우 패키지구조에 영향을 크게받기에 습관화가 필요할거같다.