0
public Map<String, List<String>> getContactMap() {
    Set<String> keys = contactMap.keySet(); //Get Key Set
    for(String key: keys){
        List<String> a=contactMap.get(key); //Get List for each Key
        a.forEach(System.out::println);     //Print the list
    }
    return null;
}

I have a Hashmap, with multiple values for a single key. I am using this code to print those values, but this does not seem to work. What am I doing wrong?

I am only getting the last inserted value as output.

This is my addContact method:

private Map<String, List<String>> contactMap = new HashMap<String, List<String>>();

@Override
public void addContact(String name, List<String> list) {
    // TODO Auto-generated method stub
    contactMap.put(name, list);
}

And this is my main():

List <String> list= new ArrayList<String>();
    list.add("0321564988");
    list.add("7891268429");
    contacts.addContact("Name1",list);
    list.clear();
    list.add("1891219122");
    list.add("9495198929");
    contacts.addContact("Name2",list);
    list.clear();
    list.add("8949219912");
    contacts.addContact("Name3",list);

This is my desired output:

Name1   Phone1
        Phone2
        Phone3
Name2   Phone1
        Phone2
        .....
4
  • If you're only getting the last inserted value, perhaps you have a bug in your insertion code. Please show how you populate contactMap. Commented Apr 21, 2015 at 7:01
  • contactMap.put(name, list); The list is okay, I checked it, during inserting.. Commented Apr 21, 2015 at 7:05
  • contactMap.put(name, list); is exactly not okay - it will replace the value each time. You need something like contactMap.computeIfAbsent(name, k -> new ArrayList<>()).add(value). Further, looping over keys and retrieving each value is an anti-pattern, use the entrySet, Commented Apr 21, 2015 at 7:07
  • 2
    Note that your printing code is a half-hearted mixture of imperative and functional programming. You can replace it with contactMap.forEach((key, a) -> a.forEach(System.out::println)); Commented Apr 21, 2015 at 9:23

3 Answers 3

2

Your code:

List <String> list= new ArrayList<String>();
list.add("0321564988");
list.add("7891268429");
contacts.addContact("Name1",list);
list.clear();
list.add("1891219122");
list.add("9495198929");
contacts.addContact("Name2",list);
list.clear();
list.add("8949219912");
contacts.addContact("Name3",list);

is wrong.

You are reusing the same List.

Look at a simple example:

List <String> list = new ArrayList<String>();
List <String> list2 = list;

list.add("0321564988");
list.add("7891268429");
list.clear();

list2.add("1891219122");
System.out.println(list2);

Output:

1891219122

Because, in Java, a List is a reference to a memory location containing that List. Java passes references (by value) so that when you pass the List to your Map you only pass a reference to that List. When you clear the List you clear the memory location and the reference in the Map still points to the same location.

I would suggest you change your method to:

public void addNumberForContact(String name, String number) {
    contactMap.computeIfAbsent(name, k -> new ArrayList<>()).add(number);
}

This way you abstract the creation of the List to a class that controls the Map so you don't have this problem.

Further, looping over a Map by key and then getting the value is an anti-pattern. A Map stores key -> value pairs together so you can use the EntrySet:

contactMap.entrySet().forEach(System.out::println); 
Sign up to request clarification or add additional context in comments.

1 Comment

Okay!! I got the issue!! Thanks a ton!! :)
1

If you only want to print the values of the values, you can do:

contactMap.entrySet().stream()
          .map(Entry::getValue)
          .flatMap(List::stream)
          .forEach(System.out::println); 

Also, if this is the only purpose of the method, it doesn't have to return anything, so change the return-type to void and remove the return statement.

4 Comments

Actually I wanted to do something like this--- Name1 Phone1 Phone2 Phone3 Name2 Phone1 Phone2 .....
Please provide some sample content and the desired output (in the question)
Shorter: contactMap.values().stream().flatMap(List::stream).forEach(System.out::println);
I don't think this is the problem, the OP's code is fine (but rather ugly) and should work if the Map is correct. I think the population of the Map is the issue.
0

The reason why you have only last list included is because of your insert method:

List <String> list= new ArrayList<String>();
    list.add("0321564988");
    list.add("7891268429");
    contacts.addContact("Name1",list);
    list.clear();
    list.add("1891219122");
    list.add("9495198929");
    contacts.addContact("Name2",list);
    list.clear();
    list.add("8949219912");
    contacts.addContact("Name3",list);

Map binds key- value pairs by referencing key (in your case String key) to object's memory address (in your case List <String>). Your map contains three different keys, but each one points to exactly the same object in memory. As a result, when you modify one Map entry, the change is valid for all keys. To correct your code you should initialize your list every time you want to add it:

List <String> list= new ArrayList<String>();
    list.add("0321564988");
    list.add("7891268429");
    contacts.addContact("Name1",list);
List <String> list1= new ArrayList<String>();
    list1.add("1891219122");
    list1.add("9495198929");
    contacts.addContact("Name2",list1);
List <String> list2= new ArrayList<String>();
    list2.add("8949219912");
    contacts.addContact("Name3",list2);

At the end it is worth pointing out, that "memory reference" is valid for elements derived from Object class. Basic types (i.e. int, boolean, double) are referenced by value, so for this sort of data your code would work fine.

Comments

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.