Analytics

Saturday, October 31, 2009

The Perils of Not Unit Testing

Overview

Unit testing is a widely accepted practice in most development shops in this day and age, especially with the advent of the tool JUnit.  JUnit was so widely effective and used early on that it has been included in the default distribution of eclipse as long as I can remember and I have been programming professionally in Java for about 8 years.  However, the drawbacks of not unit testing are concrete and arise acutely from time to time.  This article aims to give a few specific examples of the perils of not unit testing.

Unit Testing Benefits

Unit testing has several basic tangible benefits that has reduced the painstaking troubles of the days when it was not widely used.  Without getting into the specifics of the needs and arguments for unit testing, let's simply highlight the benefits as they are universally accepted by Java development professionals, especially within the Agile community.
  • an automated regression unit test suite has the ability to isolate bugs by unit, as tests focus on a unit and mock out all other dependencies
  • unit tests give feedback to the developer immediately during the test, code, test, code rhythm of development
  • unit tests find defects early in the life cycle. 
  • units tests provide a safety net that facilitates necessary refactoring to improve the design of code without breaking existing functionality
  • unit tests, along with a code coverage tool, can produce tangible metrics such as code coverage which is valuable given good quality of tests
  • unit tests provide an executable example of how client code can use the various interfaces of the code base.
  • the code resulting from unit testing is typically more readable and concise, as code which is not so is difficult to unit test. Thus it follows that code which is written in tandem with unit tests tends to be more modular and higher quality.
Perils of Not Unit Testing

Let's explore by example how not unit testing can adversely affect code and allow bugs to easily enter a code base.  The focus will be on the method level where methods are simple and straight forward, yet there still can be problems when code is not unit tested.


Example 1: Reuse some code, but you introduce a bug

This example illustrates a situation where a developer has good intentions of reusing some code, but due to a lack of unit testing, the developer unintentionally introduces a bug.  If unit tests exists, the developer could have safely refactored and could rely on the unit tests to inform him some requirement had not been covered.

Let's introduce a simple scenario where a clothing store has as system that has users input sales of its clothes. Two objects in the system are: Shirt and ShirtSaleValidator.  The ShirtSaleValidator checks the Shirt to see if the sale prices inputted are correct.  In this case, a shirt sale price has to be between $0.01 and $15. (Note this example is overly simplified, but still illustrates the benefits of unit testing.)

Coder Joe implementes the isShirtSalePriceValid method but writes no unit tests.  He follows the requirements correctly. The code is correct. 


package com.assarconsulting.store.model;

public class Shirt {

    private Double salePrice;
    private String type;
    
    public Shirt() {
    }

    public Double getSalePrice() {
        return salePrice;
    }

    public void setSalePrice(Double salePrice) {
        this.salePrice = salePrice;
    }

    public String getType() {
        return type;
    }

    public void setType(String type) {
        this.type = type;
    }
}


package com.assarconsulting.store.validator;

import com.assarconsulting.store.model.Shirt;
import com.assarconsulting.store.utils.PriceUtility;

public class ShirtSaleValidator {

    public ShirtSaleValidator() {
    }
    
    public boolean isShirtSalePriceValid(Shirt shirt) {
        
        if (shirt.getSalePrice() > 0 && shirt.getSalePrice() <= 15.00) {
            return true;
        }
        
        return false;
    }
}


Coder Bob comes along and he is "refactor" minded, he loves the DRY principle and wants to reuse code.  During some other requirement he implemented a Range object. He sees its usage in the shirt pricing requirement as well. Note that Bob is not extensively familiar with Joe's requirement, but familiar enough to feel competent enough to make a change.  In addition, their group abides by the Extreme Programming principle of collective ownership.

Thus, Bob nobly makes the change to reuse some code. He quickly translates the existing code to use the utility method, and moves on satisfied.


package com.assarconsulting.store.validator;

import com.assarconsulting.store.model.Shirt;
import com.assarconsulting.store.utils.Range;

public class ShirtSaleValidator {

    public ShirtSaleValidator() {
    }
    
    public boolean isShirtSalePriceValid(Shirt shirt) {
                
        Range< Double > range = new Range< Double >(new Double(0), new Double(15));

        if (range.isValueWithinRange(shirt.getSalePrice())) {
            return true;
        }
        
        return false;
    }
} 



package com.assarconsulting.store.utils;

import java.io.Serializable;

public class Range< T extends Comparable> implements Serializable
{
    private T lower;
    private T upper;
    
   
    public Range(T lower, T upper)
    {
        this.lower = lower;
        this.upper = upper;
    }
    
    public boolean isValueWithinRange(T value)
    {
        return lower.compareTo(value) <= 0 && upper.compareTo(value) >= 0;
    }

    public T getLower() {
      return lower;
    }
    
    public T getUpper() {
      return upper;
    }
}


Since there were no unit tests, a bug was created and never caught at time of implementation.  This bug will go unnoticed until a developer or user specifically runs manual tests through the UI or some other client.  What is the bug?  The new code allows 0 to be a price of the Shirt, which is not specified by requirements.

