Contract of the interfaces

As I mentioned in my previous post titled Don't test everything, unit testing is very valuable in the software development process, but on some occasions the test expects more that it should. In some cases those expectation work fine, in other they fail.

What the hell am I talking about? If you follow the good practice of coding against interfaces, you know that the interface defines the contract all implementation classes must adhere to. If you write an implementation of an interface your unit tests should test possibly everything that this contract specifies and nothing more.

Take a Set from Java Collections Framework for example. This interface is defined as:

A collection that contains no duplicate elements. More formally, sets contain no pair of elements e1 and e2 such that e1.equals(e2), and at most one null element. As implied by its name, this interface models the mathematical set abstraction.

At some point I had a unit test that had some input values and my test was assuming the correct results in the set that was returned. I knew that my implementation wa returning a HashSet and I also knew what was in that set. The asserts were quite simple (the code is simplified):

String[] addresses = new String[] {"address1", "address2", "address3"};
Set set = new HashSet(Arrays.asList(addresses));
Iterator i = set.iterator();
assertEquals("address1", i.next());
assertEquals("address2", i.next());
assertEquals("address3", i.next());

It's a good test, you may think. Well, it isn't! This test runs or fails depending on which JDK you use. The problem with this test is that it assumes the order of the elements in the returning set. Set interface does not guarantee the order of its elements. We should not test that.

Let's have a look at the following unit test

import junit.framework.TestCase;

import java.util.Set;
import java.util.Arrays;
import java.util.HashSet;
import java.util.Iterator;

public class TestJdkDiff extends TestCase
{
    public void testOrder()
    {
        String javaVersion = System.getProperty("java.version");
        String javaVendor = System.getProperty("java.vendor");
        System.out.println("Running " + javaVendor + " " + javaVersion);

        String[] addresses = new String[] {"address1", "address2", "address3"};
        final Set set = new HashSet(Arrays.asList(addresses));
        for (Iterator i = set.iterator(); i.hasNext();)
        {
            System.out.println(i.next());
        }
    }
}

Execution of the previous test on Java 6 produces the following output

Running Sun Microsystems Inc. 1.6.0_02
address1
address2
address3

The same test executed on Sun's JVM 1.4 prints out

Running Sun Microsystems Inc. 1.4.2_12
address2
address3
address1

Oops! Now I know why my test failed. Even, in JDK the implementation of some classes can change from time to time. As long as the contact is kept all should be fine.

So to fix my initial test one should write

String[] addresses = new String[] {"address1", "address2", "address3"};
Set set = new HashSet(Arrays.asList(addresses));
assertEquals(3, set.size());
assertTrue(set.contains("address1"));
assertTrue(set.contains("address2"));
assertTrue(set.contains("address3"));

Happy coding!

Trailing Comma in Arrays

Most of us know how to initialize arrays. A usual way to initialize array is to specify all its elements. For example an array of integers can be initialized in following way:

Integer[] integerArray = new Integer[] {
    new Integer(1),
    new Integer(2),
    new Integer(4)
};

I was generating code for a unit test and needed to create a List that I would pass to the constructor of a particular class I wanted to test. So I took the easiest way to create array then convert it to the list. In the process I generated the elements of the array by copying and pasting previously added element. The code looked like this:

List initList = Arrays.asList(new Integer[] {
    new Integer(1),
    new Integer(2),
    new Integer(1),
    new Integer(2),
});

Notice the trailing comma! To my surprise, my IDE did not warn me about compilation error, moreover the code compiled and ran just fine.

According to Java Language Specification, chapter 10.6 Array Initializers

An array initializer is written as a comma-separated list of expressions, enclosed by braces "{" and "}".

The length of the constructed array will equal the number of expressions.

The expressions in an array initializer are executed from left to right in the textual order they occur in the source code. The nth variable initializer specifies the value of the n-1st array component. Each expression must be assignment-compatible (§5.2) with the array's component type, or a compile-time error results.

If the component type is itself an array type, then the expression specifying a component may itself be an array initializer; that is, array initializers may be nested.

A trailing comma may appear after the last expression in an array initializer and is ignored.

I must admit that I never used trailing commas in arrays in my whole software engineering career and on this occasion, this took my be surprise. I guess making mistakes it one way of discovering the truth and realizing that a mistake may not be a mistake after all.

So, there you go! Initializing arrays in Java with a trailing comma is totally valid. I learn something new every day :-)

First element in the List

Imagine this scenario. You are working on an existing application. There is a framework used. One of the benefits that this framework gives you is retrieving and processing parameters coming from the client, let's say a web application. The framework gives you all parameters as a List. Now in this particular case you always get only one parameter, but is it enclosed in the List. How do you get it out efficiently?

The intended method would be described: Get the list of parameters and if not empty, return first element in the list otherwise return null. The code was looked similar to this:

1 public E getSingleParam(List params) {
2     E param = null;
3     if (params != null && params.size() >= 1) {
4         param = params.iterator().next();
5     }
6 }

This code works fine but it has several points for improvement. First is how the code at line 3 expresses the intention of check whether the list of parameters is not null and not empty, specifically the later. List interface defines boolean isEmpty() method that is in most cases more efficient to run than getting the size and comparing it to 0 (greater than) or 1 (grater than or equal). That is if the list implements an internal flag for its "emptiness" state or has an internal counter as opposed to re-counting its elements. In the worst case scenario the efficiency will be the same as getting the size and comparing it with 0. In that case isEmpty() method is still a nice convenience method to call and should be preferred before the size() > 0 alternative. Another reason is that it speaks for itself. All we need to know is whether there are any elements in the list or not. Getting the size should be used to for other purposes (e.g. calculating the width of the table column when displaying the results in a tabular form – 100% / size()).

1 public E getSingleParam(List params) {
2     E param = null;
3     if (params != null && !params.isEmpty()) {
4         param = params.iterator().next();
5     }
6 }

The second point of making the code to perform better and to make it easier to read is the line 4. On this line we are getting the first element of the list. As shown in the example above, this is an inefficient way as we construct an Iterator only for the purpose of one iteration. Constructing new objects and disposing of them is usually expensive and should be avoided. A better way is to access the element directly, such as:

1 public E getSingleParam(List params) {
2     E param = null;
3     if (params != null && !params.isEmpty()) {
4         param = params.get(0);
5     }
6 }

This way no new objects are constructed (no Iterator). Another difference between the two is the exception that could be possibly thrown. The exception would be thrown if we did not have the previous check or in the case of concurrent modification of the list which is very unlikely in this case. The exception thrown in first case would be a NoSuchElementException. In the second way, an IndexOutOfBoundsException would be thrown. Both of them are runtime exceptions and do not have to be declared. In this case getting one exception or the other should not make any difference.