Request for Review : CR#8004015 : Add interface extends and defaults for basic functional interfaces (original) (raw)

Talden talden at gmail.com
Thu Nov 29 08:21:40 UTC 2012

On Thu, Nov 29, 2012 at 6:50 PM, David Holmes <david.holmes at oracle.com>wrote:

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.

I've personally never a one-style-to-rule-them-all kinda guy and I usual format trivial methods this way - that is, trivial getters, setters and delegating implementations.

I've considered the vertical brevity a readability benefit in these cases and the absence of 'abstract' (for class methods) or the presence of default (for interfaces) seems cue enough.

Still, at least it's not the opposite extreme.

While I'm happy with (though 2-space vs 4-space indentations are my preference)...

@Override
public default Double operate(Double left, Double right) { return

operateAsDouble((double) left, (double) right); }

I've worked with those that think this is necessary (assume a fixed width font to understand the indentation)...

@Override
public default Double operate(
                               Double left,
                               Double right
                             )
{
    return operateAsDouble( ( double ) left, ( double ) right );
}

Monitor aspects are getting relatively wider and screens larger overall. IMO longer lines and fewer content-less lines simply fits the medium better.

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.

Agreed. Don't put superfluous {@inheritDoc} in there but please don't omit the Javadoc entirely if the contract is further constrained.

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.

I have a dilemma, I disagree with the first point but not the second.

Describing the complexity of the algorithm selected by default is useful information - It's saying how it scales, not how it's implemented. Documentation on how methods scale in a library is definitely beneficial.

However, you're right, that documentation should be specific to the JavaSEimplementation not the JavaSE spec. It should state that the behaviour could vary with the supplier and the documentation for another JavaSE implementation should be checked or the behaviour tested. The spec could specify that it's the reference implementation and note that other implementations might use alternate implementations with different scaling characteristics - then it's easy, implementers need only alter that documentation when they deviate from the reference algorithm (though that opens a small future compatibility can-o-worms if the spec wants to use a different reference implementation in future releases).

Surely the JDK has precedent for this in overridable methods of classes expecting to be extended. I'm guessing such details are omitted which makes for some potentially confusing performance differences Java implementations.

-- Aaron Scott-Boddendijk

More information about the core-libs-dev mailing list