Clean Code 3장

package chap3;
​
import chap3.etc.PageCrawlerImpl;
import chap3.etc.PageData;
import chap3.etc.SuiteResponder;
import chap3.etc.WikiPage;
import chap3.etc.WikiPagePath;
​
public class HtmlUtil {
​
    public static String testableHtml(PageData pageData, boolean includeSuiteSetup) throws Exception {
        WikiPage wikiPage = pageData.getWikiPage();
        StringBuffer buffer = new StringBuffer();
        if (pageData.hasAttribute("Test")) {
            if (includeSuiteSetup) {
                WikiPage suiteSetup = PageCrawlerImpl.getInheritedPage(SuiteResponder.SUITE_SETUP_NAME, wikiPage);
​
                if (suiteSetup != null) {
                    WikiPagePath pagePath = suiteSetup.getPageCrawler().getFullPath(suiteSetup);
                    String pagePathName = PathParser.render(pagePath);
                    buffer.append("!include -setup .")
                          .append(pagePathName)
                          .append("\n");
                }
            }
            WikiPage setup = PageCrawlerImpl.getInheritedPage("SetUp" ,wikiPage);
            if(setup != null) {
                WikiPagePath setupPath = wikiPage.getPageCrawler().getFullPath(setup);
                String setupPathName = PathParser.render(setupPath);
                buffer.append("!include -setup .")
                      .append(setupPathName)
                      .append("\n");
            }
        }
        buffer.append(pageData.getContent());
        if (pageData.hasAttribute("Test")) {
            WikiPage tearDown = PageCrawlerImpl.getInheritedPage("TearDown", wikiPage);
            if(tearDown != null) {
                WikiPagePath tearDownPath = wikiPage.getPageCrawler().getFullPath(tearDown);
                String tearDownPathName = PathParser.render(tearDownPath);
                buffer.append("\n")
                      .append("!include -teardown .")
                      .append(tearDownPathName)
                      .append("\n");
            }
​
            if (includeSuiteSetup) {
                WikiPage suiteTearDown = PageCrawlerImpl.getInheritedPage(SuiteResponder.SUITE_TEARDOWN_NAME, wikiPage);
                if (suiteTearDown != null) {
                    WikiPagePath pagePath = suiteTearDown.getPageCrawler().getFullPath(suiteTearDown);
                    String pagePathName = PathParser.render(pagePath);
                    buffer.append("!include -teardown .")
                          .append(pagePathName)
                          .append("\n");
                }
            }
        }
        pageData.setContent(buffer.toString());
        return pageData.getHtml();
    }
    
    //...
}
​

3-1 HtmlUtil.java

FitNesse 라는 오픈 소스 테스트 도구 코드 중 일부를 가져왔습니다. 하지만 이 코드를 보고 몇 분 이내에 어떤 역할을 하는지 이해하기는 매우 힘듭니다. 이유는 다음과 같습니다.

• 추상화 수준이 다양합니다.
• 하나의 함수 안에 너무 많은 코드가 들어가 있습니다.(약 50줄 정도 들어있습니다.)
• 두 겹 이상 으로 중첩된 if 문이 존재합니다.

이 코드에서 메서드 몇개를 추출하고, 이름을 변경하고, 구조를 변경해보겠습니다.

    public static String renderPageWithSetupsAndTearDowns(PageData pageData, boolean isSuite) throws Exception {
        boolean isTestPage = pageData.hasAttribute("Test");
        if (isTestPage) {
            WikiPage testPage = pageData.getWikiPage();
            StringBuffer newPageContent = new StringBuffer();
            includeSetupPages(testPage, newPageContent, isSuite);
            newPageContent.append(pageData.getContent());
            includeTeardownPages(testPage, newPageContent, isSuite);
            pageData.setContent(newPageContent.toString());
        }
        return pageData.getHtml();
    }

3-2 HtmlUtil.java

코드를 완벽히 이해하긴 어렵지만 TestPage에 setUp 페이지와 teardown 페이지를 넣은 후 해당 페이지를 html로 렌더링 한다는 것을 이해할 수 있습니다.

