[클린코드] Chapter17-냄새와 휴리스틱

주석

C1: 부적절한 정보

다른 시스템(ex. 소스 코드 관리 시스템, 이슈 추적 시스템 등)에 저장할 정보는 주석으로 적절치 못하다.
예를 들어, 변경 이력은 장황한 날짜와 따분한 내용으로 소스 코드만 번잡하게 만든다.
주석은 코드와 설계에 기술적인 설명을 부연하는 수단이다.

C2: 쓸모 없는 주석

오래된, 엉뚱한, 잘못된 주석은 더 이상 쓸모가 없다.
쓸모 없어질 주석은 아예 달지 않는 편이 가장 좋다.
쓸모 없어진 주석은 재빨리 삭제하는 편이 가장 좋다.
쓸모 없는 주석은 일단 들어가고 나면 코드에서 쉽게 멀어진다.
코드와 무관하게 혼자서 따로 놀며 코드를 그릇된 방향으로 이끈다.

C3: 중복된 주석

코드만으로 충분한데 구구절절 설명하는 주석이 중복된 주석이다.

C4: 성의 없는 주석

주석을 달 참이라면 시간을 들여 최대한 멋지게 작성한다. 단어를 신중히 선택한다.
주절대지 않는다. 당연한 소리를 반복하지 않는다. 간결하고 명료하게 작성한다.

C5: 주석 처리된 코드

읽는 사람을 헷갈리게 만든다. 흉물 그 자체다. 즉각 지워버려라!
걱정할 필요 없다. 소스 코드 관리 시스템이 기억하니깐 누군가 정말로 필요하면 이전 버전을 가져올것이다.

환경

E1: 여러 단계로 빌드해야 한다.

빌드는 간단히 한 단계로 끝나야 한다. 소스 코드 관리 시스템에서 이것저것 따로따로 체크아웃할 필요가 없어야 한다.

E2: 여러 단계로 테스트해야 한다.

모든 테스트를 한 번에 실행하는 능력은 아주 근본적이고 아주 중요하다.
따라서 그 방법이 빠르고, 쉽고, 명백해야 한다.

함수

F1: 너무 많은 인수

함수에서 인수 개수는 적을수록 좋다. 아예 없으면 가장 좋다. 다음으로 하나, 둘, 셋이 좋다.
넷 이상은 그 가치가 아주 의심스러우므로 최대한 피한다. (50쪽 “함수 인수” 참조)

F2: 출력 인수

함수에서 뭔가의 상태를 변경해야 한다면 (출력 인수를 쓰지 말고) 함수가 속한 객체의 상태를 변경한다. (56쪽 “출력 인수” 참조)

F3: 플래그 인수

boolean 인수는 함수가 여러 기능을 수행한다는 명백한 즈억다. 플래그 인수는 혼란을 초래하므로 피해야 마땅하다. (52쪽 “플래그 인수” 참조)

F4: 죽은 함수

아무도 호출하지 않는 함수는 삭제한다. 죽은 코드는 낭비다.
소스 코드 관리 시스템이 모두 기억하므로 걱정할 필요 없다.

일반

G1: 한 소스 파일에 여러 언어를 사용한다

소스 파일 하나에 언어 하나만 사용하는 방식이 가장 좋다.
- 예를 들어, 어떤 JSP 파일은 HTML, 자바, 태그 라이브러리 구문, 영어 주석, Javadoc, XML, javascript 등을 포함한다..
각별한 노력을 기울여 소스 파일에서 언어 수와 범위를 최대한 줄이도록 애써야 한다.

G2: 당연한 동작을 구현하지 않는다

함수나 클래스는 다른 프로그래머가 당연하게 여길만한 동작과 기능을 제공해야 한다.
그렇지 않으면 코드를 읽거나 사용하는 사람이 더 이상 함수 이름만으로 함수 기능을 직관적으로 예상하기 어렵다.
저자를 신뢰하지 못하므로 코드를 일일이 살펴야 한다.

G3: 경계를 올바로 처리하지 않는다

로직에서 경계 부분을 항상 조심하고 신경써야한다.
모든 경계 조건을 찾아내고, 모든 경계 조건을 테스트하는 테스트 케이스를 작성하라.

