1

I want to add values to a HashMap, which would be used by methods in the same class. I have two solutions:

  1. Adding all the values with static
  2. When the first method is called, add the values

Solution #1:

private static Map<Character, String> codes = new HashMap<>();

static {
    codes.put('A', ".-");
    codes.put('B', "-...");
    codes.put('C', "-.-.");
    codes.put('D', "-..");
    codes.put('E', ".");
    codes.put('F', "..-.");
    // ...
}

Solution #2:

boolean methodIsCalled = false;

public static char decode(String s) {
    if(!methodIsCalled) {
        addValues();
        methodIsCalled = true;
    }
    // ...
}

private static void addValues() {
    codes.put('A', ".-");
    codes.put('B', "-...");
    codes.put('C', "-.-.");
    codes.put('D', "-..");
    codes.put('E', ".");
    codes.put('F', "..-.");
    // ...
}

Which one is the most efficient? Which one is the best practice?

  • Why are you putting that into a HashMap in the first place? Why not an array of 26 Strings? – user40980 Jan 1 '15 at 4:05
  • How does an array of 26 strings solve this problem? In particular, how to you do the mapping? You have the implicit conversion from A -> 0, B ->0, ..., in mind? – Wickoo Jan 1 '15 at 4:16
  • @Ali a final array handles immutability and initalization. A->0, B->1 does it quite nicely avoiding many of the other issues of other libraries needed, extra static blocks or builders. It's clear and reasonable. – user40980 Jan 1 '15 at 4:38
  • @MichaelT Maybe for this particular example, given that the user wants to have mapping for the morse code, an array of String of size 26 is fine, however, it is not a general solution. My answer was towards a general solution, not this particular one. In many cases, the domain is so large, we cannot use a direct mapping to integers and we need to use a hashmap. – Wickoo Jan 1 '15 at 4:41
  • 1
    @MichaelT btw, an array is only size-wise immutable, but you can change one of its elements. a[0] = "new val". – Wickoo Jan 1 '15 at 4:44
3

Your solution 1 may be problematic, as the hashmap is static and will be initialised only once, and is shared by all instances of your class. Is this your intended behavior or you want each instance have its own map? If you one only one map, I would suggest to pass the set to the constructor instead of using a static one, for example:

public class Data {
  private final Map<Character, String> codes;
  public Data(Map<Character, String> codes) { this.codes = codes}
}

Your solution 2 adds the overhead of lazy initilization of the set, each time you need it, and adds the ugly check methodIsCalled to the logic of your program. I think initilizing the map in the constructor is a better option.

public class Data {
  private final Map<Character, String> codes;
  public Data() { 
     this.codes = new HashMap<>();
     codes.put('A', ".-");
     codes.put('B', "-...");
     codes.put('C', "-.-.");
     codes.put('D', "-..");
     codes.put('E', ".");
     codes.put('F', "..-.");
  } 
}

The other question you need to answer is that if you change the values of this hashmap later or not. If you don't change it, you better look for immutable hashMaps. One option is to use Collections.unmodifiableMap(map).

You can also use Google Guava libraries to that allow you to initialize a map in one line and get an immutable map:

ImmutableMap.<Character, String>builder()
     .put('A', ".-")
     .put('B', "-...")
     .put('C', "-.-.")
     .put('D', "-..")
     .put('E', ".")
     .put('F', "..-.")
     .build();
3

Nothing can beat Guava's ImmutableMap with its optimized memory consumption, but here are a pair of pure solutions:

/* Name of the class has to be "Main" only if the class is public. */
class Ideone
{
  private static final Map<Character, String> codes1;

  static {
    Map<Character, String> temp= new HashMap<Character, String>();
    temp.put('A', ".-");
    temp.put('B', "-...");
    temp.put('C', "-.-.");
    temp.put('D', "-..");
    temp.put('E', ".");
    temp.put('F', "..-.");
    // ...
    codes1 = Collections.unmodifiableMap(temp);
  }

  private static final Map<Character, String> codes2 = Collections.unmodifiableMap(new HashMap<Character, String>() {
    {
      put('A', ".-");
      put('B', "-...");
      put('C', "-.-.");
      put('D', "-..");
      put('E', ".");
      put('F', "..-.");
      // ...
    }
  });
}
0

If you don't insist on lazy initialization (and for a small, non-growing map of 26 items you shouldn't), then why not optimize for readability instead? I would always use something like

private static Map<Character, String> codes = newMap(
  'A', ".-",
  'B', "-...",
  'C', "-.-.",
  ...
);

(with a suitably defined helper function newMap).

  • Can you even write newMap with parameter packs? – VF1 Jan 1 '15 at 9:38
  • 1
    @VF1 You can, if you define it as a Generic function with varargs. – Kilian Foth Jan 1 '15 at 10:14
0

In these cases, I suppose question is not about efficient method - but rather when do you actually need that map initialized and ready..

in the case of static initialization - by the time class is loaded

your lazy loading approach may be necessary if you need to set 'huge' map, and say often those values come from an expensive source (i.e across network), even then you may not need an external flag.

Collection.isEmpty() would tell you if that has already been initialized or not (provided, of course at least one value would be there to be initialized)

protected by gnat Mar 24 '18 at 8:15

Thank you for your interest in this question. Because it has attracted low-quality or spam answers that had to be removed, posting an answer now requires 10 reputation on this site (the association bonus does not count).

Would you like to answer one of these unanswered questions instead?

Not the answer you're looking for? Browse other questions tagged or ask your own question.