본격적으로 어떻게 하면 더 이해하기 쉬운 코드를 쓸 수 있는지 설명해보겠습니다.

작게 만들어라!

함수를 작게만드는 게 무조건 좋다 라는 자료를 찾기 어렵지만 경험적으로 적은 줄로 구성된 함수가 이해하기 편합니다. 그렇다면 함수를 얼마나 작게 만들어야 할까요? 저자는 20줄도 길다고 얘기합니다. 일반적으로 3-2 함수도 길다고 얘기하고 3-3 함수 정도로 줄여야 하는게 좋다고 합니다.

 public static String renderPageWithSetupsAndTearDowns(PageData pageData, boolean isSuite) throws Exception {
        if (isTest(pageData)) {
            includeSetupAndTeardownPages(pageData, isSuite);
        }
        return pageData.getHtml();
    }

작게 만들 수 있는 방법으로는 다음과 같은 방법을 활용하라고 합니다.

블록과 들여쓰기

if/else, while 문 등에 들어가는 불록은 한줄이어야 합니다. 즉, 들여쓰기 수즌은 1단을 권장하고 2단을 넘어가선 안됩니다.

한가지만 해라!

함수는 한 가지만을 해야합니다. 하지만 한 가지 라는 것은 알기 어렵습니다. 코드 3-1은 한 가지를 안한다는 것을 명확히 알 수 있지만 코드 3-3은 한 가지를 하는 것일까요? 혹자는 3가지를 한다고 주장할 수 있습니다.

1. 페이지가 테스트 페이지인지 판단합니다.
2. 테스트 페이지라면 setup 페이지와 teardown 페이지를 추가합니다.
3. 페이지를 HTML 로 렌더링합니다.

3가지 단계는 지정된 함수 이름 아래에서 추상화 수준이 하나입니다.

이를 지정된 함수 이름(renderPageWithSetupsAndTearDowns) 아래에서 추상화 수준이 하나인 단계만 수행한다면 그 함수는 한가지 작업만 합니다. 함수가 한가지 작업만 하는지 판단하는 또 다른 방법 하나는 함수 이름을 단순히 다른 표현이 아니라 의미 있는 이름으로 다른 함수를 추출 할 수 있다 면 그 함수는 여러 작업을 하고 있는 것입니다.

함수당 추상화 수준은 하나로!

함수가 한가지 작업만 하려면 함수 내 모든 문장의 추상화 수준이 동일 해야합니다. 코드 3-1 코드를 살펴보겠습니다.

• getHtml() 은 추상화 수준이 아주 높습니다.
• String pagePathName = PathParser.render(pagePath) 는 추상화 수준이 중간입니다.
• append("\n") 와 같은 코드는 추상화 수준이 아주 낮습니다.

하나의 함수에 여러 추상화 수준이 있으면 해당 코드가 특정 기능의 근본 개념인지, 아니면 세부사항인지 알아보기가 힘듭니다. 또한 사람들이 함수에 세부사항을 점점 더 추가합니다.

추상화 수준을 유지하기 위해선 내려가기 규칙(위에서 아래로 코드 읽기) 방법을 쓰면 해결됩니다.

즉, 위에서 아래로 코드를 읽어 나갈 때 마다 함수 추상화 수준이 한번에 한단계 씩 낮아집니다. 예를들면 아래와 같습니다.(TO 키워드는 함수를 나타내는 키워드 입니다.)

TO 설정 페이지와 해제 페이지를 포함하려면, 설정 페이지를 포함하고, 테스트 페이지 내용을 포함하고, 해제 페이지 내용을 포함한다.
  
  TO 설정 페이지를 포함하려면, 슈트이면 슈트 설정 페이지를 포함한 후 일반 설정 페이지를   포함한다.
  
  TO 슈트 설정 페이지를 포함하려면, 부모 계층에서 "SuiteSetUp" 페이지를 찾아 
  include 문과 페이지 경로를 추가한다.
  
  TO 부모 계층을 검색하려면....