G4: 안전 절차 무시

안전 절차를 무시하면 안된다.
실패하는 테스트 케이스를 일단 제껴두고 나중으로 미루는 태도는 신용카드가 공짜 돈이라는 생각만큼 위험하다.

G5: 중복

코드에서 중복을 발견할 때마다 추상화할 기회로 간주한다.
좀 더 미묘한 유형은 여러 모듈에서 일련의 switch/case 나 if/else 문으로 똑같은 조건을 거듭확인하는 중복이다. 이런 중복은 다형성(polymorphism)으로 대체해야 한다.
더더욱 미묘한 유형은 알고리즘이 유사하나 코드가 서로 다른 중복이다. TEMPLATE METHOD 패턴이나 STRATEGY 패턴으로 중복을 제거한다.
사실 최근 15년 동안 나온 디자인 패턴은 대다수가 중복을 제거하는 잘 알려진 방법에 불과하다.
어디서든 중복을 발견하면 없애라.

G6: 추상화 수준이 올바르지 못하다

추상화는 저차원 상세 개념에서 고차원 일반 개념을 분리한다.
세부 구현과 관련한 상수, 변수, 유틸리티 함수는 기초 클래스에 넣으면 안된다. 기초 클래스는 구현 정보에 무지해야 마땅하다.
소스 파일, 컴포넌트, 모듈도 마찬가지다. 우수한 소프트웨어 설계자는 개념을 다양한 차원으로 분리해 다른 컨테이너에 넣는다.
고차원 개념과 저차원 개념을 섞어서는 안된다.

public interface Stack {
  Object pop() throws EmptyException;
  void push(Object o) throws FullException;
  double percentFull();
  class EmptyException extends Exception {}
  class FullException extends Exception {}
}

percentFull 함수는 추상화 수준이 올바르지 못하다. Stack을 구현하는 방법은 다양하다. 어떤 구현은 ‘꽉 찬 정도’라는 개념이 타당하지만 어떤 구현은 알아낼 방법이 전혀 없다. 그러므로 함수는 BoundedStack 과 같은 파생 인터페이스에 넣어야 마땅하다.
크기가 무한한 스택은 0을 반환하면 되지 않나? 라고 물을지도 모른다. 하지만 진정으로 무한한 스택은 존재하지 않는다. 다음 코드는 스택 크기를 확인했다는 이유만으로 OutOfMemoryException 예외가 절대 발생하지 않으리라 장담하지 못한다.

stack.percentFull() < 50.0;

G7: 기초 클래스가 파생 클래스에 의존한다

기초 클래스는 파생 클래스를 아예 몰라야 한다.
물론 예외는 있다. 간혹 파생 클래스의 개수가 확실히 고정되었따면 기초 클래스에 파생 클래스를 선택하는 코드가 들어간다.
기초 클래스와 파생 클래스를 다른 JAR 파일로 배포하면, 그리고 기초 JAR 파일이 파생 JAR 파일을 전혀 모른다면, 독립적인 개별 컴포넌트 단위로 시스템을 배치할 수 있다. 그렇게 되면변경이 시스템에 미치는 영향이 아주 작아지므로 현장에서 시스템을 유지보수하기 한결 수월하게 된다.

G8: 과도한 정보

잘 정의된 모듈은 인터페이스가 아주 작다. 작은 인터페이스로도 많은 동작이 가능하다.
잘 정의된 인터페이스는 많은 함수를 제공하지 않는다. 그래서 결합도(coupling)가 낮다. 부실하게 정의된 인터페이스는 반드시 호출해야 하는 온갖 함수를 제공한다.그래서 결합도가 높다.
우수한 소프트웨어 개발자는 클래스나 모듈 인터페이스에 노출할 함수를 제한할줄 알아야 한다.
클래스가 제공하는 메서드 수는 작을수록 좋다. 함수가 아는 변수 수도 작을수록 좋다. 클래스에 들어 있는 인스턴스 변수 수도 작을수록 좋다.
자료를 숨겨라. 유틸리티 함수를 숨겨라. 상수와 임시 변수를 숨겨라. 메서드나 인스턴스 변수가 넘쳐나는 클래스는 피하라. 하위 클래스에서 필요하다는 이유로 protected 변수나 함수를 마구 생성하지 마라. 인터페이스를 매우 작게 그리고 매우 깐깐하게 만들어라. 정보를 제한해 결합도를 낮춰라.

