Over a million developers have joined DZone.
{{announcement.body}}
{{announcement.title}}

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

DZone's Guide to

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

· Integration Zone
Free Resource

Modernize your application architectures with microservices and APIs with best practices from this free virtual summit series. Brought to you in partnership with CA Technologies.

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

  2. new HashMap() {
    }
  3. In that anonymous class, we’re using an instance initialiser to initialize 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 double brace initialisation! 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-output

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

The Integration Zone is proudly sponsored by CA Technologies. Learn from expert microservices and API presentations at the Modernizing Application Architectures Virtual Summit Series.

Topics:

Published at DZone with permission of Lukas Eder, DZone MVB. See the original article here.

Opinions expressed by DZone contributors are their own.

THE DZONE NEWSLETTER

Dev Resources & Solutions Straight to Your Inbox

Thanks for subscribing!

Awesome! Check your inbox to verify your email so you can start receiving the latest in tech news and resources.

X

{{ parent.title || parent.header.title}}

{{ parent.tldr }}

{{ parent.urlSource.name }}