Thanks for the corrections. I have incorporated all of these into the integration version of the patch.
On Apr 12 2013, at 12:50 , Akhil Arora wrote: > Hi Mike, > > a few small things - > > UnmodifiableMap.forEach > is missing Objects.requireNonNull(action); Added. > > EmptyMap.replaceAll(BiFunction) > should just return instead of throwing UnsupportedOpEx > particularly since EmptyList.replaceAll also returns silently > after checking if function is null to fulfil the NPE contract I agree. > > Hashtable > is missing a synchronized override of forEach added. > > CheckedMap.typeCheck(BiFunction) > is missing from the patch (won't compile without it) Apparently I forgot a qrefresh before generating the webrev. I had this in my local copy as it's necessary. > > On 04/11/2013 01:03 PM, Mike Duigou wrote: >> Another revision incorporating primarily documentation feedback. >> >> http://cr.openjdk.java.net/~mduigou/JDK-8010122/2/webrev/ >> >> I've also included the java.util.Collections overrides for the default >> methods. All of these are performance enhancements--the semantics were >> already correct because the defaults use only public methods. >> >> This is likely, modulo formatting of the source examples, the version that >> will be pushed to TL on Friday unless somebody squawks. >> >> Moving the current compute, computeIfPresent, computeIfAbsent and merge >> defaults to ConcurrentMap and replacing the current Map defaults with >> versions that throw ConcurrentModificationException will likely be resolved >> in a future issue if the EG determines it to be important. >> >> Mike >> >> >> On Apr 10 2013, at 22:42 , Mike Duigou wrote: >> >>> I've posted an updated webrev with the review comments I have received. >>> >>> http://cr.openjdk.java.net/~mduigou/JDK-8010122/1/webrev/ >>> >>> One important point to consider: >>> >>> - The current implementations of compute, computeIfPresent, >>> computeIfAbsent, merge are implemented so that they can work correctly for >>> ConcurrentMap instances. For non-concurrent implementations it might be >>> better to fail and throw ConcurrentModification exception whenever >>> concurrent modification is detected. For regular Map implementations the >>> retry behaviour will only serve to mask errors. >>> >>> Thoughts? >>> >>> Mike >>> >>> On Apr 8 2013, at 11:07 , Mike Duigou wrote: >>> >>>> Hello all; >>>> >>>> This is a combined review for the new default methods on the java.util.Map >>>> interface being added for the JSR-335 lambda libraries. The reviews are >>>> being combined because they share a common unit test. >>>> >>>> http://cr.openjdk.java.net/~mduigou/JDK-8010122/0/webrev/ >>>> >>>> 8004518: Add in-place operations to Map >>>> forEach() >>>> replaceAll() >>>> >>>> 8010122: Add atomic operations to Map >>>> getOrDefault() >>>> putIfAbsent() * >>>> remove(K, V) >>>> replace(K, V) >>>> replace(K, V, V) >>>> compute() * >>>> merge() * >>>> computeIfAbsent() * >>>> computeIfPresent() * >>>> >>>> The * operations treat null values as being absent. (ie. the same as there >>>> being no mapping for the specified key). >>>> >>>> The default implementations provided in Map are overridden in HashMap for >>>> performance purposes, in Hashtable for atomicity and performance purposes >>>> and in Collections for atomicity. >>>> >>>> Mike >>> >> >