Switch 문

Switch 문은 본질적으로 N 가지를 처리하기 때문에 작게 만들기 어렵습니다. 하지만 다형성을 이용하여 각 switch 문을 저차원 클래스에 숨기고 절대로 반복하지 않는 방법이 있습니다.

    public Money calculatePay(Employee e) throws InvalidEmployeeType {
        switch (e.type) {
            case COMMISSIONED:
                return calculateCommissionedPay(e);     
            case HOURLY:
                return calculateHourlyPay(e);
            case SALARIED:
                return cacualteSalariedPay(e);
            default:
                throw new InvalidEmployeeType(e.type);
        }
    }

이 함수는 다음과 같은 문제가 있습니다.

1. 함수가 길다. 새 직원 유형을 추가하면 더 길어진다.
2. '한 가지' 작업만 수행하지 않는다.
3. 코드를 변경할 이유가 여러가지 이기 때문에 SRP 원칙이 지켜지지 않았다.
4. 새 직원 유형을 추가할 때마다 코드를 변경하기 때문에 OCP 원칙이 위반되었다.
5. 위 함수와 구조가 동일한 함수가 무한정 존재할 수 있다.
   • isPayday(Employee e, Date date);
   • deliveryPay(Employee e, Money pay);
   • ...

이 문제를 해결하기 위해서 아래와 같은 코드로 리팩토링 해보겠습니다.

public abstract class Employee {
    public abstract boolean isPayday();
    public abstract Money calculatePay();
    public abstract void deliverPay(Money pay);
}
​
---
  
public interface EmployeeFactory {
    public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType;
}
​
---
​
public class EmployeeFactoryImpl implements EmployeeFactory{
    @Override
    public Employee makeEmployee(EmployeeRecord r) throws InvalidEmployeeType {
        switch (r.type) {
            case COMMISSIONED:
                return new CommissionedEmployee(r);
            case HOURLY:
                return new HourlyEmployee(r);
            case SALARIED:
                return new SalariedEmployee(r)
        }
    }
}

switch 문을 추상 팩토리에 숨겼습니다. 팩토리는 switch 문을 사용해 적절한 Employee 파생클래스의 인스턴스를 생성합니다. calculatePay, isPayday, deliverPay 등과 같은 함수는 Employee 인터페이스를 거쳐 호출됩니다.

이렇게 상속 관계로 숨긴 후 절대로 다른 코드에 노출하지 않게 할 수 있지만, 불가피한 경우도 종종 생깁니다.

서술적인 이름을 사용해라!

길고 서술적인 이름의 함수가 짧고 어려운 이름보다 좋습니다. 또한 이름을 붙일 땐 일관성이 있어야합니다. 즉, 모듈 내 함수이름은 같은 문구, 명사, 동사를 사용합니다.

코드 3-2 와 코드 3-3 을 보면 includeSetupPages, includeSuiteSetupPages 와 같은 함수명이 있는걸 알 수 있습니다. 문체가 비슷하면 향후 코드를 읽을 때 더 가독성이 생깁니다.

함수 인수

함수 인수는 적을 수록 좋습니다. 많더라도 2개 까지는 괜찮고 3개 이상 부턴 피하는게 좋습니다. 가독성 면에서도 그렇고 테스트 관점에서도 인수가 늘어나면 늘어날 수록 모든 조합을 구성해 테스트해보기가 부담스러워 집니다.

단항 형식

• 인수에게 질문을 던지는 경우

boolean fileExists("MyFile")

• 인수를 뭔가로 변환해 결과를 반환하는 경우

InputStream fileOpen("MyFile")

• 이벤트(이벤트라는 사실이 코드에 명확히 드러나야 합니다.)

void passwordAttempFailedNtimes(int attempts)

위 3경우를 제외한 나머지 경우엔 단항 함수는 가급적 피합니다. 예를들어 아래와 같은 함수가 있습니다.

void includeSetupPageInto(StringBuffer pageText)

