Clean Code 읽기(8)

17장 냄새와 휴리스틱

• “Refactoring”에서 마틴 파울러는 다양한 ‘코드 냄새’를 한다. 거기에 추가한 ‘냄새’들과 코드를 짜면서 사용하는 기교와 휴리스틱을 소개한다.
• 🙋‍♂️ 리팩토링 책은 클린코드를 다 읽은 후 다음에 읽을 책이다.

일반

1. 한 소스 파일에 여러 언어를 사용한다.

   • 오늘날 프로그래밍 환경은 한 파일 내 여러 언어를 지원한다.
   • 현실적으로 한 소스 파일에 여러 언어가 쓰일 수 밖에 없으나, 언어 수와 범위를 최대한 줄여야한다.

2. 당연한 동작을 구현하지 않는다.

   • 함수나 클래스는 다른 프로그래머가 당연하게 여길 만한 동작과 기능을 제공해야 한다.
   • 당연한 동작을 구현하지 않으면 코드를 읽는 사람이 함수 이름만으로 기능을 직관적으로 예상할 수 없다.

3. 경계를 올바로 처리하지 않는다.

   • 흔히 개발자들은 머릿속에서 코드를 돌려보고 끝낸다. 직관에 의존하고 모든 경계를 증명하려 애쓰지 않는다.

   • 모든 경계 조건, 모든 예외에 대한 테스트 케이스를 작성하고 테스트해라.

   • 🙋‍♂️ 경계 얘기가 예외처리랑 비슷한 얘기인거같은데 맞나? 보통 가장 정상적인 케이스만 생각하는데 다른 케이스도 고려하고 테스트 케이스를 작성하라는 얘기같다.

4. 안전 절차 무시

   • 컴파일러 경고를 꺼버리면 빌드가 쉬워질지 모르지만 끝없는 디버깅에 시달리는 끔찍한 상황이 발생할지도 모른다.
   • 실패하는 테스트 케이스를 만들지 않고 나중으로 미루는 태도는 신용카드가 공짜 돈이라고 생각하는것과 같다.

5. 중복

   • 가장 중요한 규칙이다.
   • 코드에서 중복을 발견할 때마다 추상화할 기회로 간주해라. 하위 루틴이나 다른 클래스로 분리해라. 추상화 수준이 높아질 수록 구현이 빨라지고 오류가 적어진다.
   • switch/case나 if/else 문으로 조건을 거듭 확인하는 중복은 다형성으로 대체해야한다.
   • 알고리즘이 유사하지만 코드가 서로 다른 중복도 제거해야한다. Template method 패턴이나 Strategy 패턴으로 중복을 제거한다.
   • 🙋‍♂️ 추상화할 기회로 간주하라는 얘기가 처음에는 이해가 안됐다. 추상화??? –> 이런 케이스들은 직접 코드를 보는게 중요한것 같다. “자바 웹 프로그래밍 Next” 책을 보고 실습을 따라하면서 함수로 추출, 클래스로 추출하는 기교를 보며 좀 알 수 있었다.
   • 🙋‍♂️ 다형성을 사용하라는것도 처음에는 이해도 안될 뿐더러 왜 저렇게 까지 해야하나 생각이 들었다. 이것도 역시 “자바 웹 프로그래밍 Next” 책에서의 실습을 보고 이해를 할 수 있었다. 근데 아직까지도 저렇게까지 작게 나눠야해? 라는 생각이 조금은 들었다.
   • 🙋‍♂️ Template method, Strategy 패턴이 무슨말인지 모르겠다.. 디자인 패턴에 대해 알아볼 필요가 있다.

6. 추상화 수준이 올바르지 못하다.

   • 추상화로 개념을 분리할 때는 철저해야 한다. 모든 저차원 개념은 파생 클래스에, 모든 고차원 개념은 기초 클래스에 넣는다.
   • 예를 들어, 세부 구현과 관련된 상수, 변수, 유틸리티 함수 등은 기초 클래스가 아닌 파생 클래스에 넣어야한다.
   • 고차원 개념과 저차원 개념을 한 곳에 섞어서는 안된다.

7. 기초 클래스가 파생 클래스에 의존한다.

   • 기초 클래스가 파생 클래스를 사용한다면 문제가 있다. 일반적으로 기초 클래스는 파생 클래스를 몰라야 한다.

