Hi Mike,
Just a few minor javadoc suggestions. Otherwise, the "meat" looks good.
Thanks,
Jim
-----------------------------------------------------------------------------------
Copyrights -
- why do some of the copyrights say 2010, 2011, 2012 ? Is this the
correct format? DoubleBinaryOperater.java (and others) just have 2012
java/util/BinaryOperator.java
- why is extends Combiner<T,T,T> commented out on the interface
decl? Is this part of the phase-in of these changes?
- for the javadoc for operate(T,T), I'd say "Returns the result of
some /binary /operation upon the operands."
java/util/DoubleBinaryOperator.java
- for the javadoc for operate(double,double), I'd say "Returns the
result of some /binary /operation upon the operands."
java/util/DoubleMapper.java
- javadoc - simply insert comma between "Given an input object" and
"maps to an appropriate {@code double) value"
java/util/DoubleUnaryOperator.java
- "Returns a {@code double} value computed from the double
operand." Should the second "double" also be "{@code double}"?
java/util/functions/Factory.java
- "Returns an object. The returned object may be an existing object
or a newly created object." This might be better: "Returns an object
which may have been created previously or is newly created."
- for make() - "Returns an object" --> "Creates and returns an
object or returns one previously created"
java/util/functions/IntBinaryOperator.java
- " Combines two {@code int} operands of the same type producing an
{@code int} result" -- I'd insert a comma between "type" and "producing"
- "some operation" -> "some /binary/ operation"
java/util/IntMapper.java
- javadoc - simply insert comma between "Given an input object" and
"maps to an appropriate {@code int) value"
java/util/functions/LongBinaryOperator.java
- "some operation" -> "some /binary/ operation"
java/util/LongMapper.java
- javadoc - simply insert comma between "Given an input object" and
"maps to an appropriate {@code long) value"
java/util/Mapper.java
- javadoc - simply insert comma between "Given an input object" and
"maps to an appropriate output object"
java.util/functions/Predicate.java
- I think "condition" might be better than "criteria"
java/util/functions/UnaryOperator.java
- "@param <T> the type of input objects to {@code operate} and of
the result" is hard to parse. Perhaps this would be better: "@param
<T> the operand and result type of the {@code operate} method"
- why is "extends Map<T,T>" commented out?
java/util/functions/package-info.html
- this seems a little awkward:
35 * <p>All functional interface implementations are expected to:
36 * <ul>
37 * <li>When used for aggregate operations upon many elements it should not
be
38 * assumed that the operation will be called upon elements in any specific
order.
39 * </li>
40 * </ul>
It appears you were/are expecting additional things to be added to the
list of expectations. Maybe it would be best to reserve the list
structure of the comment for the time where additional conditions are
added. Until then a simpler sentence would be cleaner:
"When used for aggregate operations upon many elements, no functional
interface implementation should assume that the operation will be called
upon elements in any specific order.
As Porky would say, That's all folks! :-)
On 10/31/2012 04:16 PM, Mike Duigou wrote:
There's a large set of library changes that will be coming with Lambda. We're
getting near the end of the runway and there's lots left to do so we want to
start the process of getting some of the more stable pieces put back to the
JDK8 repositories. We've spent a some time slicing things into manageable
chunks. This is the first bunch. We'd like to time-box this review at one week,
since there are many more pieces to follow.
The first chunk is the basic set of functional interface types. While this set
is not complete, it is enough to be able to proceed on some other pieces. This
set contains no extension methods (we'll do those separately) and does not
contain all the specializations we may eventually need.
The specification is limited; most of the interesting restrictions (side-effect-freedom,
idempotency, stability) would really be imposed not by the SAM itself by by how the SAM
is used in a calculation. However, some common doc for "how to write good SAMs"
that we can stick in the package doc would be helpful. Suggestions welcome.
Elements of this naming scheme include:
- Each SAM type has a unique (arity, method name) pair. This allows SAMs to
implement other SAMs without collision.
- The argument lists are structured so that specializations act on the first argument(s), so
IntMapper<T> is a specialization of Mapper<R,T>, and IntBinaryOperator is a
specialization of BinaryOperator<T>.
In order to get the most useful feedback out of this review, we'd like to ask
you follow the following guidelines for the review:
- We are time-boxed at one week. (until Nov. 7th)
- Please review the whole bunch in a single message if possible, rather than in
bits and pieces. It is far easier to extract useful feedback from one complete
review than from a dozen partial ones.
- Please wait a few days before replying to other people's reviews! We want to
keep the discussion on-topic to maximize the useful review content. It is far
too easy for the discussion to spiral off into minutia and lose sight of the
goal -- which is to provide useful feedback on the API we're asking for
feedback on.
http://cr.openjdk.java.net/~mduigou/8001634/2/webrev/
--
Jim Gish | Consulting Member of Technical Staff | +1.781.442.0304
Oracle Java Platform Group | Core Libraries Team
35 Network Drive
Burlington, MA 01803
jim.g...@oracle.com
