Request for Review : CR#8004015 : Add interface extends and defaults for basic functional interfaces
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