8. 과도한 정보

   • 잘 정의된 모듈은 작은 인터페이스로도 많은 동작이 가능하며 많은 함수를 제공하지 않는다. 그래서 결합도가 낮다.
   • 부실하게 정의된 모듈은 간단한 동작에도 온갖 인터페이스가 필요하며 반드시 호출해야 하는 온갖 함수를 제공한다. 그래서 결합도가 높다.
   • 자료, 유틸함수, 상수와 임시변수를 숨겨라. 메서드나 인스턴스 변수가 넘쳐나는 클래스는 피해라. 인터페이스를 매우 작게 만들어라. 정보를 제한하여 결합도를 낮춰라

9. 죽은 코드

   • 불가능한 조건을 확인하는 if문이나 throw문이 없는 try catch문 등 실행되지 않는 코드를 죽은 코드라고 한다. 바로 제거해야 한다.

10. 수직 분리

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

11. 일관성 부족

    • 표기법을 신중하게 선택하고 따라야한다.
    • 한 함수에서 HttpServletResponse 객체이름을 response라고 정의했다면 다른 함수에서도 HttpServletResponse 객체 이름을 response라고 정의해라.

12. 잡동사니

    • 비어 있는 기본 생성자는 필요가 없다. 제거해라

13. 인위적 결합

    • 서로 무관한 개념을 인위적으로 결합하지 않는다.
    • 일반적인 enum은 특정 클래스에 속할 이유가 없다. 범용 static 함수 또한 특정 클래스에 속할 이유가 없다.
    • 직접적인 상호작용이 없는 두 모듈사이에서 인위적인 결합이 일어난다. 함수, 상수, 변수를 선언할 때는 시간을 들여 올바른 위치에 넣는다.
    • 🙋‍♂️ 처음 딱 보고 뭔소리야 했는데, enum이나 범용 static메서드도 따로 추출하라는 얘기인것 같다. 어디에서나 쓰는 enum, static 메서드를 쓰기 위해서 특정 클래스를 계속 호출할 필요가 없으니까

14. 기능 욕심

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

      public class HourlyPayCalculator {
        public Money calculateWeeklyPay(HourlyEmployee e) {
          int tenthRate = e.getTenthRate().getPennies();
          int tenthsWorked = e.getTenthWorked();
          int straightTime = Math.min(400, tenthsWorked);
          ...
        }
      }
      

      • calculateWeeklyPay 메서드를 보면 HourlyEmployee 객체에서 get메서드를 이용해 정보들을 가져온다. 이는 다른 클래스의 범위를 욕심내는 것이다.

15. 선택자 인수

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

      public cint calculateWeeklypay(boolean overtime) {
        int tenthRate = getTenthRate();
        int tenthsWorked = getTenthsWorked();
        int straightTime = Math.min(400, tenthsWorked);
        ...
        double overtimeRate = overtime ? 1.5 : 1.0 * tenthRate;
        ...
      }
      

      • overtime이 true라면 1.5배로 지급하고 false라면 그대로 지급한다.

    • 코드를 읽는사람은 calculateWeeklyPay(false)라는 코드를 발견할 대마다 의미를 떠올리느라 골치를 앓는다.

      public int straightPay() {
        ...
      }
            
      public int overTimePay() {
        ...
      }
            
      public int overTimeBonus(int overTimeTenths) {
        ...
      }
      

      • 위처럼 3개의 메서드로 쪼갤 수 있다.

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

16. 모호한 의도

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

17. 잘못 지운 책임

    • 코드를 배치하는 위치는 매우 중요하다.
    • 때로는 개발자가 독자에게 직관적인 위치가 아닌 개발자가 편한 위치에 배치할 수 있다.

18. 부적절한 static 함수

    • 일반적으로 staic 함수보다 인스턴스 함수가 더 좋다.
    • 특정 객체와 관련이 없고 모든 정보를 인수에서 가져오며 함수를 재정의할 가능성이 없다면 static 함수로 만들어도 좋다.
    • 조금이라도 의심스럽다면 인스턴스 함수로 정의한다.

19. 서술적 변수

    • 서술적인 이름의 변수는 많이 써도 좋다. 게산을 몇 단계로 나누고 중간값으로 좋은 변수 이름만 붙여도 해독하기 쉬워진다.