변환 함수에서 출력인수를 사용하면 혼란을 일으킵니다. 입력 인수를 변환하는 함수라면 변환 결과는 반환값으로 돌려줘야 합니다.

String transform(StringBuffer in) 이
void transform(StringBuffer out, StringBuffer in) 보다 좋습니다. 

플래그 인수

플래그 인수를 함수 파라미터로 넣는것은 해당 함수가 여러 역할을 하고 있다는 것을 공표하는것과 다를바 없습니다. 따라서 플래그 인수(Boolean 값) 은 인수로 넣지 않도록 합니다.

이항 함수

이항 함수는 단항 함수보다 이해하기 힘듭니다. 예를들어 writeField(name) 은 writeField(outputStream, name) 보다 이해하기 더 힘듭니다. 따라서 이항 함수보단 단항 함수를 쓰는게 더 바람직 합니다. 하지만 2차원 좌표를 나타내는 Point(x, y)는 예외 입니다. 이는 자연적인 순서가 있어 이해하는데 변합니다.

삼항 함수

인수가 3개인 함수는 인수가 2개인 함수보다 훨씬 이해하기 어렵습니다. 따라서 삼항 함수를 만들땐 신중히 고려해야 합니다.

인수 객체

인수가 2,3 개 필요하다면 일부를 독자적인 클래스 변수로 선언할 가능성을 짚어볼 수 있습니다. 예를들어 다음과 같은 함수가 있습니다.

Circle makeCircle(double x, double y, double radius);
Circle makeCircle(Point center, double radius);

위 예제에서 살펴보면 이차원 좌표(x, y) 를 하나의 객체 Point 로 만들어 리팩토링 하였습니다.

인수목록

때론 인수의 개수가 가변적인 함수도 필요합니다. 예를들어 String.format 과 같은 함수가 있습니다.

String.format("%s worked %.2f hours", name, hours)

이는 가변 인수를 하나의 리스트 인수로 보고 판단을 하면 됩니다. 즉, 위의 코드는 하나의 인수를 받는 형태가 됩니다.

동사와 키워드

단항 함수는 함수와 인수가 동사/명사 쌍을 이뤄야 합니다. 예를들어 write(name) 과 같은 코드는 바로 이해할 수 있습니다. 또한 함수 이름에 키워드를 넣을 때 더 좋아지는 경우가 있습니다. 예를들어 assertEquals(expected, actual) 보다 assertExpectedEqualsActual(expected, actual) 이 더 좋습니다. 이러면 인수 순서를 기억할 필요가 없습니다.

부수효과를 일으키지 마라!

public class UserValidator {
        private Cryptographer cryptographer;
        
        
        public boolean checkPassword(String userName, String password) {
            User user = UserGateway.findByName(suerName);
            if (user != User.NULL) {
                String codedPhrase = user.getPhraseEncodedByPassword();
                String phrase = cryptographer.decrypt(codedPhrase, password);
                if ("Valid Password".equals(phrase)) {
                    Session.initialize();
                    return true;
                }
            }
            return false;
        }
    }

위 함수를 보면 비밀번호를 체크하는 줄 만 알았지만 Session.initialize() 라는 세션 초기화 함수가 들어있어 부수효과가 일어납니다.

이 부수효과 때문에 checkPassword 함수는 세션이 초기화 되어도 괜찮은 경우에만 호출이 가능합니다. 이와 같은 부수효과를 명확히 하기 위해선 함수 이름을 명확히 짓는게 좋습니다. 위 함수는 checkPasswordAndInitializeSession 이라는 함수명이 더 좋습니다. 하지만 이 함수명은 2가지 역할을 하게 됩니다.

출력인수

출력인수는 피해야 합니다. 함수에서 상태를 변경해야 한다면 함수가 속한 객체 상태를 변경하는 방식을 택해야 합니다.

명령과 조회를 분리해라!

함수는 뭔가를 수행하거나 뭔가에 답하거나 둘중에 하나만 해야합니다. 이 둘을 다하면 안됩니다.

public boolean set(String attribute, String value);