G9: 죽은 코드

실행되지 않는 코드를 가리킨다.
- ex. 불가능한 조건을 확인하는 if문, throw문이 없는 try문에서의 catch 블록, 아무도 호출하지 않는 유틸리티 함수와 switch/case 문에서 불가능한 case 조건
죽은 코드는 시간이 지나면 악취를 풍기기 시작한다.
죽은지 오래될수록 악취는 강해진다. 죽은 코드는 설계가 변해도 제대로 수정되지 않기 때문이다. 컴파일은 되지만 새로운 규칙이나 표기법을 따르지 않는다.
적절한 장례식을 치뤄주라. 시스템에서 제거하라.

G10: 수직 분리

변수와 함수는 사용되는 위치에 가깝게 정의한다.
지역 변수는 처음으로 사용하기 직전에 선언하며 수직으로 가까운 곳에 위치해야 한다.
비공개 함수는 처음으로 호출한 직후에 정의한다. 비공개 함수는 전체 클래스 범위에 속하지만 그래도 정의하는 위치와 호출하는 위치를 가깝게 유지한다.
비공개 함수는 처음으로 호출되는 위치를 찾은후 조금 아래로 내려가면 쉽게 눈에 띄어야 한다.

G11: 일관성 부족

어떤 개념을 특정 방식으로 구현했다면 유사 개념도 같은 방식으로 구현한다.
한 함수에서 response 라는 변수에 HttpServletResponse 인스턴스를 저장했다면 다른 함수에서도 일관성 있게 동일한 변수명을 사용한다.
한 메서드를 processVerificationRequest 라 명명했다면 (유사한 요청을 처리하는) 다른 메서드도 (processDeletionRequest처럼) 유사한 이름을 사용한다.
이처럼 간단한 일관성만으로도 코드를 읽고 수정하기 대단히 쉬워진다.

G12: 잡동사니

비어있는 기본생성자, 미사용 변수, 미사용 함수, 정보를 제공하지 못하는 주석은 모두 코드만 복잡하게 만들 뿐이므로 제거해야 마땅하다.
소스 파일은 언제나 깔끔하게 정리하라! 잡동사니를 없애라!

G13: 인위적 결합

서로 무관한 개념을 인위적으로 결합하지 않는다.
예를 들어, 일반적인 enum 은 특정 클래스에 속할 이유가 없다. enum이 클래스에 속한다면 enum을 사용하는 코드가 특정 클래스를 알아야만 한다. 범용 static 함수도 마찬가지로 특정 클래스에 속할 이유가 없다.
뚜렷한 목적 없이 변수, 상수, 함수를 당장 편한 위치(물론 잘못된 위치)에 넣어버린 결과다. 게으르고 부주의한 행동이고 변수, 상수, 함수를 선언시엔 시간을 들여 올바른 위치를 고민한다.

G14: 기능 욕심

마틴 파울러가 말하는 코드 냄새중 하나다.
클래스 메서드는 자기 클래스의 변수와 함수에 관심을 가져야지 다른 클래스의 변수와 함수에 관심을 가져선 안된다.
메서드가 다른 객체의 참조자와 변경자를 사용해 그 객체 내용을 조작한다면 메서드가 그 객체 클래스의 범위를 욕심내는 탓이다.
자신이 그 클래스에 속해 그 클래스 변수를 직접 조작하고 싶다는 뜻이다.
아래 코드에서 calculateWeeklyPay 메서드는 HourlyEmployee 클래스의 범위를 욕심낸다. calculateWeeklyPay 메서드는 HourlyEmployee 객체에서 온갖 정보를 가져온다.

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);
    }
}

