CODEWITHSUNDEEP
loopsfor-loopguavamultimap
aperezra
aperezra

Reputation: 165

Iterate through multi map in Java

I have a multimap in where my key is a String and the values are Integers. I would like to iterate through all those Integers, in order to calculate the mean value of them, for finally, just store the key and the mean value.

This is what I have written at the moment

        int visits = 0;
    for (String key : result.keys()) {
        Object[] val = result.get(key).toArray();
        for (int i=0; i<val.length; i++){
            visits+=(Integer)val[i];
        }
        visits=visits/val.length;
        result.removeAll(key);
        result.put(key, visits);
    }

But I'm getting this error

at java.util.AbstractList$Itr.checkForComodification(AbstractList.java:372)
at java.util.AbstractList$Itr.next(AbstractList.java:343)
at com.google.common.collect.AbstractMapBasedMultimap$Itr.next(AbstractMapBasedMultimap.java:1150)
at com.google.common.collect.TransformedIterator.next(TransformedIterator.java:48)
at subset.calcMax.meanCalc(calcMax.java:147)
at subset.calcMax.main(calcMax.java:208)

it points to the line for (String key : result.keys()) but the error is not in this iteration, because if I delete what is in the for loop it works. So my problem is in the iteration through the values that are for each key.

I would appreciate your help.

Thanks in advance!

Upvotes: 3

Views: 6031

Answers (1)

Etienne Neveu
Etienne Neveu

Reputation: 12692

As explained in the comments, collections throw ConcurrentModificationExceptions when modified while being iterated. It's bad practice to mutate the source collection anyway, so you're better off creating a new collection and returning that.

I would write:

ImmutableMultimap<String, Integer> computeMeanVisits(Multimap<String, Integer> multimap) {
    ImmutableMultimap.Builder<String, Integer> result = ImmutableMultimap.builder();

    for (String key : multimap.keySet()) {
        Collection<Integer> values = multimap.get(key);
        result.put(key, mean(values));
    }
    return result.build();
}

int mean(Collection<Integer> values) {
    int sum = 0;
    for (Integer value : values) {
        sum += value;
    }
    return sum / values.size();
}

As an aside:

  • I don't like your use of .toArray() to iterate on the values. In Java, it's usually preferred to manipulate collections directly. Direct Array manipulations should be reserved for very specific, high performance code, or when you have to deal with bad APIs that only accept arrays. Note that in your example, using .toArray() also makes you lose genericity, forcing you to cast each value to an Integer.
  • you should use the .keySet() method instead of the keys() method, which returns a Multiset. When iterating over this Multiset, keys associated with multiple values will appear multiple times.
  • your "visits" variable is not reset to 0 before computing a new mean

Upvotes: 1

Related Questions