Chapter2. Function (1)

Clean Code 2번째 이야기! 함수에 대해 정리해보려고 한다. 함수는 2차례 정리를 진행하려고 한다. 오늘은 간단한 코드를 리팩토링하면서 진행해보겠다. 

아래 코드를 쉽게 읽을 수 있는가? 바로 이해가 되는가? 

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

결론부터 말하자면 다음과 같은 문제점이 있다고 할 수 있다.

• 메소드의 길이가 길다.

• 하나의 메소드에서 여러가지 역할을 수행하고 있음

• 메소드의 이름이 지극히 추상적이라 역할을 유추하기 어려움

• 메소드내 코드들의 추상화 단계가 서로 다름

• 메소드의 중복성이 존재

  
package improved;

import fitnesse.responders.run.SuiteResponder;
import fitnesse.wiki.*;

import java.util.HashMap;
import java.util.Map;

public class FitnessExample {

    public String testableHtml(PageData pageData, boolean includeSuiteSetup) throws Exception {
        return new SetUpTearDownSurrounder(pageData, includeSuiteSetup).getTestHtmlWithSetUpAndTearDown();
    }

    private class SetUpTearDownSurrounder {
        private Map prefixByResponder = new HashMap();
        private PageData pageData;
        private boolean includeSuiteSetup;
        private WikiPage wikiPage;

        public SetUpTearDownSurrounder(PageData pageData, boolean includeSuiteSetup) {
            this.pageData = pageData;
            this.includeSuiteSetup = includeSuiteSetup;
            wikiPage = pageData.getWikiPage();

            init();
        }

        public void init() {
            prefixByResponder.put(SuiteResponder.SUITE_SETUP_NAME, "!include -setup .");
            prefixByResponder.put("Setup", "!include -setup .");
            prefixByResponder.put(SuiteResponder.SUITE_TEARDOWN_NAME, "!include -teardown .");
            prefixByResponder.put("TearDown", "!include -teardown .");
        }

        public String getTestHtmlWithSetUpAndTearDown() throws Exception {
            final WikiPage wikiPage = this.pageData.getWikiPage();
            final String content = getTestContentWithSetUpAndTearDown();
            this.pageData.setContent(content);

            return this.pageData.getHtml();
        }

        public String getTestContentWithSetUpAndTearDown() throws Exception {
            StringBuffer buffer = new StringBuffer();
            buffer.append(getSetUpTestPageForRender());
            buffer.append(this.pageData.getContent());
            buffer.append(getTearDownTestPageForRender());

            return buffer.toString();
        }

        public String getSetUpTestPageForRender() throws Exception {
            StringBuffer buffer = new StringBuffer();
            if (isTestPage()) {
                if (this.includeSuiteSetup) {
                    buffer.append(getPageForRender(SuiteResponder.SUITE_SETUP_NAME));
                }
                buffer.append(getPageForRender("SetUp"));
            }

            return buffer.toString();
        }
        public String getTearDownTestPageForRender() throws Exception {
            StringBuffer buffer = new StringBuffer();
            if (isTestPage()) {
                buffer.append(getPageForRender("TearDown"));
                if (this.includeSuiteSetup) {
                    buffer.append(getPageForRender(SuiteResponder.SUITE_TEARDOWN_NAME));
                }
            }
            return buffer.toString();
        }

        public boolean isTestPage() throws Exception {
            return this.pageData.hasAttribute("Test");
        }

        public String getPageForRender(String pageName) throws Exception {
            WikiPage suiteSetup = PageCrawlerImpl.getInheritedPage(pageName, this.wikiPage);
            if (suiteSetup != null) {
                WikiPagePath pagePath = this.wikiPage.getPageCrawler().getFullPath(suiteSetup);
                String pagePathName = PathParser.render(pagePath);
                return String.format("%s%s\n", prefixByResponder.get(pageName), pagePathName);
            }

            return "";
        }
    }

}

Small

메소드는 보는 이로 하여금 이야기책을 읽는 것 같이 느껴져야 한다고 한다. 장편소설이 아닌, 이야기 책이다. 그렇기 때문에 함수는 간결하고, 작아야하며 읽기

