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

One to One shared primary key association in JPA with Hibernate

Overview

One of the less commonly used mappings in JPA and Hiberate is the @OneToOne mapping with a shared primary key. Conceptually this type of mapping is simple to understand. However, recently I tried to implement this relationship between two objects and ran into some petty issues. No clear example was listed on the net, so I decided to post a solution on a blog for possible reference for others.

OneToOne Shared Primary Key

A one to one shared primary key is not the most common mapping pattern, but is still used by many domain object designers. More common is the one to one mapping with a separate foreign key. A one to one shared primary key is simply where two objects have a one to one relationship where they share a primary key.

For example, let's use the prominant Person to Address example.

A Person can have one Address only, and an Address can only have one Person living in it. Let's consider Person the owner of the relationship, meaning its primary key gets generated first and essentially objectwise an Address is "set" in the Person object. A one to one shared primary key relationship would mean that a Person table does not have a FK column to Address, rather the Address table's primary key value is the same as Person, and acts as a foreign key to the Person table.



In contrast a one to one relationship with a foreign key would result in the Person table having a field like ADDRESS_ID, which acts as a foreign key to the Address table. The Person primary key generation is independent of the Address primary key. This means the primary key to Person and Address are not the same generally, but could be by coincidence.


Essentially, one to one with a shared primary key saves a column in the database and usually forces a bidirectional relationship through an ORM such as hibernate.

References and Example

When attempting to implement one to one shared mapping with JPA, I had a little trouble. The official JPA documentation does not give an example with a generated id, but only gives one where the ids are manually set. The Hibernate annotations site does the same. In addition there are a few blog postings which address the issue, but they are muddled and don't specify clearly the implementation details. Thus, I had to experiment and put together knowledge from all over to reach a solution.

In Hiberate Core (without JPA or Annotation), the concept and mapping implementation is explained nicely. 5.1.13 One-to-One.

The most informational resource was the book: Java Persistence with Hibernate - 7.1 Single Valued entity associations, pg278-282. This gave the implemenation with annotations.

In order to use a one to one with a shared primary key, ids on both sides of the object need a @GeneratedValue. The owner can use a generic generator where it gets a fresh number every time, but the other side of the one to one needs a custom hibernate extension, which is from @GenericGenerator. This grabs the primary key from the owner object and places it in the dependent object. In our example, Address will have the hibernate extension @GenericGenerator.

Person class

package com.assarconsulting.addressbook.model;

import javax.persistence.CascadeType;
import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.Id;
import javax.persistence.OneToOne;
import javax.persistence.Table;

@Entity
@Table(name = "PERSON")
public class Person {

@Id
@GeneratedValue
@Column(name = "PERSON_ID")
private Long id;

@Column(name = "NAME", nullable=false)
private String name;

@Column(name = "AGE", nullable=false)
private int age;

@OneToOne(cascade = CascadeType.ALL, mappedBy = "person")
private Address address;

public Person() {
}

public Person(String name, int age) {
   this.name = name;
   this.age = age;
}

public Long getId() {
   return id;
}

public void setId(Long id) {
   this.id = id;
}

public String getName() {
   return name;
}

public void setName(String name) {
   this.name = name;
}

public int getAge() {
   return age;
}

public void setAge(int age) {
   this.age = age;
}

public String getStreetNumber() {
   return address.getStreetNumber();
}

public String getStreetName() {
   return address.getStreetName();
}

public String getCity() {
   return address.getCity();
}

public String getState() {
   return address.getState();
}

public String getZipCode() {
   return address.getZipCode();
}

public Address getAddress() {
   return address;
}

public void setAddress(Address address) {
   this.address = address;
   address.setPerson(this);
}

}

Address Class

package com.assarconsulting.addressbook.model;

import javax.persistence.Column;
import javax.persistence.Entity;
import javax.persistence.GeneratedValue;
import javax.persistence.Id;
import javax.persistence.OneToOne;
import javax.persistence.PrimaryKeyJoinColumn;
import javax.persistence.Table;

