Take the 2-minute tour ×
Programmers Stack Exchange is a question and answer site for professional programmers interested in conceptual questions about software development. It's 100% free, no registration required.

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?

share|improve this question
    
Why are you putting that into a HashMap in the first place? Why not an array of 26 Strings? –  MichaelT Jan 1 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? –  Ali Jan 1 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. –  MichaelT Jan 1 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. –  Ali Jan 1 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". –  Ali Jan 1 at 4:44

4 Answers 4

up vote 3 down vote accepted

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();
share|improve this answer

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)

share|improve this answer

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', "..-.");
      // ...
    }
  });
}
share|improve this answer

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).

share|improve this answer
    
Can you even write newMap with parameter packs? –  VF1 Jan 1 at 9:38
1  
@VF1 You can, if you define it as a Generic function with varargs. –  Kilian Foth Jan 1 at 10:14

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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