Request for Review : CR#8004015 : Add interface extends and defaults for basic functional interfaces

[David Holmes]

Mike,

On 28/11/2012 3:32 AM, Mike Duigou wrote:
> On Nov 27 2012, at 03:56 , Stephen Colebourne wrote:
>
>> On 27 November 2012 02:12, Mike Duigou<mike.duigou at oracle.com>  wrote:
>>> In the original patch which added the basic lambda functional interfaces, CR#8001634 [1], none of the interfaces extended other interfaces. The reason was primarily that the javac compiler did not, at the time that 8001634 was proposed, support extension methods. The compiler now supports adding of method defaults so this patch improves the functional interfaces by filing in the inheritance hierarchy.
>>>
>>> Adding the parent interfaces and default methods allows each functional interface to be used in more places. It is especially important for the functional interfaces which support primitive types, IntSupplier, IntFunction, IntUnaryOperator, IntBinaryOperator, etc. We expect that eventually standard implementations of these interfaces will be provided for functions like max, min, sum, etc. By extending the reference oriented functional interfaces such as Function, the primitive implementations can be used with the boxed primitive types along with the primitive types for which they are defined.
>>>
>>> The patch to add parent interfaces and default methods can be found here:
>>>
>>> http://cr.openjdk.java.net/~mduigou/8004015/0/webrev/
>>> http://cr.openjdk.java.net/~mduigou/8004015/0/specdiff/java/util/function/package-summary.html
>>
>> Each of the default methods is formatted on a single line. I consider
>> this to be bad style, they should be formatted as per "normal"
>> methods:
>> @Override
>> public default Double operate(Double left, Double right) {
>>   return operateAsDouble((double) left, (double) right);
>> }
>>
>> It is vitally important to get this kind of formatting/style correct.
>
>> Developers the world over will be copying what the style is in these
>> classes.
>
> I totally agree that we should be consistent but I don't believe that there's a consensus yet on what good style is for these cases. What's your rationale for it being "bad style"?

I have to concur with Stephen - these are just method definitions and 
should follow the same style that is used for method definitions in 
classes. No need to contemplate introducing a new style just for default 
methods.

>> There is also no Javadoc on the default method override.
>
> That was intentional. The goal was to inherit from the original definition.

I don't think we can just leave it at that. If we are introducing 
additional constraints over that specified in base method then they need 
to be documented. We should be able to use implicit doc inheritance plus 
@{inheritDoc} to bring down what is unchanged and then add what is needed.

I don't agree that we need to describe what the default implementation 
does, for two reasons:

1. Normal methods don't usually specify how they are implemented - it is 
an implementation detail. The "default" simply indicates that this 
method does have an implementation and you should expect that 
implementation to obey the contract of the method.

2. It is not obvious to me that the JDK's choice for a default 
implementation has to be _the_ only possible implementation choice. In 
many/most cases there will be a very obvious choice, but that doesn't 
mean that all suppliers of OpenJDK classes have to be locked in to that 
choice.

Cheers,
David
-----

>> In this case,
>> passing a null to either parameter will result in an NPE. This should
>> be documented.
>
> Agreed. However... The following seems entirely overkill and given how these interfaces are likely to be used the javadoc will not be visible. My inclination is to add the description regarding null behaviour to the class javadoc at the same point where it's described that IntBlock can be used for Block<Integer>. ie.
>
> This is the primitive type specialization of Block for int and also may be used as a Block<Integer>  provided that the parameter to accept(Integer) always is non-null.
>
>>
>> More generally, you/Oracle should define a standard form of words for
>> describing what a default method does. Interfaces have not had them
>> before, and their behaviour needs documenting (even if it seems
>> obvious).
>>
>> /**
>> * An operation upon two operands yielding a result.
>> * The operands and the result are all of the same type.
>> *<p>
>> * The default implementation calls {@link operate(double,double)},
>> * throwing NullPointerException if either input is null.
>> *
>> * @param left  the first operand, not null
>> * @param left  the first operand, not null
>> * @return the result, not null
>> */
>> @Override
>> public default Double operate(Double left, Double right) {
>>   return operateAsDouble((double) left, (double) right);
>> }
>>
>> Stephen
>>
>
>