기능 욕심은 한 클래스의 속사정을 다른 클래스에 노출하므로, 별다른 문제가 없다면 제거하는 편이 좋다.
하지만 때로는 어쩔 수 없는 경우도 생긴다.

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 메서드는 HourlyEmployee 클래스를 욕심낸다. 하지만 그렇다고 HourlyEmployee 클래스가 보고서 형식을 알 필요는 없다.(역할과 책임 관점에서)
함수를 HourlyEmployee 클래스로 옮기면 객체 지향 설계의 여러 원칙을 위반한다.
HourlyEmployee가 보고서 형식과 결합되므로 보고서 형식이 바뀌면 해당 클래스도 바뀐다..

G15: 선택자 인수

선택자 인수(boolean)는 큰 함수를 작은 함수 여럿으로 쪼개지 않으려는 게으름의 소산이다. (p.379 예제 참고)
enum, int 등 함수 동작을 제어하려는 인수는 하나 같이 바람직하지 않고 일반적으로 인수를 넘겨 동작을 선택하는 대신 새로운 함수를 만드는 편이 좋다.

G16: 모호한 의도

코드를 짤 때는 의도를 최대한 분명히 밝힌다.
- 행을 바꾸지 않고 표현한 수식, 헝가리식 표기법, 매직 번호 등은 모두 저자의 의도를 흐린다.

G17: 잘못 지운 책임

코드 설계 시 코드 배치 위치를 결정하는 것은 중요하다. 여기서 배치 위치는 독자가 여기있겠구나 싶은 곳에 배치하는것이 좋다.
때로는 독자에게 직관적인 위치가 아니라 개발자에게 편한 곳에 배치하기도 한다. 이때 결정을 내리는 기준 중 한가지는 함수의 이름을 살펴보는 것이다.
- 근무 시간 총계를 보고서로 출력하는 함수가 필요하다고 했을때, 보고서 모듈의 getTotalHours 함수와 근무시간을 입력받는 saveTimeCard 함수 중 어느쪽에서 계산하는 것이 맞을까? 전자다.
성능을 높이고자 근무시간을 입력 받는 곳에서 총계를 계산한다고 하면 computeRunningTotalOfHours 이라는 함수를 내부에 넣어주는것이 좋다.

G18: 부적절한 static 함수

Math.max(double a, double b)는 좋은 static 메서드다. 특정 인스턴스와 관려된 기능이 아니기에 new Math().max(a, b)라 하면 오히려 우습다. 결정적으로 재정의할 가능성은 전혀 없다.
그런데 간혹 우리는 static 으로 정의하면 안되는 함수를 static 으로 정의한다.
아래와 같이 수당을 계산하는 함수인데 재정의할 가능성이 존재하기에 적절치 않다. (수당 계산 알고리즘은 여러개 일수있으니, 일반 수당 계산과 초과 근무 수당 계산)

HourlyPayCalculator.calculatePay(employee, overtimeRate);

일반적으로 static 함수보다 인스턴스 함수가 더 좋다. 조금이라도 의심스럽다면 인스턴스 함수로 정의한다. 반드시 static 함수로 정의해야겠다면 재정의할 가능성은 없는지 꼼꼼히 따져본다.

G19: 서술적 변수

켄트 벡이 Smalltalk Best Practice Patterns 라는 훌륭한 책과 Implementation Patterns 라는 훌륭한 책에서 지적하는 문제다.
프로그램의 가독성을 높이는 가장 효과적인 방법 중 하나가 계산을 여러 단계로 나누고 중간 값으로 서술적인 변수 이름을 사용하는 방법이다.

Matcher match = headerPattern.matcher(line);
if(match.find())
{
  String key = match.group(1);
  String value = match.group(2);
  headers.put(key.toLowerCase(), value);
}

위 코드에서 서술적 변수 이름을 사용했기 때문에 첫번째로 일치하는 그룹이 key에 해당되며 두번째 그룹은 value라는 부분이 명백하게 드러난다.
서술적 변수명은 많이 써도 괜찮고, 일반적으로 많을수록 더 좋다.

G20: 이름과 기능이 일치하는 함수

Date newDate = date.add(5);

위 함수를 보면 date.add가 의미하는 바가 날짜인지, 시간인지, 주인지 모호하다. 5일을 더해 date 인스턴스를 변경하는 함수라면 addDaysTo 혹인 increaseByDays라는 이름이 좋다.
이름만으로 분명하지 않기에 구현을 살피거나 문서를 뒤적여야 한다면 더 좋은 이름으로 바꾸거나 아니면 더 좋은 이름을 붙이기 쉽게 기능을 정리해야 한다.