20. 이름과 기능이 일치하는 함수

    • 코드만으로 함수가 어떤일을 명확히 알 수 없는 이름이 있다. 좀 더 명확하게 이름을 붙여라

21. 알고리즘을 이해해라

    • 실제 알고리즘을 충분히 이해하지 않는다면 if문과 플래그가 많아진다.
    • 테스트 케이스를 통과하고, 작성자가 알고리즘이 올바르다는 사실을 알아야한다.
    • 알고리즘이 올바르다는 사실을 이해하려면 기능이 뻔히 보일 정도로 함수를 깔끔하고 명확하게 재구성하는 방법이 가장 좋다.

22. 논리적 의존성은 물리적으로 드러내라

    • 논리적인 의존성만으로는 부족하다. 의존하는 모든 정보를 명시적으로 요청하는 편이 좋다.

      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 generateReport(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();
          ...
        }
        ...
      }
      

      • 위 코드는 논리적인 의존성이 존재한다. PAGE_SIZE라는 상수이다. HourlyReporter 클래스가 페이지 크기를 알 필요가 없다. HourlyReportFormatter가 책임질 정보이다.
      • HourlyReporter 클래스는 HourlyReportFormatter가 페이지 크기를 알 것이라고 가정한다. 이 가정이 논리적 의존성이다.
      • HourlyReportFormatter에 getMaxPageSize()라는 메서드를 추가하면 논리적 의존성이 물리적 의존성으로 변한다.

    • 🙋‍♂️읽고 따라쓰긴했는데 무슨 의미인지 명확히 이해하기 힘들다..

23. if/else 혹은 Switch/case 보다 다형성을 사용하라.
    • switch 문을 사용하는 이유는 올바른 선택이라기보다 쉬운 선택이기 때문이다.
    • switch문이 함수보다 더 쉽게 변하는 경우는 극히 드물다. 모든 switch문을 의심해라
24. 표준 표기법을 따르라
    • 업계 표준에 기반한 구현 표준을 따라야 한다. 팀이 정한 표준은 팀원들 모두가 따라야 한다. 모두가 동의한 위치에 넣는다는 사실이 중요하다. 이 사실을 이해할 정도로 팀원들이 성숙해야한다.
    • 🙋‍♂️ 책에서 이렇게 까지 얘기를 다룬다는것은 팀이 정한 표준을 따르지않는 개인들이 많다는 얘기가 아닐까.. 아직 협업을 해보지않아서 모르겠지만 중요하다고 느껴진다
25. 매직 숫자는 명명된 상수로 교체하라
    • PI를 나타내는 3.14….나 하루를 초로 나타내는 86,400 등 매직넘버를 상수로 치환해라
    • 🙋‍♂️ 다른책에서도 본 내용이다. 처음에는 이렇게 해야하나? 생각했는데 요즘은 오버해서 치환하는 경우도있다. ㅋㅋㅋㅋ
26. 정확하라
    • 코드에서 뭔가를 결정할 때는 정확히 결정한다. 결정의 이유와 예외처리 방법을 분명히 알아야 한다. null을 반환할지도 모른다? null 체크를 반드시한다.
27. 관례보다 구조를 사용하라
    • 명명 관례(자바에서 camel case사용하는 등)도 좋지만 구조자체로 강제하면 더 좋다. switch/case문보다 추상 메서드가 있는 기초클래스가 더 좋다.
28. 조건을 캡슐화하라
    • 조건의 의도를 분명히 밝히는 함수로 표현한다.
      • if(timer.hasExpired() && ..)같은 코드보다 if(shouldBeDeleted(timer))처럼 함수로 분명하게 나타내는것이 더 좋다.
29. 부정 조건은 피해라
    • if(buffer.shouldCompact)코드가 if(!buffer.shouldNotCompact())코드보다 좋다.

30. 함수는 한가지만 해야한다.

    • 함수는 한가지만 수행해야한다. 여러 일을 처리한다면 더 작은 함수 여럿으로 나누는것이 마땅하다.
    • 🙋‍♂️ 함수 파트에서도 본 내용이며, 코드워즈 문제풀때 최대한 적용시키려 노력한다.

