Win a copy of Kotlin in Action this week in the Kotlin forum!
  • Post Reply Bookmark Topic Watch Topic
  • New Topic

Need help with a map  RSS feed

 
Johnny Dey
Greenhorn
Posts: 19
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
I have a method that accepts a map that has int as keys and string as values. the keys and values of the input map has 1 to many relationship. for example the map could have the following elements (keys & values separated by a semi colon) {43:"Marty", 78:"Perkins", 56:"Perkins", 67: "Andrew", 45:"Bill", 12: "Marty"}. my method returns a reverse map which has the keys of the original map as a set of values and values as keys (Defined as Map<Integer, Set<Integer>>). the returned map should be like this, for example, {"Marty":{12,43}, "Perkins":{78,56}, "Andrew":{45}, Bill:{12}}. I have following code.


the problem is that the output shows an empty set. I understands its because of set.clear() method. However, if I remove that line then all the keys are stored every set of every element. How can I get the desired output. What am I missing here? Please help.
 
Jesper de Jong
Java Cowboy
Sheriff
Posts: 16028
87
Android IntelliJ IDE Java Scala Spring
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
What goes wrong in your code is this: You have only one Set, and you are storing the exact same Set object as the value of every entry in the Map.

Collections such as Map store values by reference - when you put a Set in the Map, it is not going to create a copy of the Set, it just stores a reference to the Set. You are re-using the same Set object for every entry. When you call clear() on the Set, then the Set will be made empty, and since there's only one Set object you'll see that it is empty for every entry in the map.

What you should do to fix this: Don't create just one Set. Create a new Set object inside the loop, instead of outside the loop, so that a new Set object is created for every Map entry. Remove line 14 (set.clear()), you do not want to remove the content of the Set object after you've filled it.

In short: Move line 3 to between lines 4 and 5. Remove line 14. Line 12 (map.put(s, set)) does not need to be inside the inner loop. You can move it to after line 13.
 
Johnny Dey
Greenhorn
Posts: 19
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Jesper de Jong wrote:What goes wrong in your code is this: You have only one Set, and you are storing the exact same Set object as the value of every entry in the Map.

Collections such as Map store values by reference - when you put a Set in the Map, it is not going to create a copy of the Set, it just stores a reference to the Set. You are re-using the same Set object for every entry. When you call clear() on the Set, then the Set will be made empty, and since there's only one Set object you'll see that it is empty for every entry in the map.

What you should do to fix this: Don't create just one Set. Create a new Set object inside the loop, instead of outside the loop, so that a new Set object is created for every Map entry. Remove line 14 (set.clear()), you do not want to remove the content of the Set object after you've filled it.

In short: Move line 3 to between lines 4 and 5. Remove line 14. Line 12 (map.put(s, set)) does not need to be inside the inner loop. You can move it to after line 13.


Thank you for your kind reply. As per your suggestion I changed my code and it now looks the following


It produces the following output
Sue:[81]
Marty:[42]
Dave:[31]
Ed:[29]

However I wanted multiple set elements but it is saving only one element for each set. If I use an anonymous set, I cant use the add method to add the elements of the set in the put method of the map.
 
Tobias Bachert
Ranch Hand
Posts: 85
18
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
The problem is, that you are currently replacing already existing sets instead of adding values to these sets (additionally your inner loop does not what I guess you think it does (it only adds 'i' to set)).

Solution that is near to your version:

If you are stuck with a Java version < 8, then the following would be the re-written version with entrySet instead of keySet:

Otherwise this could be reduced to result.computeIfAbsent(...).add(entry.getKey()) but I would rather use the Collectors API as I think it states more explicit what it does:
 
Johnny Dey
Greenhorn
Posts: 19
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
Tobias Bachert wrote:The problem is, that you are currently replacing already existing sets instead of adding values to these sets (additionally your inner loop does not what I guess you think it does (it only adds 'i' to set)).

Solution that is near to your version:

If you are stuck with a Java version < 8, then the following would be the re-written version with entrySet instead of keySet:

Otherwise this could be reduced to result.computeIfAbsent(...).add(entry.getKey()) but I would rather use the Collectors API as I think it states more explicit what it does:


Thank you so much for this elegant solution. It works as expected. I had been stuck on this problem for a whole day. I am new to Java and just starting to learn the collections framework from a book. So, I don't have that much depth in this topic.
 
Johnny Dey
Greenhorn
Posts: 19
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
One more question though. Why did you use Integer instead of int in the for loop? Excuse my lack of knowledge,but  doesn't Java autobox primitives into corresponding wrapper classes?
 
Knute Snortum
Sheriff
Posts: 4073
112
Chrome Eclipse IDE Java Postgres Database VI Editor
  • Likes 1
  • Mark post as helpful
  • send pies
  • Quote
  • Report post to moderator
@Tobias: Thank you for your contribution.  You should know that in the Beginning Java forum we don't post complete solutions, but rather try to guide the OP to the answer.  The goal is to allow the OP to discover their own solutions, if possible.
 
  • Post Reply Bookmark Topic Watch Topic
  • New Topic
Boost this thread!