Stupidity, brought to you by the letter L

I grabbed some time formatting code from an older project of mine to use in my current project.

However, I noticed that the constants defined at the top of the class had some weird values. The constant, number of milliseconds per whatever, was ending in a 1 instead of a 0. My first thought was that a co-worker had mangled my source to fix some stupid off-by-one time issue they were having, incidentally breaking everything else that relied on that constant.

While 600 to 601 is a plausible change, 60 milliseconds per minute to 600 milliseconds per minute should have set off alarm bells in my head.

I spent several minutes grepping through the svn repository trying to find the lazy rat-bastard that had messed with my code, only to find that it wasn't a '601' but a '60l' (60L). The L is there to force the value into a long integer type.

Lesson learned? Use uppercase L.

Don't use Integer keys with WeakHashMap

WeakHashMap relies on the key of an entry being garbage collected (due to being weakly referenced) so that the entire Map.Entry can be discarded.

However, if you use Integer as your map key, you can get suprising results when combined with autoboxing, due to a hidden cache of j.l.Integer objects embedded in java.lang.Integer itself.

This cache has been discussed in several other places (ie. http://www.owasp.org/index.php/Java_gotchas), particularly in respect to testing equality.

However, this cache of Integers from -128...127 will also mess with your WeakHashMap, pinning any elements in that range in memory, never to be released.

Here is a quick and dirty test demonstrating this issue:

 public void testWeakHashMapWithIntKeys()
 {
  WeakHashMap<integer, stringbuffer> map = new WeakHashMap<integer, stringbuffer>();

  map.put( 1, new StringBuffer("Test 1") );
  map.put( 2, new StringBuffer("Test 2") );
  map.put( 3, new StringBuffer("Test 3") );
  map.put( 126, new StringBuffer("Test 126") );
  map.put( 127, new StringBuffer("Test 127") );
  map.put( 128, new StringBuffer("Test 128") );

  System.out.println(map);

  System.gc();

  System.out.println(map);
 }

You should get something like this:

 {128=Test 128, 126=Test 126, 127=Test 127, 3=Test 3, 2=Test 2, 1=Test 1}
 {126=Test 126, 127=Test 127, 3=Test 3, 2=Test 2, 1=Test 1}

Where the second line shows that all elements less than 128 have been retained.

This also applies to the other wrapper classes, java.lang.Byte, java.lang.Short, java.lang.Long, etc.

You can avoid this problem by not using autoboxing, and always invoking the new operator to create your keys. However, this seems to me to be error-prone. A more fool-proof strategy would be to wrap your Integer (or even a int!) in a custom class to avoid all this froo-fra.

Update: my google-foo found an example in "Pro Java Programming" (by Brett Spell), of how to not use WeakHashMap. His examples work by accident because the values he uses for the key are larger than 127.

Searching for common bugs

Google Code Search isn't anything new, but I had fun with it today to search for a bug that I ran into in my project's code base.

Its a simple error to make:

if ( s == "" )
{
   foo();
}

Correct usage would be

if ( s.equals( "" ) )
{
   foo();
}

The tricky thing is that the first (incorrect) usage will work a surprisingly large percentage of the time.

Most strings will be empty because they were initialized to "" somewhere, rather than the result of a string operation that returned an empty string:

s.substring( s.length() ) // returns an empty string

Being initialized to the constant "" means that s will be pointing to the same jvm object that will be used in the equality comparison in the first if() statement.

Anyway, here is a link to the results of a code search for == ""

google code search for == ""

Interestingly enough, some of the first hits are valid uses of == "" (at least from their comments), because the string was interned first before being used in the comparison.