This could have been easily caught if there was an existing set of unit tests to regression test this requirement.  We could have a minimum set of simple tests that checked the range of prices for a shirt.  The set of unit tests could run on each check in of code or each build.  For example, the test suit could have asserted the following.
  • $0 = price executes isShirtSalePriceValid to false
  • $0.01 = price executes isShirtSalePriceValid to true
  • $5 = price executes isShirtSalePriceValid to true
  • $15 = price executes isShirtSalePriceValid to true
  • $16 = price executes isShirtSalePriceValid to false
  • $100 = price executes isShirtSalePriceValid to false 
If Bob has these tests to rely on, the first bullet point test would have failed, and he would have caught his bug  immediately.

Peril - Imagine hundreds of business requirements that are more complicated than this without unit testing.  The compounding effect of not unit testing resulting in bugs, repeated code and difficult maintenance could be exponential compared to the safety net and reduced cost unit testing provides.

Example 2:  Code not unit tested yields untestable code, which leads to unclean, hard to understand code.

Let's continue the clothing store system example, which involves pricing of a shirt object. The business would like to introduce Fall Shirt Sale, which can be described as:

For the Fall, a shirt is eligible to be discounted by 20% if it is priced less than $10 and is a Polo brand. The Fall sales last from Sept 1, 2009 till Nov 15th 2009.

This functionality will be implemented in the ShirtSaleValidator class by Coder Joe who plans not to write unit tests.  Since testing methods is not on his radar, he is not concerned with making the method testable, ie, making short and concise methods to not introduce too much McCabe's cyclomatic complexity.  Increased complexity is difficult to unit test as many test cases are necessary to achieve code coverage.  His code is  correct, but may turn out something like below.


package com.assarconsulting.store.validator;

import java.util.Calendar;
import java.util.Date;
import java.util.GregorianCalendar;

import com.assarconsulting.store.model.Shirt;
import com.assarconsulting.store.utils.PriceUtility;

public class ShirtSaleValidator {

    private Calendar START_FALL_SALE_AFTER = new GregorianCalendar(2009, Calendar.AUGUST, 31);
    private Calendar END_FALL_SALE_BEFORE = new GregorianCalendar(2009, Calendar.NOVEMBER, 16);
    
    public ShirtSaleValidator() {
    }
    
    public boolean isShirtEligibleForFallSaleNotTestable(Shirt shirt) {
        
        Date today = new Date();
        
        if (today.after(START_FALL_SALE_AFTER.getTime()) && today.before(END_FALL_SALE_BEFORE.getTime())) {
            
            if (shirt.getSalePrice() > 0 && shirt.getSalePrice() <= 10 ) {
                
                if (shirt.getType().equals("Polo")) {
                    return true;
                }
            }
        }
        
        return false;
    }
}


The problems with this code are numerous, including misplacement of logic according to OO principles and lack of Enums.

However, putting these other concerns aside, let's focus on the readability of this this method. It is hard to ascertain the meaning of this code by just looking at it in a short amount of time. A developer has to study the code to figure out what requirements it is addressed.  This is not optimal.

Now's lets think about the testability of this method.  If anyone was to test Joe's code, after he decided to leave it this way due to his NOT unit testing, it would be very difficult to test.  The code contains 3 nested if statements where 2 of them have 'ands' and they all net result in many paths through the code. The inputs to this test would be a nightmare.  I view this type of code as a consequence of not following TDD, i.e. writing code without the intention of testing it.

A more TDD oriented way of writing this code would be as follows.


package com.assarconsulting.store.validator;

import java.util.Calendar;
import java.util.Date;
import java.util.GregorianCalendar;

import com.assarconsulting.store.model.Shirt;
import com.assarconsulting.store.utils.PriceUtility;

public class ShirtSaleValidator {

    private Calendar START_FALL_SALE_AFTER = new GregorianCalendar(2009, Calendar.AUGUST, 31);
    private Calendar END_FALL_SALE_BEFORE = new GregorianCalendar(2009, Calendar.NOVEMBER, 16);
    
    public ShirtSaleValidator() {
    }
    
    public boolean isShirtEligibleForFallSale(Shirt shirt) {
        
        return isFallSaleInSession() &&
                isShirtLessThanTen(shirt) &&
                isShirtPolo(shirt);

    }

    protected boolean isFallSaleInSession() {
        Date today = new Date();
        return today.after(START_FALL_SALE_AFTER.getTime()) && today.before(END_FALL_SALE_BEFORE.getTime());
    }
    
    protected boolean isShirtLessThanTen(Shirt shirt) {
        
        return shirt.getSalePrice() > 0 && shirt.getSalePrice() <= 10;
    }
    
    protected boolean isShirtPolo(Shirt shirt) {
        return shirt.getType().equals("Polo");
    }
}


From this code we can see that the method isShirtEligibleForFallSale() reads much like the requirement.  The methods that compose it are readable. The requirements are broken up amongst the methods.  We can test each component of the requirement separately with 2-3 test methods each.  The code is clean and with a set of unit tests, there is proof of its correctness and a safety net for refactoring.

Peril - Writing code without the intention of testing can result in badly structured code as well as difficult to maintain code.

Conclusion

The above examples are only simple illustrations of the drawbacks of foregoing unit testing.  The summation and compounding effect of the perils of not unit testing can make development difficult and costly to a system. I hope the illustrations above communicate the importance of unit testing code.

Source Code 


peril-not-unit-testing.zip