Refactoring Java 8 stream code

Question

I have implemented the following code using Java 8.

Map<String, String> coMap = getHashMap(); String newCoName = coMap.entrySet() .stream() .filter(coEntry -> coEntry.getValue().equals(newcoId)) .map(coEntry -> coEntry.getKey()) .collect(Collectors.joining()); String oldCoName = coMap.entrySet() .stream() .filter(coEntry -> coEntry.getValue().equals(oldcoId)) .map(coEntry -> coEntry.getKey()) .collect(Collectors.joining());

Now. I want to know any better way of doing this instead of repeating the same lines of code twice.

put the code in a method passing the coId as argument: getCoName(String coId) — Pau
– Pau, Commented Dec 22, 2016 at 9:21

Eran · Accepted Answer · 2016-12-22 10:03:58Z

A bigger issue than repeating the same code twice is executing the same code twice.

It would be more efficient to run a single Stream pipeline to produce your output :

Map<String,String> keysByValue = coMap.entrySet() .stream() .collect(Collectors.groupingBy(Map.Entry::getValue, Collectors.mapping(Map.Entry::getKey, Collectors.joining())));

This would give you for each value of your original Map (not only the two values your original code is searching for), the joint keys having that value.

Then you can extract from the Map the data you need :

String newCoName = keysByValue.get(newcoId); String oldCoName = keysByValue.get(oldcoId);

Sample input and output :

Map<String,String> coMap = new HashMap<> (); coMap.put("a","foo"); coMap.put("b","foo"); coMap.put("c","bar"); coMap.put("d","bar"); Map<String,String> keysByValue = ... // same as the code above String newValueKeys = keysByValue.get("foo"); String oldValueKeys = keysByValue.get("bar"); System.out.println (newValueKeys); System.out.println (oldValueKeys);

Output :

ab cd

It’s not clear whether there can be other values, besides newcoId and oldcoId. If so, a filter step letting only these two values pass can avoid doing unnecessary work. If we assume that there are only the two values (or using the filter step), we could also use partitioningBy instead of groupingBy.
@Holger I figured it would be useful to produce a Map with all the values of the original Map as keys, but that would depend on the actual usage - i.e. whether we can produce this Map once and keep re-using it for different pairs of values, or we have to produce it every time the original Map is updated.

baao · Accepted Answer · 2016-12-22 09:21:42Z

Since the whole difference is an id, a simple method does that for you.

String getName(int id) { // supposed id is an integer return coMap.entrySet() .stream() .filter(coEntry -> coEntry.getValue().equals(id)) .map(coEntry -> coEntry.getKey()) .collect(Collectors.joining()); }

Viet · Accepted Answer · 2016-12-22 09:42:00Z

Other way uses with FunctionalInterface Predicate, your condition filter will be dynamic

public static Predicate<Map.Entry> getPredicate(String col) { return p -> p.getValue().equals(col); } public static String getName(HashMap<String, String> coMap, Predicate<Map.Entry> predicate) { return coMap.entrySet() .stream() .filter(predicate) .map(coEntry -> coEntry.getKey()) .collect(Collectors.joining()); }

Calling in your code:

 getName(coMap, getPredicate(newcoId)); getName(coMap, getPredicate(oldcoId));

Peter Gelderbloem · Accepted Answer · 2016-12-22 09:22:08Z

most IDE's can refactor it for you using "Extract Method" refactoring: http://refactoring.com/catalog/extractMethod.html or you can do it by hand

Harmlezz · Accepted Answer · 2016-12-22 11:36:37Z

0

You may use this helper method:

public static String join(Map<String, String> map, String value) { return map.keySet().stream() .filter(key -> Objects.equals(map.get(key), value)) .collect(Collectors.joining()); }

Here some example code using the method:

 Map<String, String> coMap = new HashMap<>(); coMap.put("A", null); coMap.put("B", "V"); coMap.put("C", "V"); coMap.put("D", "Z"); coMap.put("E", "Z"); coMap.put("F", null); System.out.println("Vs: " + join(coMap, "V")); System.out.println("Zs: " + join(coMap, "Z"));

And here the output:

Vs: BC Zs: DE

edited Dec 22, 2016 at 11:36

answered Dec 22, 2016 at 9:25

Harmlezz

8,08829 silver badges37 bronze badges

9 Comments

Tom Over a year ago

Is there a reason for removing .map(coEntry -> coEntry.getKey())?

Harmlezz Over a year ago

You are interessted in the keys right? Hence this algorithm saves you from mapping your Map.Entrys to keys. Thats all. I added null-safe comparison in case the value is null.

Tom Over a year ago

"You are interessted in the keys right?" No, I'm not.

Harmlezz Over a year ago

But you are finally joining the keys with .collect(Collectors.joining()) to a String

Holger Over a year ago

Well, streaming over the keys only, saves you from the cheap Entry.getKey() call and introduces an expensive map.get call. The dealing with Optional makes the code even worse. You can simply use Objects.equals to check two objects for equality in a null-safe way.

|

Collectives™ on Stack Overflow

Refactoring Java 8 stream code

5 Answers 5

2 Comments

Comments

Comments

Comments

9 Comments

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

2 Comments

Comments

Comments

Comments

9 Comments

Related