본문 바로가기
프로그래밍/Java

나쁜 코드 지우기 6

by 이재만박사 2017. 11. 28.

* 일반


G14. 기능 욕심


 - 클래스 메서드는 자기 클래스의 변수와 함수에 관심을 가져야지 다른 클래스의 변수와 함수에 관심을 가져서는 안 된다

메서드가 다른 객체의 참조자(accessor)와 변경자(mutator)를 사용해 그 객체 내용을 조작한다면

메서드가 그 객체 클래스의 범위를 욕심내는 탓이다

자신이 그 클래스에 속해 그 클래스 변수를 직접 조작하고 싶다는 뜻이다

예를 들어, 다음 코드를 살펴보자


public class HourlyPayCalculator {

    public Money calculateWeeklyPay(HourlyEmployee e) {

        int tenthRate = e.getTenthRate().getPennies();

        int tenthsWorked = e.getTenthsWorked();

        int straightTime = Math.min(400, tenthWorked);

        int overTime = Math.max(0, tenthsWorked - straightTime);

        int straightPay = straightTime * tenthRate;

        int overtimePay = (int)Math.round(overTime * tenthRate * 1.5);

        return new Money(straightPay + overtimePay);

    }

}



calculateWeeklyPay 메서드는 HourlyEmployee 객체에서 온갖 정보를 가져온다

즉, calculateWeeklyPay 메서드는 HourlyEmployee 클래스의 범위를 욕심낸다

calculateWeelyPay 메서드는 자신이 HourlyEmployee 클래스에 속하기를 바란다


기능 욕심은 한 클래스의 속사정을 다른 클래스에 노출하므로, 별다른 문제가 없다면, 제거하는 편이 좋다

하지만 때로는 어쩔 수 없는 경우도 생긴다 다음 코드를 살펴보자


public class HourlyEmployeeReport {

    private HourlyEmployee employee;


    public HourlyEmployeeReport(HourlyEmployee e) {

        this.employee = e;

    }


    String reportHours() { 

        "Name : %s\tHours : %d.%1d\n",

        employee.getName(),

        employee.getTenthsWorked() / 10,

        employee.getTenthsWorked() % 10);

    }

}



확실히 reportHours 메서드는 HourlyEmplyee 클래스를 욕심낸다 

하지만 그렇다고 HourlyEmployee 클래스가 보고서 형식을 알 필요는 없다

함수를 HourlyEmployee 클래스로 옮기면 객체 지향 설계의 여러 원칙을 위반한다

HourlyEmployee가 보고서 형식과 결합되므로 보고서 형식이 바뀌면 클래스도 바뀐다




G15. 선택자 인수


 - 함수 호출 끝에 달리는 false 인수만큼이나 밉살스런 코드도 없다 도대체 무슨 뜻인가?

true로 바꾸면 뭐가 달라지는가? 

선택자(selector) 인수는 목적을 기억하기 어려울 뿐 아니라 각 선택자 인수가 여러 함수를 하나로 조합한다

선택자 인수는 큰 함수를 작은 함수 여럿으로 쪼개지 않으려는 게으름의 소산이다

다음 코드를 살펴보자


public int calculateWeeklyPay(boolean overtime) {

    int tenthRate = getTenthRate();

    int tenthsWorked = getTenthsWorked();

    int straightTime = Math.min(400, tenthsWorked);

    int overTime = Math.max(0, tenthsWorked - straightTime);

    int straightPay = straightTime * tenthRate;

    double overtimeRate = overtime ? 1.5 : 1.0 * tenthRate;

    int overtimePay = (int)Math.round(overTime * overtimeRate);

    return straightPay + overtimePay;

}


초과근무 수당을 1.5배로 지급하면 true고 아니면 false다

독자는 calculateWeeklyPay(false)라는 코드를 발견할 때마다 의미를 떠올리느라 골치를 앓는다

안타깝게도 저자는 다음과 같은 코드를 구현할 기회를 놓쳤다


public int straightPay() {

    return getTenthsWorked() * getTenthRate();

}


public int overTimePay() {

    int overTimeTenths = Math.max(0, getTenthsWorked() - 400);

    int overTimePay = overTimeBonus(overTimeTenths);

    return straightPay() + overTimePay;

}


private int overTimeBonus(int overTimeTenths) { 

    double bonus = 0.5 * getTenthRate() * overTimeTenths;

    return (int) Math.round(bonus);

}



물론 부울 인수만이 문제라는 말이 아니다. 

enum, int 등 함수 동작을 제어하려는 인수는 하나 같이 바람직하지 않다

일반적으로, 인수를 넘겨 동작을 선택하는 대신 새로운 함수를 만드는 편이 좋다



G16. 모호한 의도


 - 코드를 짤 때는 의도를 최대한 분명히 밝힌다

행을 바꾸지 않고 표현한 수식, 헝가리식 표기법, 매직 번호 등은 모두 저자의 의도를 흐린다

예를 들어 overTimePay 함수를 다음과 같이 짤 수도 있다


public int m_otCalc() {

    return iThswkd * iThsRate + 

    (int) Math.round(0.5 * iThsRate * 

    Math.max(0, iThsWkd - 400)

    );

}


짧고 빽빡할 뿐만 아니라 거의 해석이 불가하다

독자에게 의도를 분명히 표현하도록 시간을 투자할 가치가 있다



댓글