Wednesday, June 23, 2010

Turning functional code into great code: Part 1: Testing

I recently came into contact with an eager young developer with a year or two of work experience, who showed me his code and asked me how to improve it. Since I've been asked this by many people over the course of many years, I thought I might be useful for other young (or not-so-young!) developers who wish to improve their code also. I plan to break this into multiple parts, providing explanations and brief justifications for those who may be new to various best practices, even basic ones. Hope you stay tuned!

Caveat: I am not the world's foremost expert in, well, much of anything, and I have formed various opinions and biases over the years. So if you're a senior developer or architect, I'm sure you'll see something here that either I missed or you disagree with. Please feel free to let me know in the comments and I'll be happy to discuss it further. Also please note that I'll only be covering testability in this part, and will gradually introduce more sophisticated improvements in future installments.

Okay, without further ado, let's get started! The code in question builds an XSLT stylesheet for validation and processing of data contained in an XML file.

StylesheetCreationAction.java

package net.galluzzo.example.stylesheetbuilder.old;

import java.io.FileOutputStream;
import java.io.PrintStream;
import java.util.ArrayList;
import java.util.List;

import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;

import org.apache.struts.action.Action;
import org.apache.struts.action.ActionForm;
import org.apache.struts.action.ActionForward;
import org.apache.struts.action.ActionMapping;

public class StylesheetCreationAction extends Action
{
String stylesheetDirectory = "C:\\apache-tomcat\\webapps\\stylesheet-builder\\xslt";

public ActionForward execute( ActionMapping mapping, ActionForm form,
HttpServletRequest request, HttpServletResponse response )
{
//...

createStylesheet( parentNodeName, childNodeNames, ruleList,
outputFilename );

return new ActionForward( "success" );
}

public void createStylesheet( String parentNodeName, String childNodeNames,
List ruleList, String outputFilename ) throws Exception
{
System.out.println( "Inside createStylesheet" );

StylesheetHelper stylesheetHelper = new StylesheetHelper();

String ruleContents = "";
System.out.println( "Number of rules: " + ruleList.size() );

for ( int z = 0; z < ruleList.size(); z++ )
{
ArrayList rule = (ArrayList) ruleList.get( z );

String columnName = (String) rule.get( 1 );
int ruleId = ( Integer.parseInt( rule.get( 2 ) + "" ) );
String param1 = (String) ( rule.get( 3 ) );
String param2 = (String) ( rule.get( 4 ) );
String params = "";
if ( ruleId == 1 || ruleId == 2 || ruleId == 3 || ruleId == 5 )
{
params = param1;
}
else
{
params = param1 + "," + param2;
}
System.out.println( "Rule table values: " + columnName + " "
+ ruleId + " " + params );

switch ( ruleId )
{
case 1: // concat
{
System.out.println( "Case 1" );
ruleContents += stylesheetHelper.concatRule( z + 1,
columnName, params );
break;
}
case 2: // greater than
{
System.out.println( "Case 2" );
if ( params.indexOf( "-" ) < 0 )
{
int paramInt = Integer.parseInt( params );
ruleContents += stylesheetHelper.greaterThanRule(
z + 1, columnName, paramInt );
}
else
{
ruleContents += stylesheetHelper.greaterThanRule(
z + 1, columnName, params );
}
break;
}
// Several other cases here which we'll get to later
}
}

// Get header and footer stylesheet content.
String stylesheetHeader = stylesheetHelper.getStylesheetHeader(
parentNodeName, childNodeNames );
String stylesheetFooter = stylesheetHelper.getStylesheetFooter( parentNodeName );

// Get the beginning and ending text for the <xslt:choose> node.
String xsltChooseStart = stylesheetHelper.getXsltChooseStart();
String xsltWhenEnd = stylesheetHelper.getXsltWhenEnd();
String xsltOtherwiseStart = stylesheetHelper.getXsltOtherwiseStart();
String xsltChooseEnd = stylesheetHelper.getXsltChooseEnd();

// Get XML nodes.
String parentNode = stylesheetHelper.getParentNode();
String rejectedParentNode = stylesheetHelper.getRejectedParentNode();

// Construct the entire stylesheet.
String xsltText = stylesheetHeader + ruleContents + xsltChooseStart
+ parentNode + xsltWhenEnd + xsltOtherwiseStart
+ rejectedParentNode + xsltChooseEnd + stylesheetFooter;

// Create the output file.
FileOutputStream fo = new FileOutputStream( stylesheetDirectory
+ outputFilename );
PrintStream ps = new PrintStream( fo );
ps.println( xsltText );
}
}

We will cover the StylesheetHelper class in a future installment.

Testability


The goal of this series of blog posts is to improve this code. But how do I do that and still ensure that the refactored code still does what the old code did? By writing a unit test, then making changes, and after each change, ensuring that the test still passes.

Unit tests are programs that test a "unit" of code, usually a class, by executing various functionality (methods) on it, and programmatically ensuring that the results are correct. I will use the JUnit 4 framework in this example, for the simple reasons that I know it fairly well, and it is widely used.

