Software Development

Don’t be “Clever”: The Double Curly Braces Anti Pattern

From time to time, I find someone using the double curly braces anti pattern (also called double brace initialisation) in the wild. This time on Stack Overflow:

Map source = new HashMap(){{
    put("firstName", "John");
    put("lastName", "Smith");
    put("organizations", new HashMap(){{
        put("0", new HashMap(){{
            put("id", "1234");
        }});
        put("abc", new HashMap(){{
            put("id", "5678");
        }});
    }});
}};

In case you do not understand the syntax, it’s actually easy. There are two elements:

  1. We’re creating anonymous classes that extend HashMap by writing:
    new HashMap() {
    }
  2. In that anonymous class, we’re using an instance initialiser to initialise the new anonymous HashMap subtype instance by writing things like:
    {
        put("id", "1234");
    }

    Essentially, these initialisers are just constructor code.

So, why is this called the Double Curly Braces Anti Pattern

There are really three reasons for this to be an anti pattern:

1. Readability

This is the least important reason, it’s readability. While it may be a bit easier to write, and feel a bit more like the equivalent data structure initialisation in JSON:

{
  "firstName"     : "John"
, "lastName"      : "Smith"
, "organizations" : 
  {
    "0"   : { "id", "1234" }
  , "abc" : { "id", "5678" }
  }
}

And yes. It would be really awesome if Java had collection literals for List and Map types. Using double curly braces to emulate that is quirky and doesn’t feel quite right, syntactically.

But let’s leave the area where we discuss taste and curly braces (we’ve done that before), because:

2. One type per instance

We’re really creating one type per instance! Every time we create a new map this way, we’re also implicitly creating a new non-reusable class just for that one simple instance of a HashMap. If you’re doing this once, that might be fine. If you put this sort of code all over a huge application, you will put some unnecessary burden on your ClassLoader, which keeps references to all these class objects on your heap. Don’t believe it? Compile the above code and check out the compiler output. It will look like this:

Test$1$1$1.class
Test$1$1$2.class
Test$1$1.class
Test$1.class
Test.class

Where the Test.class is the only reasonable class here, the enclosing class.

But that’s still not the most important issue.

3. Memory leak!

The really most important issue is the problem that all anonymous classes have. They contain a reference to their enclosing instance, and that is really a killer. Let’s imagine, you put your clever HashMap initialisation into an EJB or whatever really heavy object with a well-managed lifecycle like this:

public class ReallyHeavyObject {

    // Just to illustrate...
    private int[] tonsOfValues;
    private Resource[] tonsOfResources;

    // This method almost does nothing
    public void quickHarmlessMethod() {
        Map source = new HashMap(){{
            put("firstName", "John");
            put("lastName", "Smith");
            put("organizations", new HashMap(){{
                put("0", new HashMap(){{
                    put("id", "1234");
                }});
                put("abc", new HashMap(){{
                    put("id", "5678");
                }});
            }});
        }};
        
        // Some more code here
    }
}

So this ReallyHeavyObject has tons of resources that need to be cleaned up correctly as soon as they’re garbage collected, or whatever. But that doesn’t matter for you when you’re calling the quickHarmlessMethod(), which executes in no time.

Fine.

Let’s imagine some other developer, who refactors that method to return your map, or even parts of your map:

public Map quickHarmlessMethod() {
        Map source = new HashMap(){{
            put("firstName", "John");
            put("lastName", "Smith");
            put("organizations", new HashMap(){{
                put("0", new HashMap(){{
                    put("id", "1234");
                }});
                put("abc", new HashMap(){{
                    put("id", "5678");
                }});
            }});
        }};
        
        return source;
    }

Now you’re in big big trouble! You have now inadvertently exposed all the state from ReallyHeavyObject to the outside, because each of those inner classes holds a reference to the enclosing instance, which is the ReallyHeavyObject instance. Don’t believe it? Let’s run this program:

public static void main(String[] args) throws Exception {
    Map map = new ReallyHeavyObject().quickHarmlessMethod();
    Field field = map.getClass().getDeclaredField("this$0");
    field.setAccessible(true);
    System.out.println(field.get(map).getClass());
}

This program returns:

class ReallyHeavyObject

Yes, indeed! If you still don’t believe it, you can use a debugger to introspect the returned map:

debug-output1

You will see the enclosing instance reference right there in your anonymous HashMap subtype. And all the nested anonymous HashMap subtypes also hold such a reference.

So, please, never use this anti pattern

You might say that one way to circumvent all that hassle from issue 3 is to make the quickHarmlessMethod() a static method to prevent that enclosing instance, and you’re right about that.

But the worst thing that we’ve seen in the above code is the fact that even if you know what you are doing with your map that you might be creating in a static context, the next developer might not notice that and refactor / remove static again. They might store the Map in some other singleton instance and there is literally no way to tell from the code itself that there might just be a dangling, useless reference to ReallyHeavyObject.

Inner classes are a beast. They have caused a lot of trouble and cognitive dissonance in the past. Anonymous inner classes can be even worse, because readers of such code might really be completely oblivious of the fact that they’re enclosing an outer instance and that they’re passing around this enclosed outer instance.

The conclusion is:

Don’t be clever, don’t ever use double brace initialisation

Lukas Eder

Lukas is a Java and SQL enthusiast developer. He created the Data Geekery GmbH. He is the creator of jOOQ, a comprehensive SQL library for Java, and he is blogging mostly about these three topics: Java, SQL and jOOQ.
Subscribe
Notify of
guest

This site uses Akismet to reduce spam. Learn how your comment data is processed.

2 Comments
Oldest
Newest Most Voted
Inline Feedbacks
View all comments
Sasidhar
Sasidhar
9 years ago

Thanks for the information. Can you phase explain why does this happen only with double braces initialization?

Thanks.

I am
I am
8 years ago
Reply to  Sasidhar

As explained in the beginning, a double brace is equivalent of an anonymous class which is equivalent of creating a new class itself, the more it is nested, the more the classes.

Wish there was some discussion on better alternatives and using JDK 8 lambdas.

Back to top button