Re: Mixing OO and DB

From: Robert Martin <unclebob_at_objectmentor.com>
Date: Sat, 8 Mar 2008 16:19:41 -0600
Message-ID: <2008030816194175249-unclebob_at_objectmentorcom>


On 2008-03-06 02:03:52 -0600, Marshall <marshall.spight_at_gmail.com> said:

> On Mar 5, 11:35 pm, Robert Martin <uncle..._at_objectmentor.com> wrote:

>> On 2008-03-05 00:27:50 -0600, Marshall <marshall.spi..._at_gmail.com> said:
>> 
>> A function should be no longer than five or ten lines of code.  This is
>> about as obvious and self-evident as it can be.  Are you really going
>> to argue that functions should be longer?

>
> I'm going to argue that I have seen no evidence that writing
> methods any smaller than 150 lines produces any measurable
> benefits. I'm further going to argue that you haven't either.
> Your above assertions are baseless; they are founded on
> sand. They are nothing more than an aesthetic you have
> developed. Like all nostrums, they benefit the salesman
> and not the patient. It is a common human activity: make
> shit up and pretend it's celestial truth. It's as obvious and
> self-evident as hierarchies of angels.

Here's an interesting study: http://www.enerjy.com/blog/?p=241

It corelates CC with defects. CC > 11 leads to greater defects. That means that if a function has more than 11 different execution pathways through it, the defect rate increases.

Now, one way to keep the CC low, is to keep the function size low. Indeed, the longer the function the more likely it is for CC to increase since every if/while/switch splits the execution path.

But studies and numbers like this just tell us what we already know. Long functions are hard to read, and small ones are easy. As an example, please consider the following side by side example:

***Before Partitioning***
  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();

  }

***After Partitining***
public class SetupTeardownIncluder {

  private PageData pageData;

  private boolean isSuite;

  private WikiPage testPage;

  private StringBuffer newPageContent;

  private PageCrawler pageCrawler;

  public static String render(PageData pageData) throws Exception {

    return render(pageData, false);

  }

  public static String render(PageData pageData, boolean isSuite)

    throws Exception {

    return new SetupTeardownIncluder(pageData).render(isSuite);

  }

  private SetupTeardownIncluder(PageData pageData) {

    this.pageData = pageData;

    testPage = pageData.getWikiPage();

    pageCrawler = testPage.getPageCrawler();

    newPageContent = new StringBuffer();

  }

  private String render(boolean isSuite) throws Exception {

    this.isSuite = isSuite;

    if (isTestPage())

      includeSetupAndTeardownPages();

    return pageData.getHtml();

  }

  private boolean isTestPage() throws Exception {

    return pageData.hasAttribute("Test");

  }

  private void includeSetupAndTeardownPages() throws Exception {

    includeSetupPages();

    includePageContent();

    includeTeardownPages();

    updatePageContent();

  }

  private void includeSetupPages() throws Exception {

    if (isSuite)

      includeSuiteSetupPage();

    includeSetupPage();

  }

  private void includeSuiteSetupPage() throws Exception {

    include(SuiteResponder.SUITE_SETUP_NAME, "-setup");

  }

  private void includeSetupPage() throws Exception {

    include("SetUp", "-setup");

  }

  private void includePageContent() throws Exception {

    newPageContent.append(pageData.getContent());

  }

  private void includeTeardownPages() throws Exception {

    includeTeardownPage();

    if (isSuite)

      includeSuiteTeardownPage();

  }

  private void includeTeardownPage() throws Exception {

    include("TearDown", "-teardown");

  }

  private void includeSuiteTeardownPage() throws Exception {

    include(SuiteResponder.SUITE_TEARDOWN_NAME, "-teardown");

  }

  private void updatePageContent() throws Exception {

    pageData.setContent(newPageContent.toString());

  }

  private void include(String pageName, String arg) throws Exception {

    WikiPage inheritedPage = findInheritedPage(pageName);

    if (inheritedPage != null) {

      String pagePathName = getPathNameForPage(inheritedPage);

      buildIncludeDirective(pagePathName, arg);

    }

  }

  private WikiPage findInheritedPage(String pageName) throws Exception {

    return PageCrawlerImpl.getInheritedPage(pageName, testPage);

  }

  private String getPathNameForPage(WikiPage page) throws Exception {

    WikiPagePath pagePath = pageCrawler.getFullPath(page);

    return PathParser.render(pagePath);

  }

  private void buildIncludeDirective(String pagePathName, String arg) {

    newPageContent

      .append("\n!include ")

      .append(arg)

      .append(" .")

      .append(pagePathName)

      .append("\n");

  }

}

-- 
Robert C. Martin (Uncle Bob)  | email: unclebob_at_objectmentor.com
Object Mentor Inc.            | blog:  www.butunclebob.com
The Agile Transition Experts  | web:   www.objectmentor.com
800-338-6716                  |
Received on Sat Mar 08 2008 - 23:19:41 CET

Original text of this message