1

I have a method that adds maps to a cache and I was wondering what I could do more to simplify this loop with Java 8.

What I have done so far:
Standard looping we all know:

for(int i = 0; i < catalogNames.size(); i++){
    List<GenericCatalog> list = DummyData.getCatalog(catalogNames.get(i));
    Map<String, GenericCatalog> map = new LinkedHashMap<>();
    for(GenericCatalog item : list){
        map.put(item.name.get(), item);
    }
    catalogCache.put(catalogNames.get(i), map);};

Second iteration using forEach:

catalogNames.forEach(e -> {
    Map<String, GenericCatalog> map = new LinkedHashMap<>();
    DummyData.getCatalog(e).forEach(d -> {
        map.put(d.name.get(), d);
    });
    catalogCache.put(e, map);});

And third iteration that removes unnecessary bracers:

catalogNames.forEach(objName -> {
    Map<String, GenericCatalog> map = new LinkedHashMap<>();
    DummyData.getCatalog(objName).forEach(obj -> map.put(obj.name.get(), obj));
    catalogCache.put(objName, map);});

My question now is what can be further done to simplify this?
I do understand that it's not really necessary to do anything else with this method at this point, but, I was curios about the possibilities.

3
  • 7
    Tip: shorter does not always mean simpler Commented Mar 2, 2017 at 10:45
  • Why do you want to simplify this. I think your solution #2 is conceive and you understand directly what it does. Commented Mar 2, 2017 at 10:49
  • As I said in the question, I agree that solution #2 and solution #3 are both easy and simple (I'm using #2 in the code atm). I was just curious about what more can be done. Commented Mar 2, 2017 at 10:56

2 Answers 2

3

There is small issue with solution 2 and 3 they might cause a side effects

Side-effects in behavioral parameters to stream operations are, in general, discouraged, as they can often lead to unwitting violations of the statelessness requirement, as well as other thread-safety hazards.

As an example of how to transform a stream pipeline that inappropriately uses side-effects to one that does not, the following code searches a stream of strings for those matching a given regular expression, and puts the matches in a list.

ArrayList<String> results = new ArrayList<>();
 stream.filter(s -> pattern.matcher(s).matches())
       .forEach(s -> results.add(s));  // Unnecessary use of side-effects!

So instead of using forEach to populate the HashMap it is better to use Collectors.toMap(..). I am not 100% sure about your data structure, but I hope it is close enough.

There is a List and corresponding Map:

List<Integer> ints = Arrays.asList(1,2,3);

Map<Integer,List<Double>> catalog = new HashMap<>();
catalog.put(1,Arrays.asList(1.1,2.2,3.3,4.4));
catalog.put(2,Arrays.asList(1.1,2.2,3.3));
catalog.put(3,Arrays.asList(1.1,2.2));

now we would like to get a new Map where a map key is element from the original List and map value is an other Map itself. The nested Map's key is transformed element from catalog List and value is the List element itself. Crazy description and more crazy code below:

Map<Integer, Map<Integer, Double>> result = ints.stream().collect(
        Collectors.toMap(
                el -> el, 
                el -> catalog.get(el).stream().
                        collect(Collectors.toMap(
                                c -> c.intValue(), 
                                c -> c
                        ))

        )
);
System.out.println(result);
// {1={1=1.1, 2=2.2, 3=3.3, 4=4.4}, 2={1=1.1, 2=2.2, 3=3.3}, 3={1=1.1, 2=2.2}}

I hope this helps.

Sign up to request clarification or add additional context in comments.

Comments

0

How about utilizing Collectors from the stream API? Specifically, Collectors#toMap

Map<String, Map<String, GenericCatalog>> cache = catalogNames.stream().collect(Collectors.toMap(Function.identity(),
    name -> DummyData.getCatalog(name).stream().collect(Collectors.toMap(t -> t.name.get(), Function.identity(),
            //these two lines only needed if HashMap can't be used
            (o, t) -> /* merge function */,
            LinkedHashMap::new));

This avoids mutating an existing collection, and provides you your own individual copy of a map (which you can use to update a cache, or whatever you desire).

Also I would disagree with arbitrarily putting end braces at the end of a line of code - most style guides would also be against this as it somewhat disturbs the flow of the code to most readers.

Comments

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.