G21: 알고리즘을 이해하라

코드가 단순히 돌아가여 테스트 코드를 통과한다고 끝나면 안된다.
함수가 동작하는 방식을 완전히 이해하는지 확인해야 한다.
이를 위해선 기능이 뻔히 보일 정도로 함수를 깔끔하고 명확하게 재구성하는 방법이 최고다.

G22: 논리적 의존성은 물리적으로 드러내라

한 모듈이 다른 모듈에 의존한다면 물리적 의존성도 있어야 한다. 물리적으로 의존하면 의존하는 정보를 명시적으로 요청하는 편이 좋다.
근무 시간 보고서를 가공되지 않은 상태로 출력하는 함수를 만든다고 할때 HourlyReporter 클래스는 정보를 모아 HourlyReportFormatter 클래스에 넘기고 HourlyReportFormatter 는 넘어온 정보를 출력한다.

public class HourlyReporter {
  private HourlyReportFormatter formatter;
  private List<LineItem> page;
  private final int PAGE_SIZE = 55;

  public HourlyReporter(HourlyReportFormatter formatter) {
    this.formatter = formatter;
    page = new ArrayList<LineItem>();
  }

  public void generateReporter(List<HourlyEmployee> employees) {
    for (HourlyEmployee e : employees) {
      addLineItemToPage(e);
      if (page.size() == PAGE_SIZE) {
        printAndClearItemList();
      }
    }
    if (page.size() == 0)
      printAndClearItemList();
  }

  private void printAndClearItemList() {
    formatter.format(page);
    page.clear();
  }

  private void addLineItemToPage(HourlyEmployee e) {
    LineItem item = new LineItem();
    item.name = e.getName();
    item.hours = e.getTenthsWorked() / 10;
    item.tenths = e.getTenthsWorked() % 10;
    page.add(item);
  }

  private class LineItem {
    public String name;
    public int hours;
    public int tenths;
  }
}

해당 코드에서 PAGE_SIZE라는 상수를 통해 논리적 의존성을 가진다.
해당 상수는 HourlyReporter 클래스는 HourlyReportFormatter 클래스가 페이지 크기를 알 것이라고 가정한다. 이러한 가정을 논리적 의존성이라고 하는데 이때 HourlyReportFormatter 가 페이지 크기를 처리하지 못한다면 오류가 발생하게 된다.
이를 해결하고자 HourlyReportFormatter 에 getMaxPageSize() 메서드를 추가하게 되면 위와 같은 논리적 의존성이 물리적 의존성으로 변환된다. 그래서 상수 대신 함수를 이용하여 논리적 의존성으로 인한 문제 대신 물리적 의존성 갖도록 변환해준다.

G23: If/Else 혹은 Switch/Case 문보다 다형성을 사용하라

대다수 개발자가 switch 문을 사용하는 이유는 올바르기보다는 손쉬운 선택이기 때문이다. 그러므로 switch를 선택하기 전에 다형성을 먼저 고려하라는 의미다.
유형보다 함수가 더 쉽게 변하는 경우는 극히 드물다. 그러므로 모든 switch 문을 의심해야 한다.
선택 유형 하나에는 switch 문을 한번만 사용하고, 같은 선택을 수행하는 다른 코드에서는 다형성 객체를 생성해 switch 문을 대신한다.

G24: 표준 표기법을 따르라

인스턴스 변수 선언 위치, 이름을 정하는 방법, 괄호를 넣는 위치 등에 대한 구현 표준을 따라야 한다.
이는 코드 자체로 충분해야 하며 별도의 문서로 설명할 필요가 없어야 하며 이렇게 정한 표준은 모든 팀원이 따라야 한다.
저자가 따르는 표기법이 궁금하다면 512쪽 목록 B-7에서 목록 B-14까지 제시한 코드를 살펴본다.

G25: 매직 숫자는 명명된 상수로 교체하라

