Set Map.Entry value AFTER iteration complete?

Question

Can I save Map.Entry objects in a temporary Map and then go back and change their values AFTER iteration has completed?

For example, the following implements a sort of transaction in two steps. In step 1, a map of entries and maps of entries (so a tree of sorts) is recursively processed. Entries are processed into new values which are saved to a temporary map, keyed by the corresponding Map.Entry. If all entries are computed without Exception, step 2 is to simply iterate over the temporary map and assign the corresponding new value.

void performPerilousProcedure(Object val) throws Exception
{
    if (processing of val fails)
        throw new Exception();
}
void recurivePerilousProcedure(Map someMap, Map tmp) throws Exception
{
    Iterator<Map.Entry<String,Object>> iter1;
    iter1 = someMap.entrySet().iterator();
    while (iter1.hasNext()) {
        Map.Entry<String,Object> entry1 = iter1.next();
        Object val = entry1.getValue();
        if (val instanceof Map) {
            recursivePerilousProcedure((Map)val, tmp);
        } else {
            Object newval = performPerilousProcedure(val);
            // ok to use Map.Entry as key across iter?
            tmp.put(entry1, newval);
        }   
    }   
}
void doit(Map<String,Object> someMap) throws Exception
{
    HashMap<Map.Entry<String,Object>,Object> tmp = new HashMap();
    // Try to process map of entries and maps of entries and ma ... 
    recursivePerilousProcedure(someMap, tmp);
    // All entries success processed, now simply assign new vals
    Iterator<Map.Entry<String,Object>> iter2;
    iter2 = tmp.keySet().iterator();
    while (iter2.hasNext()) {
        Map.Entry<String,Object> entry2 = iter2.next();
        Object newval = tmp.get(entry2);
        entry2.setValue(newval); // commit new value
    }   
}

The question is:

Can the Map.Entry objects used as keys of tmp survive outside the iterations?

Provided that there are no concurrent modifications, is this a valid use of the Map interface?

Answer 1

The documentation of Map.Entry is very clear about it:

These instances maintain a connection to the original, backing map. This connection to the backing map is valid only for the duration of iteration over the entry-set view. During iteration of the entry-set view, if supported by the backing map, a change to a Map.Entry's value via the setValue method will be visible in the backing map. The behavior of such a Map.Entry instance is undefined outside of iteration of the map's entry-set view.

An undefined behavior does not preclude doing the intended thing and in practice, when the map is HashMap or TreeMap, these entry instances keep working as long as you don’t modify the the map (in a different way than calling setValue on an entry).

The question is whether you want to build your application on such an implementation specific behavior which, while longstanding, has not been specified explicitly.

---

There’s another problem in that you are using these entries as keys in another map. The equality of an entry is determined by the combination of key and value, but not the associated map. In other words, if two entries of different maps happen to have the same key and value, you have a clash and trying to put the other entry will keep the already existing entry but associate it with the new value not meant for this entry (or more precise, not meant for this target map).

Even worse, you are changing the equality while the entry instance is already in use as a key, by calling setValue on them, which is definitely illegal and turns the HashMap into an inconsistent state:

Note: great care must be exercised if mutable objects are used as map keys. The behavior of a map is not specified if the value of an object is changed in a manner that affects equals comparisons while the object is a key in the map.

If all that you are going to do, is to iterate over tmp once, there is no need to use a map as no lookup operation is required. A List<Map.Entry<Key,Value>> would do instead of Map<Key,Value>, to keep a combination of key and value you can iterate over. In fact, any object capable of holding two values could be used instead of Map.Entry<Key,Value>. If you replace it with an object capable of holding three values, you could keep track of the target Map, key, and new value and just perform a simple put at the end.

So the first approach could look like

void recursivePerilousProcedure(Map<String,Object> someMap,
                                List<Map.Entry<Map.Entry<String,Object>,Object>> tmp)
                                throws Exception {
    for(Map.Entry<String,Object> entry1: someMap.entrySet()) {
        Object val = entry1.getValue();
        if (val instanceof Map) {
            recursivePerilousProcedure((Map<String,Object>)val, tmp);
        } else {
            Object newval = performPerilousProcedure(val);
            tmp.add(new AbstractMap.SimpleEntry<>(entry1, newval));
        }
    }
}
void doit(Map<String,Object> someMap) throws Exception {
    List<Map.Entry<Map.Entry<String,Object>,Object>> tmp = new ArrayList<>();

    // Try to process map of entries and record new values
    recursivePerilousProcedure(someMap, tmp);

    // All entries success processed, now simply assign new vals
    // relies on Map implementation detail
    for(Map.Entry<Map.Entry<String,Object>,Object> entry: tmp) {
        Object newval = entry.getValue();
        entry.getKey().setValue(newval); // assign new value
    }
}

which is relying on the implementation detail but has the slight advantage that no map lookup is needed when calling setValue. Or the clean solution:

record NewValue(Map<String,Object> target, String key, Object newValue) {
    void apply() {
        target.put(key, newValue);
    }
}
void recursivePerilousProcedure(Map<String,Object> someMap,
                                List<NewValue> tmp) throws Exception {
    for(Map.Entry<String,Object> entry1: someMap.entrySet()) {
        Object val = entry1.getValue();
        if (val instanceof Map) {
            recursivePerilousProcedure((Map<String,Object>)val, tmp);
        } else {
            Object newval = performPerilousProcedure(val);
            tmp.add(new NewValue(someMap, entry1.getKey(), newval));
        }
    }
}
void doit(Map<String,Object> someMap) throws Exception {
    List<NewValue> tmp = new ArrayList<>();
    // Try to process map of entries and record new values
    recursivePerilousProcedure(someMap, tmp);

    // All entries success processed, now simply assign new vals
    // assign all new values, cleanly
    tmp.forEach(NewValue::apply);
}

Depending on your application, there might be a third option. Just build a new map matching the intended target structure during the recursive processing, to replace the original one when no error occurred.