The problem is that I cannot actually run the above class on my machine. You see, I am writing this blog post on Linux, and the final code inside createStylesheet writes out a file into a specific Windows directory that is hard-coded into the class. So, for the very smallest change possible, for now, I will change this directory to one that works on my machine. (This is not a good solution, but we'll get to that later.)

Furthermore, this class is a Struts action, which means that it must extend from Struts' Action base class in order to do any work. I don't have a copy of Struts 1.x handy, and frankly I should not need it in order to build an XSLT stylesheet file from a list of passed-in rules. Happily, the createStylesheet method is the one I really care about, and it only uses Java APIs, not Struts or J2EE. The creation of a stylesheet is functionality that can be reused outside of a single web action, so it should go in a non-J2EE-related class. We'll call this class StylesheetCreator, thus:


public class StylesheetCreator
{
String stylesheetDirectory = "/home/eric/workspace/stylesheet-builder/resources/output/";

public void createStylesheet( String parentNodeName, String childNodeNames,
List ruleList, String outputFilename ) throws Exception
{
System.out.println( "Inside createStylesheet" );
// The rest of the original method goes here.
}
}

With that simple change, we can now write a test case for this functionality, which looks like this:


package net.galluzzo.example.stylesheetbuilder.refactored.part1;

import static org.hamcrest.CoreMatchers.is;
import static org.junit.Assert.assertThat;

import java.io.BufferedReader;
import java.io.File;
import java.io.FileReader;
import java.io.IOException;
import java.util.ArrayList;
import java.util.List;

import org.junit.Test;

/**
* Test case for {@link StylesheetCreator}.
*
* @author Eric Galluzzo
*/
public class StylesheetCreatorTest
{
private static final String STYLESHEET = // ...long XML string here

@Test
public void testAllRules() throws Exception
{
StylesheetCreator stylesheetCreator = new StylesheetCreator();

List<List<String>> rules = new ArrayList<List<String>>();
rules.add( createRule( "name", 1, ", Esq.", "" ) );
rules.add( createRule( "age", 2, "18", "" ) );
rules.add( createRule( "category", 2, "-aaa", "" ) );
rules.add( createRule( "age", 3, "100", "" ) );
rules.add( createRule( "category", 3, "-zzz", "" ) );
rules.add( createRule( "department", 4, "old", "new" ) );
rules.add( createRule( "category", 5, "4", "" ) );
rules.add( createRule( "bonus", 16, "100", "5000" ) );
rules.add( createRule( "category", 16, "-aaa", "-zzz" ) );

stylesheetCreator.createStylesheet( "employee",
"name,age,salary,category,department,bonus", rules, "test.xsl" );

String stylesheetText = readFileContents(
new File( stylesheetCreator.stylesheetDirectory, "test.xsl" ) );
assertThat( stylesheetText, is( STYLESHEET ) );
}

private List<String> createRule( String columnName, int ruleId,
String param1, String param2 )
{
List<String> rule = new ArrayList<String>();
rule.add( "employee" );
rule.add( columnName );
rule.add( String.valueOf( ruleId ) );
rule.add( param1 );
rule.add( param2 );
return rule;
}

private String readFileContents( File file ) throws IOException
{
StringBuilder sb = new StringBuilder();
BufferedReader reader = new BufferedReader( new FileReader( file ) );
try
{
String line;
while ( ( line = reader.readLine() ) != null )
{
sb.append( line ).append( "\n" );
}
}
finally
{
reader.close();
}
return sb.toString();
}
}

Note that I've taken some shortcuts here: I've defined only one test, which is a positive test, covering all the different stylesheet rules. Ordinarily one would write both positive and negative tests for each individual rule, so that when a rule broke, one would immediately know exactly which rule it was. However, the all-in-one test is a little shorter, and thus easier to understand on a blog.

It is now possible to run this in one's IDE of choice and get that wonderful green bar, indicating the test has passed! So we can now begin to improve this code, which we will do in Part 2. Stay tuned!

9 comments:

amsa leka said...

Thanks for such a great article here. I was searching for something like this for quite a long time and at last I’ve found it on your blog. It was definitely interesting for me to read about their market situation nowadays. Well written article Thank You for Sharing with Us project management courses in chennai | pmp training class in chennai | pmp training fee | project management training certification | project management training in chennai | project management certification online |

htop said...

this message is a wonderful for our website
best angularjs training in chennai
angular js training in sholinganallur
angularjs training in chennai
azure training in chennai
best java training in chennai
selenium training in chennai

Askmetraveller said...

Thanks for posting useful information.You have provided a nice article, Thank you very much for this one. And I hope this will be useful for many people.. and I am waiting for your next post keep on updating these kinds of knowledgeable things...Really it was an awesome article...very interesting to read..please sharing this information......I think your suggestion would be helpful for me. I will let you know if its work for me too. Thanks and keep post such an informative blogs.

Askmetraveller

Unknown said...

Nice article. Thanks for sharing.
Python classes in Pune
Python training in Pune
Python courses in Pune
Python institute in Pune

Vikas Kumar said...

Thanks for sharing the article.
Visit us
Click Here
For More Details
Visit Website
See More

Vikas Kumar said...

Thanks for sharing it.I got Very valuable information from your blog.your post is really very Informatve. I got Very valuable information from your blog.I’m satisfied with the information that you provide for me.

Python Training in Pune "
Python Classes
Python Placement
Python Institute Pune
Python courses

Wahid Tamboli said...

Thanks For Sharing For More Info Check Below Links

click
Visit
read more
more
more

varun said...

Thanks for your excellent blog and giving great kind of information. So useful. Nice work keep it up thanks for sharing the knowledge.
Visit us
Click Here
For More Details
Visit Website
See More

mahi said...

Please refer below if you are looking for best project center in coimbatore

Java Training in Coimbatore | Digital Marketing Training in Coimbatore | SEO Training in Coimbatore | Tally Training in Coimbatore | Python Training In Coimbatore | Final Year IEEE Java Projects In Coimbatore | IEEE DOT NET PROJECTS IN COIMBATORE | Final Year IEEE Big Data Projects In Coimbatore | Final Year IEEE Python Projects In Coimbatore

Thank you for excellent article.