I have updated the webrev with these changes and a few more. merge() required an update to it's specification to correctly account for null values.
http://cr.openjdk.java.net/~mduigou/JDK-8010122/5/webrev/ This version is currently undergoing final pre-integration testing. Unless additional problems are found or the testing fails this version will be integrated. Mike On Apr 12 2013, at 12:53 , Mike Duigou wrote: > 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 >>>> >>> >> > >