일반적으로 코드에서 숫자를 직접 사용하지 말라는 규칙이며 이는 숫자를 명명된 상수 뒤로 숨기는 것을 의미한다.
예를 들어, 86,400 이라는 숫자는 SECONDS_PER_DAY 라는 상수 뒤로 숨긴다. 쪽당 55줄을 인쇄한다면 숫자 55는 LINES_PER_PAGE 상수 뒤로 숨긴다.
하지만 어떤 상수는 이해하기 쉬우므로 코드 자체가 자명하다면, 상수 뒤로 숨길 필요가 없다.

double milesWalked = feetWalked/5280.0;
int dailyPay = hourlyRate * 8;

5280 은 마일당 피트에 대한 수치로 너무 잘 알려진 고유 숫자이다.
너무나도 잘 알려진 고유 숫자라면 주변 코드 없이 숫자만 달랑 적어놔도 독자가 금방 알아본다.

G26: 정확하라

검색 결과 중 첫 번째 결과만 유일한 결과로 간주, 부동소수점으로 통화를 표현, List로 선언할 변수를 ArrayList로 선언, 모든 변수를 protected 로 선언 하는것은 부정확한 방법이다.
코드에서 무언가를 결정할땐 정확하게 결정해야 한다. 결정을 내리는 이유와 예외를 처리할 방법을 분명히 알아야 한다.
- null을 반환할 수 있는 함수는 반드시 null 체크를 하고, 조회 결과가 하나 뿐이라 짐작한다면 하나인지 확실히 확인한다.
코드에서 모호성과 부정확은 의견차나 게으름의 결과이고 어느 쪽이든 제거해야 마땅하다.

G27: 관례보다 구조를 사용하라

설계 결정을 강제할 때는 규칙보다 관례를 사용한다. 명명 관례도 좋지만 구조 자체로 강제하면 더 좋다.
예를 들어, enum 변수가 멋진 switch/case 문보다 추상 메서드가 있는 기초 클래스가 더 좋다.
switch/case 문을 매번 똑같이 구현하게 강제하기는 어렵지만, 추상 메서드가 정의되어 있으면 해당 추상 클래스를 상속받는 파생 클래스는 해당 메서드를 모두 구현하지 않으면 안 되기 때문이다.

G28: 조건을 캡슐화하라

= 부울 논리는 이해하기 어렵기에 조건의 의도를 분명히 밝히는 함수로 표현하라

if (shouldBeDeleted(timer)) // good

if (timer.haseExpired() && !timer.isRecurrent()) // bad

G29: 부정 조건은 피하라

부정 조건은 긍정 조건보다 이해하기 어렵다. 가능하면 긍정 조건을 표현한다.

if (buffer.shouldCompact()) // good

if (!buffer.shouldNotCompact()) // bad

G30: 함수는 한 가지만 해야 한다.

한 함수 안에 여러 단락을 이어, 일련의 작업을 수행하고픈 유혹에 빠지는데 이런 함수는 한 가지만 수행하는 함수가 아니다.
함수는 한 가지 기능만을 해야하기에 좀 더 작은 함수 여럿으로 나눠야 마땅하다.

public void pay(){
	for (Employee e : employees) {
    	if (e.isPaypay()) {
        	Money pay = e.calculatePay();
            e.deliverPay(pay);
        }
    }
}

위 코드는 세 가지 임무를 수행한다.
- 1)직원 목록을 루프 돌기
- 2)각 직원의 월급일을 확인
- 3)해당 직원에게 월급 지급
위 함수는 다음 함수 셋으로 나누는 편이 좋다.

public void pay(){
	for (Employee e : employees) 
    	payIfNecessary(e); // 만약 필요하다면 지불하라
}

private void payIfNecessary(Employee e) {
	if(e.isPayday()){
    	calculateAndDeliverPay(e); // 월급을 계산하고 전달하라
    }
}

private void calculateAndDeliverPay(Employee e) {
	Money pay = e.calculatePay();
    e.deliverPay(pay);
}

위에서 각 함수는 한 가지 임무만 수행한다. (자세한 내용은 3장 “한 가지만 해라”를 참조한다.)

G31: 숨겨진 시간적인 결합

때로는 시간적인 결합이 필요하다. 하지만 시간적인 결합을 숨겨서는 안 된다.
함수 인수를 적절히 배치해 함수가 호출되는 순서를 명백히 드러내야 한다.

