Code review request: JDK-8008670 (partial java.util.stream implementation)

[Paul Sandoz]

On Mar 12, 2013, at 2:29 AM, Mike Duigou <mike.duigou at oracle.com> wrote:

> Some notes:
> 
> - Copyrights update.
> 

Pushed to lambda repo:

  http://hg.openjdk.java.net/lambda/lambda/jdk/rev/40c0a71455d7


> - The order of notes on files is the order that I read the files. It seems like not a bad order to review them.
> 
> - package docs? Coming next?
> 

Soonish :-) there is much left to review/spec.


> java.util.Map::
> 
> - @since 1.8 missing for defaults
> 
> - <tt></tt> should be replace with {@code}
> 

It is mostly consistent with the rest of the Map documentation. We should do a global replace in that case?


> - default methods need the notice that the default implementations do not provide atomicity. (copy from putIfAbsent)
> 

I have shuffled stuff around and used @implSpec.


> - ::forEach() - Rename the parameter from block?
> 

I updated that to use similar language as Iterator/Iterable.forEach.


> - ::computeIfAbsent() - "and enters it into this map" -> inserts?
> 
> - I wonder if compute if absent should replace the original get(key) with containsKey(key) and replace get() with containsKey() in the pseudo-code documentation. If implementations know that the map can't contain null then they can optimize to get(). Demonstrated with get() would seem to produce incorrect results with a present null value.
> 

> - ::computeIfAbsent()/computeIfPresent()/compute() - "key with which the specified value is to be associated" -> derived value.
> 

Perhaps the above three merit a separate discussion. Raise in a separate email thread?


> - ::computeIfAbsent()/computeIfPresent()/compute() - @throws RuntimeException or Error These should be separate.
> 

Do we really need to declare that? It would apply for every use of  lambdas and it is previously stated:

"If the function itself throws an (unchecked) exception, 
the exception is rethrown, and the current mapping is
left unchanged."

I have removed them.

http://hg.openjdk.java.net/lambda/lambda/jdk/rev/36664688aa52


> java.util.Collections::
> 
> - Map.synchronizedMap needs extension to provide synchronized implementations of Map default methods.
> 

http://hg.openjdk.java.net/lambda/lambda/jdk/rev/077efaf92c8c

You know off any tests that can be reused?


> java.util.Hashtable::
> 
> - synchronized implementations of Map default methods are needed.
> 

Good point.  I think it may help to address that and some other things together. Checked and unmodifiable lists should also be updated to be efficient for appropriate default methods.

There is also the synchronized collections/lists. We need to consider the spliterator/stream/parallelStream methods, especially how the spliterator should behave.

Paul.