이 함수는 이름이 attribute 인 속성을 찾아 값을 value 로 설정한 후 성공하면 true를 반환하고 실패하면 false를 반환합니다. 그 결과 이런 코드가 생성되었습니다.

if (set("username", "unclebob"))...

사전지식을 모른다면 코드를 보고 많이 당황스럽습니다. "username을 unclebob으로 하는거 같은데 이게 true를 리턴하는건가?" 와 같이 혼란스럽습니다. 이를 개선하기 위해선 명령과 조회를 분리해야 합니다.

if (attributeExists("username")) {
  setAttribute("username", "unclebob");
  ...
}

오류 코드보다 예외를 사용하라

if (deletePage(page) == E_OK)

위 코드의 경우 명령과 조회를 분리해라 라는 권고사항을 위반함과 동시에 오류코드를 반환하면 호출자는 오류 코드를 곧바로 처리해아 한다는 문제에 부딪힙니다.

if (deletePage(page) == E_OK) {
  if (registry.deleteReference(page.name) == E_OK) {
    if (configKeys.deleteKey(page.name.makeKey())== E_OK) {
      logger.log("page deleted");
    }  else {
      logger.log("configKey not deleted")
    } 
  }  else {
      logger.log("deleteReference from registry failed")
    }
} else {
    logger.log("delete failed")
}

반면 오류 코드 대신 예외를 사용하면 코드가 더 깔끔해집니다.

try {
    deletePage(page);
    registry.deleteReference(page.name);
    configKeys.deleteKey(page.name.makeKey());
}
catch (Exception e) {
    logger.log(e.getMessage());
}

Try/catch 블록 뽑아내기

Try/catch 블록은 원래 코드를 알아보기 힘듭니다. 따라서 블록을 별도 함수로 뽑아내는것이 좋습니다.

private void deletePageAndAllReferences(Page page) throws Exception  {
        deletePage(page);
        registry.deleteReference(page.name);
        configKeys.deleteKey(page.name.makeKey());
}
private void logError(Exception e) {
        logger.log(e.getMessage());
}
​

오류 처리도 한 가지 작업이다.

오류를 처리하는 함수는 오루만 처리 해야합니다.

Error.java 의존성 자석

새 오류 코드 대신 기존 오류 코드를 재사용하는게 좋습니다. 오류 코드 대신 예외를 사용하면 새 예외는 Exception 클래스에 파생됩니다.

반복하지 마라

중복은 제거 되어야 합니다. 중복이 일어나면 코드의 길이가 늘어날 뿐만 아니라 중복이 일어난 코드를 수정한다면 중복된 코드 갯수 만큼 수정이 일어나야 하고 그만큼 오류가 발생할 확률이 높아집니다.

구조적 프로그래밍

다익스트라가 만든 프로그래밍원칙으로 모든 함수와 함수 내 모든 블록에 입구와 출구는 하나만 존재해야 한다고 주장했습니다. 즉, 함수는 return 문을 하나만 가지고 있어야 하며 루프 안에서 break 나 continue 를 사용해서 안되고 절대로 goto 를 사용하면 안된다고 주장하였습니다. 하지만 이는 함수가 아주 클 때만 장점을 제공합니다. 따라서 함수를 작게 만들 수 있다면 return, continue, break 등은 사용해도 괜찮습니다. 오히려 이렇게 사용하면 단일 입/출구 원칙보다 더 의미를 잘 표현 할 수 있습니다.

함수를 어떻게 짜죠?

요구사항을 듣자마자 완벽한 클린코드를 짜는 사람은 없습니다. 처음엔 길고 복잡하며 중복이 존재할 수도 있습니다. 이후 단위 테스트 케이스를 만들면서 해당 단위 테스트 케이스를 통과하면서 코드를 리팩토링 해가는 과정이 필요합니다.

결론

대가 프로그래머들은 시스템을 구현할 프로그램이 아니라 풀어갈 이야기로 여긴다고합니다. 우리가 더 명확한 코드와 이해하기 쉬운 코드를 작성함으로서 이야기를 풀어가기가 더 쉬워질 수 있을 것입니다.