public class MoogDiver {
  Gradient gradient;
  List<Spline> splines;

  public void dive(String reason) {
    saturateGradient();
    reticulateSplines();
    diveForMoog(reason);
  }
  ...
}

세 함수가 순서대로 실행되는것이 목적이지만, 프로그래머가 reticulateSplines 를 먼저 호출하고 saturateGradient 을 호출하는 경우 발생하는 오류를 막을 수 없다.
따라서 실행 순서를 명확하게 표현할 수 있도록 아래와 같이 수정한다.

public class MoogDiver {
  Gradient gradient;
  List<Spline> splines;

  public void dive(String reason) {
    Gradient gradient = saturateGradient();
    List<Spline> splines = reticulateSplines(gradient);
    diveForMoog(splines, reason);
  }
  ...
}

위 코드는 일종의 연결 소자를 생성하여 시간적 결합을 노출한다. 각 함수가 내놓는 결과는 다음 함수에 필요하므로 순서를 바꿔 호출할수가 없게된다.
함수가 복잡해질수도 있다. 하지만 의도적으로 추가한 구문적인 복잡성이 원래 있던 시간적인 복잡성을 드러낸셈이다.
해당 클래스의 private 메서드에 필요한 변수일지도 몰

G32: 일관성을 유지하라

코드 구조를 잡을 때는 이유를 고민하고 그 이유를 코드 구조로 명백히 표현하라.
구조에 일관성이 없어 보인다면 남들이 맘대로 바꿔도 좋다고 생각하고, 시스템 전반에 걸쳐 구조가 일관성 있다면 남들도 일관성을 따르고 보존한다.

G33: 경계 조건을 캡슐화하라

경계 조건은 여기저기 처리하지 않고 한곳에서 별도 처리한다.
코드 여기저기에 +1 이나 -1을 흩어놓지 않는다.

if (level + 1 < tags.length) {
  parts = new Parse(body, tags, level + 1, offset + endTag;
  body = null;
}

위 코드에서 level + 1 이 두 번 나오기 때문에 변수(nextLevel)로 캡슐화하는 것이 좋다.

int nextLevel = level + 1;
if (nextLevel < tags.length) {
  parts = new Parse(body, tags, nextLevel, offset + endTag;
  body = null;
}

G34: 함수는 추상화 수준을 한 단계만 내려가야 한다

함수내 모든 문장은 추상화 수준이 동일해야 한다. 그리고 그 추상화 수준은 함수 이름이 의미하는 작업보다 한 단계만 낮아야 한다.

public String render() throws Exception {
  StringBuffer html = new StringBuffer("<hr");
  if(size > 0)
    html.append(" size=\"").append(size + 1).append("\"");
  html.append(">");

  return html.toString();
}

위 함수는 페이지를 가로질러 수평자를 만드는 HTML 태그를 생성한다. 수평자 높이는 size 변수로 지정한다.
여기서 추상화 수준은 여러개 섞여있다.
- 1)수평선에 크기가 있다.
- 2)HR 태그로 변환시 4개 이상의 연이은 ‘-‘ 기호를 감지해 HR 태그로 변환한다.

public String render() throws Exception {
  HtmlTag hr = new HtmlTag("hr");
  if (extraDashes > 0)
    hr.addAttributes("size", hrSize(extraDashes));
  return hr.html();
}

private String hrSize(int height) {
  int hrSize = height + 1;
  return String.format("%d", hrSize);
}

위와 같이 추상화 수준을 분리해야 한다.
size 변수는 추가된 대시의 개수를 저장하고, render 함수는 HR 태그만 생성한다. HTML HR 태그 문법은 HTMLTag 모듈이 처리해준다.
추상화 수준 분리는 리팩터링을 수행하는 가장 중요한 이유 중 하나고 제대로 하기에 가장 어려운 작업 중 하나이기도 하다.

G35: 설정 정보는 최상위 단계에 둬라

추상화 최상위 단계에 더야할 기본값 상수나 설정 관련 상수를 저차원 함수에 숨겨선 안된다.
대신 고차원 함수에서 저차원 함수를 호출시 인수로 넘긴다.

public static void main(String[] args) throws Exception
{
  Arguments arguments = parseCommandLine(args);
  ...
}

