LESS TALK, MORE CODE: January 2017

I have tried to make a short summary from the books, blogs that i readed for Exception Handling. I hope the summary will also be useful for you. If I learn new clues, I'll try to update the post.

1. e.printStackTrace() is a bad practice. It prints to the console. In production environments, console is not watching so you should pass exception to the logger. Also printStackTrace uses a lot of memory because of having whole stack details.

2. All processes in the try catch body should be moved to another function and this another function should throw exceptions and do only processes. It is more easier to understand and trace.

3. In a catch block, both logging and throwing exception should not be at the same time. Because when the exception is thrown it will be logged again where it will be caught. This causes multiple logging for the same exception. This leads to misleading results, especially in systems where faults are observed.

4. It is better to add parameter values at the end of the exception log. If the generic messages would be at the beginning, tracing the logs will be easier.

5. If you would use try-catch block in your function, then shouldn't be anything before try, after catch/finally block.

6. If in a try catch block all catch blocks do the same process, then combine catch blocks and use only one catch block.

7. Throw the specific checked exceptions instead of generic exceptions.

//FOR EXAMPLE USE:

public void test() throws IOException {

//INSTEAD OF:
public void test() throws Exception {

8. Don't catch Throwable. It includes Errors which means something serious happened on JVM side. So you should let them go VM.

9. You should prefer to use standard Java Exceptions which are more familiar for developers instead of your own exceptions. If you use your own exceptions, then there will be need to be looked another code fragments.

I have tried to make a short summary for chapter 3 of the Clean Code(Robert Cecil Martin) book. All codes i used and some sentences that i liked too much in the summary are quoted from the book. I hope the summary will also be useful for you.

1. The below functions has duplicated code, odd strings, unobvious data types. However, with a few simple method extractions, some renaming, and a little restructuring will make the functions more understandable.

/* use the first code snippet instead of second one */

/* first code snippet */
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();
}


/* second code snippet */
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();
}

2. "Small"
"The first rule of functions is that they should be small. The second rule of functions is that they should be smaller than that."

//USE: (How short should your functions be?)
public static String renderPageWithSetupsAndTeardowns(PageData pageData, boolean isSuite) throws Exception {
       if (isTestPage(pageData))
              includeSetupAndTeardownPages(pageData, isSuite);
       return pageData.getHtml();
}

//INSTEAD OF:  
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();
}

"The blocks within if statements, else statements, while statements, and so on should be one line long. Probably that line should be a function call.

3."Blocks and Indenting"
The indent level of a function should not be greater than one or two. This, ofcourse, makes the functions easier to read and understand."

4."Do One Thing"
"FUNCTIONS SHOULD DO ONE THING. THEY SHOULD DO IT WELL.THEY SHOULD DO IT ONLY."
"...Another way to know that a function is doing more than “one thing” is if you can extract another function from it with a name that is not merely a restatement of its implementation."

5. "Switch Statements"
"There are several problems with this function. First, it’s large, and when new employee types are added, it will grow."

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

"For example we could have
isPayday(Employee e, Date date),
or
deliverPay(Employee e, Money pay),
or a host of others. All of which would have the same deleterious structure.

The solution to this problem is to bury the switch statement in the basement of an ABSTRACT FACTORY, and never let anyone see it.

The factory will use the switch statement to create appropriate instances of the derivatives of Employee, and the various functions, such as calculatePay, isPayday, and deliverPay, will be dispatched polymorphically through the Employee interface.

My general rule for switch statements is that they can be tolerated if they appear only once, are used to create polymorphic objects, and are hidden behind an inheritance relationship so that the rest of the system can’t see them."

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 {
      
       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 SalariedEmploye(r);
                     default:
                            throw new InvalidEmployeeType(r.type);
              }
       }
}

6."Use Descriptive Names"
"Don’t be afraid to make a name long. A long descriptive name is better than a short enigmatic name. A long descriptive name is better than a long descriptive comment...Don’t be afraid to spend time choosing a name."

7. "Function Arguments"
"The ideal number of arguments for a function is zero (niladic). Next comes one (monadic), followed closely by two (dyadic). Three arguments (triadic) should be avoided where possible."

Argument Objects
"When a function seems to need more than two or three arguments, it is likely that some of those arguments ought to be wrapped into a class of their own."

//USE:
Circle makeCircle(Point center, double radius);

//INSTEAD OF:
Circle makeCircle(double x, double y, double radius);

8."Command Query Separation"

public boolean set(String attribute, String value);

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

The "set" function sets the value of a named attribute and returns true if it is successful and false if no such attribute exists. "Imagine this from the point of view of the reader. What does it mean? Is it asking whether the “username” attribute was previously set to “unclebob”? "

"Functions should either do something or answer something, but not both.

//USE THIS:
if (attributeExists("username")) {
      setAttribute("username", "unclebob");
      // ...
}

9."Prefer Exceptions to Returning Error Codes"
"Returning error codes from command functions is a subtle violation of command query separation. It promotes commands being used as expressions in the predicates of if statements."

if(deletePage(page) == E_OK)

10."Extract Try/Catch Blocks"
"Extract the bodies of the try and catch blocks out into functions of their own.
The delete function is all about error processing. It is easy to understand and then ignore.
The deletePageAndAllReferences function is all about the processes of fully deleting a page."

public void delete(Page page) {
      try {
            deletePageAndAllReferences(page);
      } catch(Exception e) {
            logError(e);
      }
}

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

"Functions should do one thing. Error handing is one thing. Thus, a function that handles errors should do nothing else."

"The whole testableHtml function refactored according to the principles described here."

package fitnesse.html;

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

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

}

References:
Robert C Martin - Clean Code: A Handbook of Agile Software Craftsmanship References:

LESS TALK, MORE CODE

Tuesday

The superclass "javax.servlet.http.HttpServlet" was not found on the Java Build Path

Cannot change version of project facet Dynamic Web Module to 2.5

Wednesday

Exception Handling - Coding Practices

Friday

Clean Code (Robert Cecil Martin) - Chapter3 Functions - My summary