Concise Method Bodies feedback
Stephen Colebourne
scolebourne at joda.org
Tue Oct 9 21:34:17 UTC 2018
My first reaction to concise method bodies was "yuck". But I wasn't
fully sure why, so I held back on commenting here to see some of the
points in the discussion.
The headline is that I am mostly in agreement with Kevin's response.
That adding this to Java will be adding a lot of style discussions and
mental overload for something that mostly seems pretty low value.
There needs to be more evidence of benefits to justify this change.
- Which method variant should I use for the method I'm writing?
- Should I change others in the file to match?
- Does it make it less likely that I'll add that extra bit of code in
because of the hassle of converting back from the short form to the
long form? (I hate it when I have to convert back from method ref to
lambda)
- Will this just result in office debate? (I hate that IntelliJ has a
helpful hint to convert lambdas to method refs and too many unthinking
IntelliJ users therefore assume method refs are better)
- Why do I need to think about all this, its not helping me write code
(and certainly making it harder to read)?
---
Comments
One of my first reactions was also the lack of consideration of
comments (something I've also noted with records). For a typical
getter-style method this feature may target, there will typically be 3
to 5 lines of Javadoc to 3 lines of code:
/**
* Gets the foo.
* <p>
* The foo is used for bar and baz.
*/
public String getFoo() {
return foo;
}
With properly documented code, CMB clearly won't be saving as much of
the codebase as it might initially appear.
The situation is worse for methods that take arguments. For example,
LocalTime has factory methods that take hour/minute and could call the
factory that takes hour/minute/sec/nano. But the docs dominate, so the
"saving" isn't really there.
/**
* Obtains an instance of {@code LocalTime} from an hour and minute.
* <p>
* This returns a {@code LocalTime} with the specified hour and minute.
* The second and nanosecond fields will be set to zero.
*
* @param hour the hour-of-day to represent, from 0 to 23
* @param minute the minute-of-hour to represent, from 0 to 59
* @return the local time, not null
* @throws DateTimeException if the value of any field is out of range
*/
public static LocalTime of(int hour, int minute) -> of(hour, minute, 0, 0);
(woo! I saved 2 lines out of 14. And made the code harder to read)
And when I hit the line length limits and have to wrap the method
parameters? It will look damn ugly.
---
Use cases
So far, my reading of the thread and JEP is that the use cases are not
particularly motivating. The comparator one is better, but I want more
than one use case. The trouble is that I can't say I've ever felt that
the braces on a method are holding me back and the discussion so far
isn't selling it as a feature.
---
Readabilty
Some of the examples posted on the spec-experts thread have been
downright unreadable. I find myself really struggling to work out
whether a line is an instance variable or a method declaration. That
simply shouldn't be the case!
---
Syntax/Semantics
(Its not too early to talk syntax, when the feature is mostly syntactic)
For the method ref form, I'm not sold on full expressions on the LHS
of :: particularly wrt when the expression is evaluated. I'm also
struggling to see use cases beyond instance variables, this and types,
so semantically I'd prefer to keep it simple.
For the expression form, the semantics are obvious, but the need seems
very low. A method can be written on one line today, so this saves
very little, and at a high cost.
I'm not a fan of either the = or arrow syntax here. The equals is
understandable, but does make the code blend into instance variables
when written without comments (I'm sure there is a puzzler there...).
The arrow is being subverted again. It should be just for mobile code,
whereas the code here runs in the context you see. And arrow is
particularly painful here because a lambda can have parameters to the
left of the arrow, so the eye has to backtrack all the way to the
method name and the absence of an = to determine that this is a method
declaration rather than an instance variable with an initial value of
a lambda.
Since its not too late to change switch, I'll argue for colon and colon-equals:
public static LocalTime of(int hour, int minute): of(hour, minute, 0, 0);
public static LocalTime of(int hour, int minute):= LocalTime::create;
switch (trafficLight) {
when RED: doRed();
}
ie. Colon introduces some code that runs within the context you can
see. Arrow introduces some code that is mobile and may be run in
another context.
---
Summary
Having seen the debate and use cases, there might be a case for a
simple version of the method ref CMB using a syntax other than =. But
I really struggle with the expression form. It just seems to be adding
complexity, making code harder to write and harder to read. Could the
solution be to keep method ref form and dump the expression form?
Thanks for listening
Stephen
More information about the amber-dev
mailing list