Showing posts with label type safety. Show all posts
Showing posts with label type safety. Show all posts

Thursday, 23 June 2011

Type Safety: why would you need a Collections.checkedSet in JDK1.4?

In one of the first posts on this blog I have discussed the problem of type checking (or lack of it) in some of Java collections even when using generics. Today I want to show you a similar issue that can break the type safety in your code. It’s a problem that can cause a really nasty bugs in your application – the ones that manifest themselves many thousand lines of code after the faulty method introduce it.
Imagine that you work with generics, but you use a third party code. What if some part of that code was written in Java 1.4 or earlier, therefore not using generics. Let’s go one step further: what if (for some reason) this code does not respect the typing of your collections? You would expect to get an error, right? Well, you’ll get one… but do you know when? Check the following code:

public static void main(String[] args) {
// create a type-safe set of Integers and add some values to it
Set<Integer> setOfInts = new HashSet<Integer>();
setOfInts.add(new Integer(7));
setOfInts.add(new Integer(13));

// pass your set to a non-generic code
Set uncheckedSet = setOfInts;
// non-generic code does not respect your typing
// and adds a String to your set of integers
uncheckedSet.add(new String("Illegal"));

// back in your code - do some set manipulations:
System.out.println(setOfInts);
setOfInts.remove(new Integer(7));

// at the end iterate trough your set
for (Integer integer : setOfInts) {
System.out.println(integer);
}
}

The code above is a simplified version of the described scenario. In lines 8 – 11 the ‘third party’ code is executed and adds a String into a set of Integers. So back to the question: in a given example when will error occur? One might assume in line 11 as the bad element is inserted… wrong! Maybe when we access the collection and print all of its elements?… wrong! We can even do some manipulations on it (line 15) and still be fine! The error will show his ugly head finally in the loop in line 18 when we will enter it for the second time. This will happen so late because in line 18 for the first time we access the inserted String and cast it to Integer – at this point ClassCastException occurs.
In the example above the distance between the faulty insertion and the place where exception is thrown is only seven lines, but you can imagine its being 7000. Your code can be working fine for 7000 hours and after that throw an exception that will basically give you no hint what’s wrong. If you do not use third party code you should not feel safe because of that – I am sure that if your project has more than 10000 lines there is a part of it so old and dusty that it was written without the usage of generics…
What can be done? Well, we are in luck as there is a quite simple way of protecting yourself from that: in java.util.Collections you will find a way to wrap your Set, Map or List into an forwarding class that will check in runtime the typing of the inserted objects. In the case of our code snippet we only need to add one line:

public static void main(String[] args) {
// create a type-safe set of Integers and add some values to it
Set<Integer> setOfInts = new HashSe<Integer>();
setOfInts = Collections.checkedSet(setOfInts, Integer.class);
setOfInts.add(new Integer(7));
setOfInts.add(new Integer(13));

// pass your set to a non-generic code
Set uncheckedSet = setOfInts;
// non-generic code does not respect your typing
// and adds a String to your set of integers
uncheckedSet.add(new String("Illegal"));

// back in your code - do some set manipulations:
System.out.println(setOfInts);
setOfInts.remove(new Integer(7));

// at the end iterate trough your set
for (Integer integer : setOfInts) {
System.out.println(integer);
}
}

As you see the only difference between this and previous snippet is additional line (#4) in which we wrap our set of integers into a CheckedSet. See that due to Generic type erasure in Java the Collections.checked* methods require a class object besides a collection to wrap. Now when we run the new code the error will manifest itself just at the moment where an error is commited: in line 12 when we try to insert a string. Exactly what we needed!

To summarize: if in your project you use either third party code or legacy code that you suspect of not using generics consider wrapping your collections with Collection.checked* methods!


Note :
Users with jdk 6 or higher will get ClassCastException in any case, if they add some wrong datatype.

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.