Search code examples
javadouble-brace-initialize

How do I fix these double braces

Hello I am trying to tidy up some code and I have stumbled across some double braces. I realize that this is a sin and that I should fix this. However, I have no idea where to begin. Can anyone help? 

private SelectItem notifyTypeItem = new SelectItem();

notifyTypeItem.setTitle("Default Notification");
    notifyTypeItem.setWidth("100%");
    notifyTypeItem.setValueMap(new LinkedHashMap<String, String>() {{
                   put("0", "None");
                   put("1", "Subtle");
                   put("2", "Intrusive");
               }}
    );
Solution

  • To understand how to fix it, you should first understand what it's doing. For this, you'll need to know about two things:

    1. Anonymous sub-classes
    2. Instance initializer blocks

    The TL;DR of how to fix it is just to split those put calls and initialization out from the setter:

    Map<String, String> valueMap = new LinkedHashMap<String, String>();
valueMap.put("0", "None");
valueMap.put("1", "Subtle");
valueMap.put("2", "Intrusive");
notifyTypeItem.setValueMap(valueMap);

    Read on for an explanation of what's happening and why it can be a bad approach.

    Anonymous Sub-classes

    An anonymous class is generally just a class without a name. For example, you can create anonymous instances of an interface like Runnable:

    Runnable r = new Runnable() {
    @Override
    public void run() {
        // Do something
    }
};
r.run(); // Does that something

    Similarly, you can create anonymous instances of abstract and concrete classes too. For example, it's very common to create an anonymous instance of ThreadLocal:

    private static ThreadLocal<SimpleDateFormat> localIsoDateFormat = new ThreadLocal<SimpleDateFormat>() {

    @Override
    protected SimpleDateFormat initialValue() {
        return new SimpleDateFormat("yyyy-MM-dd");
    }
}

    This is useful when you don't need a full, dedicated class to override a method or two, similar to creating anonymous instances of interfaces with only one method.

    Instance Initializer Blocks

    An instance initializer block allows you to execute initializers outside of your constructor. For example:

    public class MyClass {
    
    private final String s;
    {
        s = "My Class String";
    }

    public String getS() { return s; }
}

    It's essentially a stand-in for a constructor, and it's generally unnecessary, so you rarely see it. It can almost always be moved to a constructor instead.

    Combining Them

    Your example combines them. It's creating an anonymous subclass of LinkedHashMap, then it's also using an initializer block. More properly formatted, your code is:

    Map<String, String> map = new LinkedHashMap<>() {
    {
        put("0", "None");
        put("1", "Subtle");
        put("2", "Intrusive");
    }
};

    It's an anonymous instance of LinkedHashMap with an instance initializer block that does the put calls.

    Why is it bad?

    For the same reason that you need to be careful creating anonymous classes: references to the enclosing class instance.

    Anonymous classes are notorious for being the source of memory leaks in your application. Your code appears to be in a non-static context. This means that the anonymous LinkedHashMap subclass you create will have an implicit reference to the class that your method is sitting in. For example, if your method is in MyClass:

    public class MyClass {
    private SelectItem notifyTypeItem = new SelectItem();

    public void foo() {
        notifyTypeItem.setTitle("Default Notification");
        notifyTypeItem.setWidth("100%");
        notifyTypeItem.setValueMap(new LinkedHashMap<String, String>() {{
                   put("0", "None");
                   put("1", "Subtle");
                   put("2", "Intrusive");
               }}
        );
    }
}

    The newly created LinkedHashMap subclass (MyClass$1 will be the "class name", if you can call it such a thing) will have a reference to the enclosing MyClass instance. In some cases, this may be fine. But if you're creating the notifyTypeItem with the intention of passing it to something else and throwing away your MyClass instance, then you'll be creating a memory leak in your application. The MyClass instance will be referenced by the MyClass$1 instance, and the SelectItem will reference the MyClass$1 instance, so the MyClass instance will never be garbage collected until the SelectItem instance is no longer referenced. If MyClass has several other references besides the SelectItem, then that will only increase the total memory a single MyClass instance consumes, and lead to more memory leak issues.

    References

    Here are some relevant links: