Move repeated code to a common location.

It is customary to see the same java code cut-and-pasted in many classes instead of split out into a separate method.  

The following example is taken from a set of classes used to render HTML from a custom tag library.  This kind of java class uses repeated code snippets to verify the fields needed to do a render, so would have made sense to move most of it into common functions.  A lot of duplicated code is a good indicator that something needs to be simplified.  Take a look at how many checks for == null and trim() calls are done in the function below:

public int doStartTag() throws JspException  
{
    RenderInfo renderInfo = (RenderInfo)pageContext.getAttribute(
                         "renderInfo", PageContext.REQUEST_SCOPE);
    if (renderInfo == null)
    {
        throw new JspTagException("No renderInfo in scope");
    }

    :
    :
     
    JspWriter out = pageContext.getOut();
    out.print("<img");
    if (styleClass != null)
    {
        out.print(" class=\"" + styleClass + '"');  
    }
    if (src == null || src.trim().length() == 0)  
    {
        throw new JspTagException("Error. src= not provided");
    }
    out.print(" src=\"" + src + '"');  
    if (alt == null || alt.trim().length() == 0)  
    {
        throw new JspTagException("Error. alt= not provided");
    }
    out.print(" alt=\"" + alt + '"');  
    out.print(" />");  
    :
    :
}

The function was hundreds of lines lone.  In the improved example below, the common pieces of code have been moved to simple static methods in a class called RenderUtils.java yet the functionality is exactly the same.  Nothing particularly difficult; a little more upfront effort saved hours of repetitive coding throughout the project.  Moving the common code to helper functions means it is easier to add informative error messages in a single place.  The above and below examples perform the same function.  Can you guess which of the two is easier to maintain?

public int doStartTag() throws JspException  
{
    RenderInfo renderInfo = RenderUtils.getRenderInfo(this);
    :
    :
     
    JspWriter out = pageContext.getOut();
    out.print("<img");
    RenderUtils.addOptionalAttribute(out, "class", styleClass);
    RenderUtils.addMandatoryAttribute(out, "src", src);
    RenderUtils.addMandatoryAttribute(out, "alt", alt);
    out.print(" />");  
    :
    :
}

Tip: After the first few classes have been written, the common code will be clear so it is not always necessary (or possible) to plan the libraries from the beginning.  It is usually better to refactor when you discover the commonality.

Tip: If at first you don't know which package to put common code, anywhere will do.  With tools such as Eclipse it is easy to move later when the most suitable location reveals itself, so have the courage to move it around as long as you communicate with those who will be affected.

blog comments powered by Disqus