Code Review Asked by Another Noob on October 27, 2021
Hello dear colleagues.
As an exercise for Java streams, I’ve written simple program, which scrapes links to references from a news portal. Basically, I wanted to find out, which portals are the most referenced.
Right now, I ended up with the following piece of code:
final Map<String, List<URL>> hostToURLs = Analyzer.mapByHost(ReferencesStore.read());
// E.g. bbc.com -> [ https://www.bbc.com/article1, https://www.bbc.com/article2, etc ]
// The following creates LinkedHashMap sorted by the number of URLs
final LinkedHashMap<String, List<URL>> sortedHostToURLs = hostToURLs
.entrySet()
.stream()
.sorted(Map.Entry.comparingByValue(Comparator.comparingInt(List::size)))
.collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (m, n) -> { m.addAll(n); return m; }, LinkedHashMap::new));
I’m interested, if the the ugly merge lambda
(m,n)-> {m.addAll(n); return m;}
in the collect method can be replaced with some method reference from standard library. I wasn’t able to find anything useful in the documentation, nor was I able to ask google the right question to find out if such merge function exists. Thank you in advance.
The thing is that the merge function is never called in your case, because all you are doing is reordering an existing map, so IMO you don't really need to worry about it.
Personally in such cases I just use (m, n) -> m
, because it's short and doesn't distract.
Another variant would be to use a function that throws an exception when called. The JDK does this itself when you use the toMap
overload that doesn't take a merge function:
private static <T> BinaryOperator<T> throwingMerger() {
return (u,v) -> { throw new IllegalStateException(String.format("Duplicate key %s", u)); };
}
Another thing you may want to consider it not to use a (Linked)HashMap
at all here, but simply collect the Entry
s into a list. If you need to lookup an entry by key later, then just keep the original map around.
EDIT: In cases where the merge function is used, your implemention is fine. You may want to just put it in a variable, so that its name can describe its function.
One thing you could consider is using a function that creates a new list instead of "reusing" one of the existing lists. That would reflect the functional style of the code better, where one normally uses immutable data structures.
The basic problem is that Java's List
interface isn't intended for functional/immutable situations and doesn't have a simple method to concatinate two lists.
If you are open to additional libraries you'd have more options. For example, Apache's Commons has a union
method.
Answered by RoToRa on October 27, 2021
Get help from others!
Recent Questions
Recent Answers
© 2024 TransWikia.com. All rights reserved. Sites we Love: PCI Database, UKBizDB, Menu Kuliner, Sharing RPP