@Entity
@Table(name = "ADDRESS")
@org.hibernate.annotations.GenericGenerator(name="person-primarykey", strategy="foreign",
 parameters={@org.hibernate.annotations.Parameter(name="property", value="person")
})
public class Address {

@Id
@GeneratedValue(generator = "person-primarykey")
@Column(name = "PERSON_ID")
private Long id;

@Column(name = "STREET_NUMBER", nullable=false)
private String streetNumber;

@Column(name = "STREET_NAME", nullable=false)
private String streetName;

@Column(name = "CITY", nullable=false)
private String city;

@Column(name = "STATE", nullable=false)
private String state;

@Column(name = "ZIP_CODE", nullable=false)
private String zipCode;

@OneToOne
@PrimaryKeyJoinColumn
private Person person;

public Address() {
}

public Address(String streetNumber, String streetName, String city,
       String state, String zipCode) {
   super();
   this.streetNumber = streetNumber;
   this.streetName = streetName;
   this.city = city;
   this.state = state;
   this.zipCode = zipCode;
}

public Long getId() {
   return id;
}

public void setId(Long id) {
   this.id = id;
}

public String getStreetNumber() {
   return streetNumber;
}

public void setStreetNumber(String streetNumber) {
   this.streetNumber = streetNumber;
}

public String getStreetName() {
   return streetName;
}

public void setStreetName(String streetName) {
   this.streetName = streetName;
}

public String getCity() {
   return city;
}

public void setCity(String city) {
   this.city = city;
}

public String getState() {
   return state;
}

public void setState(String state) {
   this.state = state;
}

public String getZipCode() {
   return zipCode;
}

public void setZipCode(String zipCode) {
   this.zipCode = zipCode;
}

public Person getPerson() {
   return person;
}

public void setPerson(Person person) {
   this.person = person;
}
}

Why Hibernate does "delete all then re-insert" - its not so strange

Overview

Hibernate allows developers to interact with data from a relational database in an object-oriented fashion. This brings a lot of benefits to maintenance of development. However, hibernate is an abstraction of data which essentially is its function as an ORM, and this usually forces the developer to know hibernate's internals in order to use the tool effectively.

Maintaining collections in the object world is something every application does. Using hibernate you can manipulate colllections by adding and deleting items through the java api, and by way of mapping, hibernate takes care of generating the SQL to augment that data in the database. If the nuances of the mapping is not understood, hibernate may produce what seems like strange results. A particular instance arises when you map a List on the many side of a one to many relationship. When removing an item from the list, hibernate issues a delete statement to delete all elements, and then issue insert statements to insert everything back in except the one removed. This seems ineffecient and odd. Why not just delete the one item? Then if you change the collection type to a Set, only one delete statement is issued by itself. This article tries to explain this behavior and hopefully bring to light hibernate's internals.

Explanation by Example

The scenario described above occurs when mapping a one to many relationship with a association table. The association table stores two attributes, each of which is a foreign key to another table. In addition, the many side object is mapped with a List, but no index column is used. Hibernate recommends when using a list that an index column is used, but why exactly? The example should illuminate why hibernate recommends the index column when mapping a list. The example will show you what happens when using a list without an index column.

Example Setup

Let's set up the example. Let's say I have two classes: A and B. There is a one to many relationship from A to B, where A has many B's in a List. Using hibernate we map B with one to many and leave out the index column.

As instances of these classes, we have an A1 with 3 children and A9 with no children (just for contrasting sake). The figure below illustrates this

In the database, we have three tables. The association table maintains the relationship between A and B. No constraints are in the table for simplicity sake. (We want to focus on hibernate's behavior and worry less about enforcing integrity in the database.) Notice there is not indexed column in AB.


Here is the data in the tables:

Also notice that the list contains two B1's, as a list is capable of containing duplicates. So we code in java to delete an element from A, say the first element in the list which is B1.

a.getBList().remove(0);

So what occurs on hibernate's flush? It first issues an sql to delete all rows in the relationship table where B_ID = 1. Then it inserts all the relationships that are not supposed to be deleted!
This seems strange.
However the reason why this occurs is because hibernate supports the additional feature of a collection that is called a Bag. A bag is a collection which can have duplicates but is unordered. The best feature of a bag is that you can get the number of occurrences of an object through the API with

public int occurrences(Object o) 

With a list, there is no way to do the same without iterating through the whole list.

Thus, when a list is mapped without an indexed collection, hibernate treats in like a bag. In essence, a list which has an indexed column is an "indexed bag.

Since hibernate handles our list as a bag, it believes there could be duplicate entries in the association table. In addition it knows the items in table AB are unordered, so there is no way to tell which row should be deleted from the object in java. We asked earlier, why not just issue one delete statement. If it stopped here we would end up with:

So hibernate needs to continue and re-insert the rows it knows from java that are not supposed to be deleted. In memory, it can just look at the list after remove() was called and insert the remaining relationships. So we end up with what we wanted:



Conclusion

When using a list in hibernate an indexed column is necessary if you want efficient manipulation of adding and deleting items. With an indexed column, when you code a remove(index), hibernate can relate that index to the column in the db and even if there are duplicates, it can distinguish and delete one row. Better yet, if duplicates are not needed in your application, you can use a Set. A set requires no index column and hibernate can distinguish one row because it know a set does not have duplicates.

If you are wondering what is the purpose of a Bag, references below may answer that question.

References

http://www.discursive.com/books/cjcook/reference/collections-sect-bag.html
http://guanyu.info/blog/?p=69
https://forum.hibernate.org/viewtopic.php?t=957657
http://sites.google.com/site/xushengxiaotech/Home/hibernate-tips--efficient-usage-of-bag-for-association