31. 숨겨진 시간적인 결합

    • 시간적인 결합을 숨겨서는 안된다. 함수 인수를 적절히 배치해 함수가 호출되는 순서를 명백히 드러내야한다.

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

      • 위 코드는 시간적인 결합을 강제하지 않는다. 실수로 메서드 호출순서를 바꾸게 되면 오류가 발생할 수 있다.

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

      • 함수가 복잡해졌지만 의도적으로 시잔적인 복잡성을 드러낸 셈이다.

    • 각 함수가 내놓은 결과물은 다음 함수에 필요하게 만들어 시간적 복잡성을 드러낸다.

    • 🙋‍♂️이런 걸 보면 이렇게 까지 해야하나?? 라고 생각도 들긴하는데, 확실히 호출순서의 의도는 잘 드러나있다.

32. 일관성을 유지하라

    • 코드 구조를 잡을 때 이유를 고민하고 그 이유를 코드 구조로 명백히 표현해라.
    • 일관성없다면 남들이 마음대로 바꿔도 된다고 생각한다. 일관성이 보존되면 남들도 보존하려 한다.
    • 🙋‍♂️ 12에서 얘기한 “일관성 부족”의 연장설명인것같다.

33. 경계 조건을 캡슐화하라

    • 경계 조건은 빼먹거나 놓치기 쉽다. 한 곳에서 별도로 처리하는것이 좋다. 코드 여기저기에 처리하지 않는다.
    • 🙋‍♂️ if(object == null) return false.. 같은 예외처리들은 메서드 내 이곳저곳에서 검사하는데 이것을 한곳으로 모으는 방법이 뭐가 있을까?

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

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

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

      • 수평선에 크기가 있다는 개념, HR 태그 자체의 문법 두가지의 추상화 수준이 섞여있다.

      public String render() throws Exception {
        HtmlTag hr = new HtmlTag("hr");
        if (extraDashes > 0)
          hr.addAttribure("size", hrSize(extraDashes));
         return hr.html();
      }
            
      public String hrSize(int height) {
        int hrSize = height + 1;
        return String.format("%d", hrSize);
      }
      

      • render함수는 hr태그만 생성한다. hr 태그 문법은 상관하지 않는다.

    • 추상화 분리는 리팩터링을 하는 가장 중요한 이유 중 하나이며 제대로 하기에 어려운 작업중 하나이다.

    • 🙋‍♂️ 얘기한것처럼 어렵다. 추상화 수준을 분리하는것도 어려운데, 추상화 수준을 구분하는것이 더 어렵다. 위의 코드를 보고 추상화 수준이 섞여있는것을 판단하지 못했다 ㅜ

35. 설정 정보는 최상위 단계에 둬라

    • 추상화 최상위 단계에 둬야 할 기본값 상수나 설정 관련 상수를 저차원 함수에 숨겨서는 안된다. 최상위 단계에 둬야 변경하기 쉽다.

36. 추이적 탐색을 피하라

    • 일반적으로 한 모듈은 주변 모듈을 모를수록 좋다. a.getB().getC().do()같은 구조는 바람직하지 않다.
    • 요지는 자신이 직접 사용하는 모듈만 알아야 한다. 연이어 모듈을 따라가며 시스템 전체를 휘저을 필요 없다.

🙋‍♂️ 코드워즈 문제들을 TDD로 푸는 연습을 계속 하고있다. 리팩토링하는 단계에서 더 좋은방법은 없을까? 프로그래밍 언어를 막 배운 사람도 알아볼 수 있게 코드를 짤 수 있을까? 고민을 한다. 간단한 메서드를 구현하는 문제들이기 때문에, “좋은 이름 붙이기”, “메서드 크기는 작게”, “메서드 인수 사용” 등 작은 모듈은 클린 코드를 적용하는데 어느정도 문제가 없다.

🙋‍♂️ 어려운것들은 if/else문에서 다형성을 이용한다거나 추상화의 개념이다. 이런것들은 “자바 웹 프로그래밍 Next” 책을 보면서 어느정도 감은 오는데 다른곳에 어떻게 적용할 수 있을까? 생각을 해보면 잘 모르겠다. 또한 디자인패턴이나 객체지향에 대해서도 무지함을 느낀다. 자바를 객체지향 언어라고 단지 배우기만 했지, 객체 지향 디자인패턴을 제대로 사용하지 않고 있다. 더 깊게 공부할 필요를 느낀다.