편해야 한다. 아이들이 읽는 이야기책이 어려운가?!  아래 내가 직접 리팩토링한 코드를 보면, 메소드가 10줄이상 넘지 않는 것을 볼 수 있다.

그리고 메소드가 설명적인 이름을 갖기 때문에 별도의 주석없이 해당 메소드를 파악하는 것이 쉬우며 문서화의 가치도 있다고 볼 수 있다.

함수를 작게 만들려면, 중첩된 구조를 포함하지 않으며, indent 레벨도 2레벨을 넘지 않는 것이 좋다. (for 내부에 for 내부에 if가 있으면 indent 3레벨)

Do one thing

우리는 변수, 클래스, 메소드 네이밍을 할 때 굉장히 어렵다. 항상 고민하고 Rename을 하는 경우가 굉장히 많다.

메소드가 한가지 일만 수행했을 때, 네이밍에 굉장한 도움이 된다. 무엇을 하는지 명확하기 때문이다. 그리고 한가지 일만 하듯이 작게 만들어야 한다. (Small)

그러나 내가 리팩토링한 코드 중에서 getTestHtmlWIthSetupAndTearDown 이라는 메소드를 보면 한가지 일을 하지 않는다. 3가지 일을 수행한다.

1. WikiPage 생성

2. Setup과 TearDown한 테스트 Content를 생성

3. HTML 코드 반환

하지만 이 메소드는 한가지 일을 한다고 할 수 있다. 이 3가지의 일은 함수의 이름이 나타내는 추상화의 바로 하위수준의 추상화이기 때문이다.

말이 어려울 수 있다. 쉽게 말하면, 3가지의 일이 동일한 추상화 레벨이며 이 메소드의 이름에서 모두 유추가 가능하면, 한가지 일이라 할 수 있다.

비유를 하자면, 우리는 '빵을 굽는다'를 한가지 일이라고 할 수 있다. 그러나 빵을 굽기 위해서는 다음과 같은 단계가 필요하다.

1. 반죽을 한다.
1. 밀가루, 계란, 우유 등을 넣는다.
2. 재료를 섞는다.
2. 발효를 한다.
3. 모양을 만든다.
4. 굽는다.
5. 포장한다.

그러나 이 5단계는 '빵을 굽는다' 라는 일의 하위수준의 추상화이며, 동일한 추상화 단계를 가진다. 아쉽게도 '빵을 굽는다' 라는 이름에 해당 단계들이

포함되지 않지만, 5단계를 모두 수행한다고 유추할 수는 있을 것이다. 이것도 와닿지 않는다면 객체지향 원칙중 하나인 단일책임의 원칙이 메소드에도 

동일하게 적용되야 한다면 더 이해가 쉬울려나? 만약에 내가 만든 메소드가 정말 한가지 일만 수행하고 있는지 확인하고 싶다면, 해당 메소드가 해당 기능을

설명할 수 있는 의미있는 이름인지 확인해봐라!

One Level of Abstraction per Function

메소드가 하나의 일만 수행하기 위해서는 해당 메소드의 코드들이 같은 수준의 추상화 단계를 가져야 한다고 했다.

같은 추상화 단계가 무엇일까?

getHtml() : 매우 높은 수준의 추상화 단계 PathParser.render(pagePath) : 중간 수준의 추상화 단계 append("\n"); : 매우 낮은 추상화 단계

빵을 굽는다 : 매우 높은 수준의 추상화 단계 반죽을 한다 : 중간 수준의 추상화 단계 밀가루, 계란, 우유 등을 넣는다 : 매우 낮은 추상화 단계

Reading Code from Top to Bottom : The Step down rule

우리는 왼쪽에서 오른쪽, 위에서 아래 방향으로 읽는 것이 편하다. 책도 전개, 발단, 위기, 절정, 결말 순으로 진행이 되며, 우리는 순차적으로 읽지 않는가?!

결말부터 역순으로 읽는 변태는 없지 않는가?! 코드를 이야기책에 비유했다. 코드도 마찬가지이다.

메소드 밑에 바로 하위 수준의 추상화를 갖는 메소드가 이어져서 코드를 읽을 때 가독성이 좋을 것이다. 

코드를 top-down 단락으로 읽을 수 있도록 만드는 것이 추상화 수준의 일관성을 유지하는 효과적인 방법이다.