Storing a HashMap inside another HashMap and improving performance

Question

I am supposed to created a HashMap inside another HashMap as shown below which can store the value inside the inner HashMap based on the key of the outer HashMap at the runtime

i.e. required output for program should be of the format

   { 1 = {11 = "aaa",15 = "bbb"}, 2 = {13 = "ccc", 14 = "ddd"} }

where 1,2 are Key values for Outer HashMap.

Below is the Code provided for it Is there any better approach to improve performance

HashMap<Integer, HashMap<Integer, String>>Outer 
                   = new HashMap<Integer, HashMap<Integer,String>>();

    BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
    int count = Integer.parseInt(br.readLine());
    for(int i =0;i<count;i++)
    {
        String input[] = br.readLine().split("\\s");

        //HashMap<Integer,String>inner = new HashMap<Integer, String>();
        int key = Integer.parseInt(input[0]);
        if(Outer.isEmpty() || !Outer.containsKey(key))
        {
            HashMap<Integer, String> inner = new HashMap<Integer, String>();
            inner.put(Integer.parseInt(input[1]),input[2]);
            Outer.put(key, inner);
        }
        else if(Outer.containsKey(key))
            {
                HashMap<Integer, String> inner = (HashMap<Integer, String>) Outer.get(key).clone();
                inner.put(Integer.parseInt(input[1]), input[2]);
                Outer.put(key, inner);
            }
    }

Answer 1

Similar to Vadim's answer, but further improved - as it doesn't require a call to both containsKey as well as get:

Map<Integer, Map<Integer, String>> outer = new HashMap<Integer, Map<Integer, String>>();
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
int count = Integer.parseInt(br.readLine());

Pattern splitter = Pattern.compile("\\s");

for(int i = 0; i < count; i++){
    String input[] = splitter.split(br.readLine());

    int key = Integer.parseInt(input[0]);

    Map<Integer, String> inner = outer.get(key);
    if(inner == null){
        inner = new HashMap<Integer, String>();
        outer.put(key, inner);
    }
    inner.put(Integer.parseInt(input[1]), input[2]);
}

It also has some minor improvements for naming conventions, and use of the Collections interfaces instead of concrete types.

I also removed the call to clone. This could be a slight savings - and I don't think it would have given you your expected results.

Finally - one other thing that I changed that could be a slight improvement is using a pre-compiled Pattern for the splitting of your String into fields.

Answer 2

Optimizing is nearly always a bad idea. Especially in Java where the JVM is quite good at doing that by itself.

Do you really need a Map<Integer, Map<Integer, String>>, it seems to me that you really just need a Map<Pair, String> where

public final class Pair {
  private final int x;
  private final int y;
  public Pair(int x, int y) { this.x = x; this.y = y;}
}

I am not claiming at all that this will improve performance, but it might be better design. I don't quite know what you are doing, so maybe it's not better design.

Answer 3

Your code is good enough from performance point of view. Only few things came to my mind. If/else condition can be simplified and you don't need to clone map in else part (work with pointer)

HashMap<Integer, HashMap<Integer, String>>Outer = new HashMap<Integer,   HashMap<Integer,String>>();
BufferedReader br = new BufferedReader(new InputStreamReader(System.in));
int count = Integer.parseInt(br.readLine());
for(int i =0;i<count;i++)
{
    String input[] = br.readLine().split("\\s");

    //HashMap<Integer,String>inner = new HashMap<Integer, String>();
    int key = Integer.parseInt(input[0]);
    if(!Outer.containsKey(key))
    {
        HashMap<Integer, String> inner = new HashMap<Integer, String>();
        inner.put(Integer.parseInt(input[1]),input[2]);
        Outer.put(key, inner);
    }
    else
    {
        HashMap<Integer, String> inner = Outer.get(key);
        inner.put(Integer.parseInt(input[1]), input[2]);
    }
}