Type safety in Java Set and Map in JDK 1.4

Probably many of you still remember the lack of type checking in Java 1.4 Collections and how much hassle it was to deal with casting the collection elements, not to mention how many errors this introduced to the code. Since introduction of generics in Java 1.5 this have really improved and one might think that nowadays the language itself protects the programmer from the silliest of typing mistakes. Generics themselves brought with them a set of new complications, but it seems reasonable to think that in the basic code situations when using Java’s Sets or Maps and when there is no kung-fu complications like wildcards or casting we are safe and secure. Are we really?
Recently I have encountered a bug in a production code when dealing with a simple (really simple) usage of a Map. The embarassing part of this issue was that it took lots of effort to debug it and the cause of it was… well… trivial. The code below shows a sketch of how the code looked like before the bug was introduced. In real life the class was a part of a really old and rarely edited legacy code, obviously it was much larger than the one below:

import java.util.HashMap;
import java.util.Map;

public class EmployeeDataLookup {
    // A map storing relation between employee ID and name.
    private Map<Integer, String> employeeIdToName;

    public EmployeeDataLookup() {
        // Create a new EmployeeDataLookup and initialize
        // it with employee names and IDs.
        employeeIdToName = new HashMap<Integer, String>();
        addEmployee(301, "John Doe");
        addEmployee(302, "Mary Poppins");
        addEmployee(303, "Andy Stevens");
    }

    public void addEmployee(int employeeId, String employeeName) {
        employeeIdToName.put(employeeId, employeeName);
    }

    // This class is very complicated and has many other methods...

    // Lookup method for finding employee name for given employee ID.
    public String findEmployeeName(int employeeId) {
        return employeeIdToName.get(employeeId);
    }

    public static void main(String[] args) {
        // Create a EmployeeDataLookup instance
        EmployeeDataLookup employeeLookup = new EmployeeDataLookup();
        // Find the name of an employee with ID = 301
        String employeeName = employeeLookup.findEmployeeName(301);
        System.out.print("Employee 301 : " + employeeName);
    }
}

So what went wrong? Well, at some point the project functionality requirements have changed (surprising, right?) and the key of the map storing data inside of the class had to be changed. In terms of the example above you might say that a company has merged with one of the vendors, so the system storing the employee data had to be made compatible with the ID system of the vendor. Because the vendor company used 13 digit IDs to identify its employees the change to the code in Java terms meant that instead of using Integers we had to change to Longs. Simple right? Its even easier if you use an IDE like Eclipse – just change/refactor the Map key type in line 7 from Integer to Long, let the editor show you all the errors, fix them and that is it! In five minutes all is done:

import java.util.HashMap;
import java.util.Map;

public class EmployeeDataLookup2 {
    // A map storing relation between employee ID and name.
    private Map<Long, String> employeeIdToName;

    public EmployeeDataLookup2() {
        // Create a new EmployeeDataLookup and initialize
        // it with employee names and IDs.
        employeeIdToName = new HashMap<Long, String>();
        addEmployee(301, "John Doe");
        addEmployee(302, "Mary Poppins");
        addEmployee(303, "Andy Stevens");
    }

    public void addEmployee(long employeeId, String employeeName) {
        employeeIdToName.put(employeeId, employeeName);
    }

    // This class is very complicated and has many other methods...

    // Lookup method for finding employee name for given employee ID.
    public String findEmployeeName(int employeeId) {
        return employeeIdToName.get(employeeId);
    }

    public static void main(String[] args) {
        // Create a EmployeeDataLookup instance
        EmployeeDataLookup employeeLookup = new EmployeeDataLookup();
        // Find the name of an employee with ID = 301
        String employeeName = employeeLookup.findEmployeeName(301);
        System.out.print("Employee 301 : " + employeeName);
    }
}

The change above introduced the bug. Can you see it? When you run the main method now you will find out that the name of the employee with ID=301 is ‘null’! Ha!

If you do not see the trouble (or are to lazy to try) I’ll give you a hint: check out the function findEmployeeName . If you have seen it, try to imagine that this class in real life had 1000+ lines of code unrelated to the changed map. Since there are no compile errors it was like looking for a needle in a haystack… so what’s the bug exactly?

The problem is that after introducing generics, probably due to backward compatibility, some of the methods in Collection and Map interface have not been changed and still do not perform type checking. One of them is Map.get(Object) which have caused the bug in the code above. After the ’simple change’ we have still been using Integer keys to access the map that contained Longs, so even though we had a record of employee 301 in our system the get method returned null. And because of the lack of type checking there were no warnings… Busted.

So what’s the lesson from this? Be cautious about type checking even in the most basic situations. Remember that access methods in Sets, Lists and Maps can behave and bite you in the behind. The biggest pain is that the bugs introduced this way are usually hard to spot by a human, so sometimes a dozen of engineers staring at the code can have trouble finding it. There is a way to deal with them though – most of them can be easily found if you are using a static code analysis tool (eg: FindBugs).

Note:
If you are using JDK 6 or higher, its no worries for you. Because in any case you would get the correct result.