views:

457

answers:

6

I have some static initializer code:

someMethodThatTakesAHashMap(new HashMap<K, V>() {
{
  put("a","value-a"); 
  put("c","value-c");}
});

For some reason I am receiving a warning from Eclipse: The serializable class does not declare a static final serialVersionUID.

Is this complaining about the anonymous class? What can I do about that, or should I just suppress it.

+5  A: 

Yes, you could suppress the warning, but I'd rewrite it like this:

HashMap<String, String> map  = new HashMap<String, String>();
map.put("a","value-a"); 
map.put("c","value-c");
someMethodThatTakesAHashMap(map);

No suppressing needed, and much better to read, IMO.

Bart Kiers
Is the anonymous class way seen as a hack in the Java community? Is it a non-standard way for people to code something like this? In C# it is standard, but when in Rome :)
Alex Baranosky
I came to feel it's a hack for Map and List initialization after asking this related question: http://stackoverflow.com/questions/924285/efficiency-of-java-double-brace-initialization. People really would love a clean Java syntax for this, ala Ruby or Scala or Groovy, and the "double brace initializer approach" kind of gets you there, albeit with a lot of extra junk in your classpath.
Jim Ferrans
@GordonG: I don't see it used much in Java. But I wouldn't go as far as calling is a 'hack'. I don't know C#, but in the C# version does the class (HashMap in your example) also get extended? Or has C# a more "friendlier/easier" way to do this?
Bart Kiers
A: 

I generally agree with Bart K., but for informational purposes:
The warning can also be eliminated by adding the field, which can be automatically generated by hitting ctrl+1.
The warning can also be suppressed by adding the @SuppressWarnings("serial") annotation before the definition.
The anonymous class implements Serializeable, and Serializeable requires this static field so that versions can be distinguished when serializing and de-serializing. More information here:
http://www.javablogging.com/what-is-serialversionuid/

T.R.
+7  A: 

The syntax you're using is called double-brace initialization - which is actually an "instance initialization block that is part of an anonymous inner class" (certainly not a hack). So, when using this notation, you are actually defining a new class(!).

The "problem" in your case is that HashMap implements Serializable. This interface doesn't have any methods and serves only to identify the semantics of being serializable. In other words, it's a marker interface and you concretely don't have to implement anything. But, during deserialization, Java uses a version number called a serialVersionUID to verify that the serialized version is compatible with the target. If you don't provide this serialVersionUID, it will be calculated. And, as documented in the javadoc of Serializable, the calculated value is extremely sensitive and it is thus recommended be explicitly declare it to avoid any deserialization problems. And this is what Eclipse is "complaining" about (note that this is just a warning).

So, to avoid this warning, you could add a serialVersionUID to your annonymous inner class:

someMethodThatTakesAHashMap(new HashMap<String, String>() {
    private static final long serialVersionUID = -1113582265865921787L;

    {
        put("a", "value-a");
        put("c", "value-c");
    }
});

But you loose the conciseness of the syntax (and you may not even need it).

Another option would thus be to ignore the warning by adding a @SuppressWarnings("serial") to the method where you are calling someMethodThatTakesAHashMap(Map). This seems more appropriate in your case.

That all being said, while this syntax is concise, it has some drawbacks. First, if you hold a reference on the object initialized using a double-brace initialization, you implicitly hold a reference to the outer object which won't be eligible for garbage collection. So be careful. Second (this sounds like micro optimization though), double-brace initialization has a very a little bit of overhead. Third, this technique actually uses anonymous inner classes as we saw and thus eats a bit of permgen space (but I doubt that this is really a problem unless you really abuse them). Finally - and this is maybe the most important point - I am not sure it makes the code more readable (it's not a well known syntax).

So, while I like to use it in tests (for the conciseness), I tend to avoid using it in "regular" code.

Pascal Thivent
1) "note that this is just a warning" <- Reminds me of a joke about programmers.2) I've always wondered how container.add(new Button("text").setWidth(5)); was actually done, now I know, and I think it's pretty useful for situations like the one I just mentioned, i.e.: setting one property.3)How do you decide what to use for the version UID?
Leo Jweda
@Laith 1) The fact is that if you're not going to serialize the created collection, then it's really a problem 2) Not sure what you're showing here (its not a double-brace initialization nor an anonymous inner class) but there are good use-cases indeed (e.g. when using JMock) 3) It's an arbitrary decision (I often use the default calculated value).
Pascal Thivent
@Pascal 2) I've always tried doing it that way, it never worked, now I know it can be done with double braces. 3) And how do you get that value?
Leo Jweda
@Laith with `serialver` http://java.sun.com/javase/6/docs/technotes/tools/solaris/serialver.html but any decent IDE will do that for you.
Pascal Thivent
@Pascal: How can I do that in Eclipse?
Leo Jweda
@Laith this is available in the Quick Fix options (Ctrl+1 on the problem or click on the warning symbol in the left bar): *Add generated serial version ID*.
Pascal Thivent
@Pascal: Thanks :)
Leo Jweda
A: 

The ImmutableMap class from the Google Collections library is useful for this situation. e.g.

someMethodThatTakesAHashMap(ImmutableMap.<K, V>builder().put("a","value-a").put("c","value-c").build());

or

someMethodThatTakesAHashMap(ImmutableMap.of("a","value-a","c","value-c"));
finnw
A: 

To address the other half of your question, "should I suppress it?" --

Yes. In my opinion, this is a terrible warning. serialVersionUID should by default not be used, not the other way around.

If you don't add serialVersionUID, the worst thing that happens is that two versions of an object that are actually serialization-compatible are deemed incompatible. serialVersionUID is a way to declare that the serialization compatibility has not changed, overriding Java's default assessment.

Using serialVersionUID, the worst thing that happens is that you inadvertently fail to update the ID when the class's serialized form changes in an incompatible way. At best, you also get a runtime error. At worst, something worse happens. And imagine how easy it is to fail to update it.

Sean Owen
A: 

Your intent was to initialize an anonymous instance of HashMap. The warning is clue that your code is doing more than you intended.

What we're looking for is a way to initialize an anonymous HashMap instance. What we have above creates an anonymous subclass of HashMap then creates an anonymous instance of that anonymous class.

Because the code does more than was intended, I'd call it a hack.

What we really want is something like this:

foo(new HashMap<String, String>({"a", "value-a"}, {"c", "value-c"}));

But alas this isn't valid Java. There isn't a way to do anything this in a type-safe way using an array of key/value pairs. Java simple doesn't have the expressive power.

The Google Collection's ImmutableMap.of static methods are close but it means creating a version of the factory method for various numbers of key/value pairs. (See finnw's answer.)

So keep things simple. Go with Bart K's solution unless your code is littered with this initialization. If so use ImmutableMap. Or roll your own HashMap subclass with the "of" style factory methods. Or create these "of" style factory methods in a utility class. Here's one for two key/value pairs:

public final MapUtil {
    public static <K,V> Map<K,V> makeMap(K k1, V v1, K k2, V v2) {
        Map<K,V> m = new HashMap<K,V>();
        m.put(k1, v1);
        m.put(k2, v2);
        return m;
    }
}

Embrace the verbosity and take solace in the knowledge your corporate co-workers are wearing the same shackles as you.

rickmode