Sonarqube is giving HashMap initalization code compliant issue

Viewed 157

I have this code where I am initializing this HashMap like:

final HashMap<String, String> propertiesValueMap = new HashMap<String, String>(){{
            put(SOME_STRING, SOME_STRING);
            put(SOME_STRING, SOME_STRING);
            put(SOME_STRING, SOME_STRING);
         }};

The above code is giving non complaint code issue in SonarQube. is there any other way by which i can initialize?

I know one way like

Stream.of(new String[][]{
            { SOME_STRING, SOME_STRING},{SOME_STRING, SOME_STRING}
    }).collect(Collectors.toMap(data->data[0],data->data[1]
    ));

But this will return me Map<String,String> and I want HashMap<String,String>.

3 Answers

As Sonarsource saying:

Double Brace Initialization (DBI) is so obscure...

Double Brace Initialization should not be used

You could do something like

Map<String, String> propertiesValueMap = createMap();
 
private static Map<String, String> createMap() {
    Map<String, String> myMap = new HashMap<String, String>();
    myMap.put(SOME_STRING, SOME_STRING)
    return myMap;
}

Or, if you are creating an immutable Map and using Java 9 :

import static java.util.Map.entry;    

Map<String, String> propertiesValueMap = Map.ofEntries(
    entry("SOME_STRING", "SOME_STRING"),
    entry("SOME_STRING2", "SOME_STRING2")
);

This issue arose because you are writing your code against concrete implementations. Which doesn't bays you anything. It only makes your code inflexible, more difficult to maintain, more verbose and less expressive. Consider reading this question What does it mean to "program to an interface"?.

In regard to Collectors.toMap() (which expects only two arguments) currently, it does return you a HashMap. And it is not specified, because sometime in the future it could be replaced with an enhanced general purpose implementation that will better in some cases than HashMap. If you would write your code against Map interface instead of requesting a particular implementation, you would get an implementation for free without changing the code.

As I've mentioned, you can request a particular implementation of the Map interface by using a flavor of toMap() that expects a supplier mapFactory.

Collector<T,?,M> toMap(Function<? super T,? extends K> keyMapper,
                       Function<? super T,? extends U> valueMapper,
                       BinaryOperator<U> mergeFunction,
                       Supplier<M> mapFactory)

But my suggestion is to use it when need a special purpose implementation like LinkedHashMap, TreeMap, WeakedHashMap, etc. The best current general purpose implementation (which currently a HashMap) will be provided to you by default, just write your code against interfaces, and you'll be fine.

An example of usage of this flavor of toMap:

Map<String, String> result =
    list.stream()
          .collect(Collectors.toMap(
              data->data[0],
              data->data[1],
              (left, right) -> { throw new AssertionError(); }, // this argument is meant to resolve duplicates and because all keys are expected to be unique it'll trows an error if duplicate will occure
              LinkedHashMap::new));

In regard to new HashMap<String, String>(){{ someCode }};, I suggest you to utilize Java 9 static methods of the Map interface Map.of() or Map.ofEntries() instead. It makes your code leaner and saves from disadvantages that double brace initialization can bring, like possibility of memory leaks, which might emerge because the anonymous inner class that it creates under the hood would hold reference to the instance of the enclosing class. That's why this practice is considered to be an antipattern.

Starting with Java 9, the easiest way to define a fixed Map<K,V> is to just use one of the Map.of(...) variants (or ofEntries if you need more than 10 entries).

Map<String, String> myMap = Map.of(
    KEY1, VALUE1,
    KEY2, VALUE2);

That one returns a non-specific Map<K,V> though (i.e. the exact type is not guaranteed, but some properties of the result are), so if you need a HashMap then the easiest way is to just pass the result of the above to new HashMap<>():

HashMap<String, String> myMap = new HashMap<>(Map.of(
    KEY1, VALUE1,
    KEY2, VALUE2));

While this does create two maps (one of which will quickly be garbage-collected), I consider this an acceptable drawback, given that you avoid creating a whole new class just for initializing some values.

Related