7

I'm in a weird situation where have a JSON API that takes an array with strings of neighborhoods as keys and an array of strings of restaurants as values which get GSON-parsed into the Restaurant object (defined with a String for the neighborhood and a List<String> with the restaurants). The system stores that data in a map whose keys are the neighborhood names and values are a set of restaurant names in that neighborhood. Therefore, I want to implement a function that takes the input from the API, groups the values by neighborhood and concatenates the lists of restaurants.

Being constrained by Java 8, I can't use more recent constructs such as flatMapping to do everything in one line and the best solution I've found is this one, which uses an intermediate map to store a Set of List before concatenating those lists into a Set to be store as value in the final map:

public Map<String, Set<String>> parseApiEntriesIntoMap(List<Restaurant> restaurants) {
    if(restaurants == null) {
      return null;
    }
    Map<String, Set<String>> restaurantListByNeighborhood = new HashMap<>();
    // Here we group by neighborhood and concatenate the list of restaurants into a set
    Map<String, Set<List<String>>> map =
        restaurants.stream().collect(groupingBy(Restaurant::getNeighborhood,
                              Collectors.mapping(Restaurant::getRestaurantList, toSet())));
    map.forEach((n,r) -> restaurantListByNeighborhood.put(n, Sets.newHashSet(Iterables.concat(r))));

    return restaurantListByNeighborhood;
  }

I feel like there has to be a way do get rid of that intermediate map and do everything in one line...does someone have a better solution that would allow me to do this?

Naman
  • 27,789
  • 26
  • 218
  • 353
creposukre
  • 73
  • 4
  • 2
    Note that neither "one line" nor "using streams" means "better". There is nothing wrong with an old-school loop. Also, if `getRestaurantList()` returns the names of individual restaurants of a restaurant chain, it seems overkill, since it would be exceedingly rare to find more than 1 instance of a chain in a given neighbourhood, and thus a better model would be the have a `chain` attribute on `Restaurant` is you really care and simply have each instance of a `Restaurant` have its own entry. If not, then what does it mean? Should `Restaurant` be renamed `RestaurantChain` for clarity? – Bohemian Nov 18 '19 at 01:17

2 Answers2

3

You could with Java-8 simply use toMap with a mergeFunction defined as:

public Map<String, Set<String>> parseApiEntriesIntoMap(List<Restaurant> restaurants) {
    // read below about the null check
    return restaurants.stream()
            .collect(Collectors.toMap(Restaurant::getNeighborhood,
                    r -> new HashSet<>(r.getRestaurantList()), (set1, set2) -> {
                        set1.addAll(set2);
                        return set1;
                    }));
}

Apart from which, one should ensure that the check and the result from the first block of code from your method

if(restaurants == null) {
  return null;
}

when on the other hand dealing with empty Collections and Map, it should be redundant as the above code would return empty Map for an empty List by the nature of stream and collect operation itself.

Note: Further, if you may require a much relatable code to flatMapping in your future upgrades, you can use the implementation provided in this answer.


Or a solution without using streams, in this case, would look similar to the approach using Map.merge. It would use a similar BiFunction as:

public Map<String, Set<String>> parseApiEntriesIntoMap(List<Restaurant> restaurants) {
    Map<String, Set<String>> restaurantListByNeighborhood = new HashMap<>();
    for (Restaurant restaurant : restaurants) {
        restaurantListByNeighborhood.merge(restaurant.getNeighborhood(),
                new HashSet<>(restaurant.getRestaurantList()),
                (strings, strings2) -> {
                    strings.addAll(strings2);
                    return strings;
                });
    }
    return restaurantListByNeighborhood;
}
Naman
  • 27,789
  • 26
  • 218
  • 353
  • 1
    Thanks for your answer! I didn't realize I could use `Map.merge` also to group by neighborhood and that the null check wasn't necessary. – creposukre Nov 18 '19 at 03:44
  • @creposukre The null check wouldn't be necessary when the `List` passed to the method is ensured to be either empty or filled with values, which shall be a general practice while using Collections (saves one from unwanted null checks and NPEs.) – Naman Nov 18 '19 at 04:01
  • 2
    I’d rather use `restaurantListByNeighborhood.computeIfAbsent(restaurant.getNeighborhood(), key -> new HashSet<>()).addAll(restaurant.getRestaurantList());` in the loop… and since you mentioned the `flatMapping`, it would have been nice to also mention the complete solution, i.e. `restaurants.stream().collect(groupingBy(Restaurant::getNeighborhood, flatMapping(r -> r.getRestaurantList().stream(), toSet())));`, as I did see it in this Q&A yet. – Holger Nov 18 '19 at 09:26
2

You can also flatten the Set<List<String>> after collecting them using Collectors.collectingAndThen

Map<String, Set<String>> res1 = list.stream()
            .collect(Collectors.groupingBy(Restaurant::getNeighborhood,
            Collectors.mapping(Restaurant::getRestaurantList, 
                    Collectors.collectingAndThen(Collectors.toSet(), 
                            set->set.stream().flatMap(List::stream).collect(Collectors.toSet())))));
Ryuzaki L
  • 37,302
  • 12
  • 68
  • 98
  • Why would you introduce `joining` here? That would end up joining all the elements of one collection into a single string and then add other such occurrences into a final Set. – Naman Nov 18 '19 at 03:07
  • `Sets.newHashSet(Iterables.concat(r))` flattens the `Set>` to `Set` but not by joining the Strings of the List within. e.g. `Set.of(List.of("one", "two"), List.of("three","four"))` would turn into `Set.of("one", "two","three","four")` and *not* something like `Set.of("one two", "three four")`. – Naman Nov 18 '19 at 03:15
  • Yes, I meant flattening the `Set>` into a `Set` with the elements of the lists within the set being extracted out of that list and becoming elements of the Set – creposukre Nov 18 '19 at 03:24
  • 1
    Nice! This is the solution I was looking for, thank you! I was thinking of using `Collectors.collectingAndThen` but couldn't wrap my head around how to use it to achieve the desired outcome – creposukre Nov 18 '19 at 03:42