public class Arguments
{
  public static final String DEFAULT_PATH = ".";
  public static final String DEFAULT_ROOT = "FitNesseRoot";
  public static final int DEFAULT_PORT = 80;
  public static final int DEFAULT_VERSION_DAYS = 14;
  ...
}

디폴트 port 를 찾기 위해 저수준 함수들을 뒤지며 아래와 같은 코드를 발견하게 해선 안된다.

if (arguments.port == 0) // 기본값으로 80을 사용한다.

설정 관련 상수는 최상위 단계에 둔다. 그래야 변경도 쉽다.
설정 관련 변수는 나머지 코드에 인수로 넘긴다. 저차원 함수에 상수 값을 정의하면 안된다.

G36: 추이적 탐색을 피하라

일반적으로 한 모듈은 주변 모듈을 모를수록 좋다. 좀 더 구체적으로 A가 B를 사용하고 B가 C를 사용한다 하더라도 A가 C를 알아야 할 필요는 없다는 뜻이다.(예를 들어 a.getB().getC().doSomething();은 바람직하지 안다)
즉, 자신이 직접 사용하는 모듈만 알아야한다. 이를 디미터의 법칙(Law of Demeter)이라 부른다.
여러 모듈에서 a.getB().getC()라는 형태를 사용한다면 설계와 아키텍처를 바꿔 B와 C사이에 Q를 넣기 쉽지 않다. a.getB().getC()를 모두 찾아 a.getB().getQ().getC()로 모두 바꿔야 하니깐… 너무 많은 모듈이 아키텍처를 너무 많이 알게 되고 아키텍처는 굳어지게 된다.
즉, 원하는 메서드를 찾느라 객체 그래프를 따라 시스템을 탐색할 필요가 없어야 한다. 다시 말해 다음과 같은 간단한 코드로 충분해야 한다.

myCollaborator.doSomething();

[클린코드] Chapter17-냄새와 휴리스틱

주석

C1: 부적절한 정보

C2: 쓸모 없는 주석

C3: 중복된 주석

C4: 성의 없는 주석

C5: 주석 처리된 코드

환경

E1: 여러 단계로 빌드해야 한다.

E2: 여러 단계로 테스트해야 한다.

함수

F1: 너무 많은 인수

F2: 출력 인수

F3: 플래그 인수

F4: 죽은 함수

일반

G1: 한 소스 파일에 여러 언어를 사용한다

G2: 당연한 동작을 구현하지 않는다

G3: 경계를 올바로 처리하지 않는다

G4: 안전 절차 무시

G5: 중복

G6: 추상화 수준이 올바르지 못하다

G7: 기초 클래스가 파생 클래스에 의존한다

G8: 과도한 정보

G9: 죽은 코드

G10: 수직 분리

G11: 일관성 부족

G12: 잡동사니

G13: 인위적 결합

G14: 기능 욕심

G15: 선택자 인수

G16: 모호한 의도

G17: 잘못 지운 책임

G18: 부적절한 static 함수

G19: 서술적 변수

G20: 이름과 기능이 일치하는 함수

G21: 알고리즘을 이해하라

G22: 논리적 의존성은 물리적으로 드러내라

G23: If/Else 혹은 Switch/Case 문보다 다형성을 사용하라

G24: 표준 표기법을 따르라

G25: 매직 숫자는 명명된 상수로 교체하라

G26: 정확하라

G27: 관례보다 구조를 사용하라

G28: 조건을 캡슐화하라

G29: 부정 조건은 피하라

G30: 함수는 한 가지만 해야 한다.

G31: 숨겨진 시간적인 결합

G32: 일관성을 유지하라

G33: 경계 조건을 캡슐화하라

G34: 함수는 추상화 수준을 한 단계만 내려가야 한다

G35: 설정 정보는 최상위 단계에 둬라

G36: 추이적 탐색을 피하라

자바

J1: 긴 import 목록을 피하고 와일드카드를 사용하라

J2: 상수는 상속하지 않는다

Reference

Further Reading

[클린코드] Chapter10-클래스

[클린코드] Chapter11-시스템

[클린코드] Chapter12-창발성

Trending Tags