steveloughran commented on PR #921:
URL: https://github.com/apache/ozone/pull/921#issuecomment-1847636749
Hadoop stayed on 80 for a long time not so much for punched-card development
(that's where it came from!) but because it was easier to do side-by-side
reviews with the .diff viewer plugin for chrome.
we moved to 100 chars a few years back and I have no regrets. It helps us
stay with readable code when using builder APIs:
```java
public interface FutureDataInputStreamBuilder
extends FSBuilder<CompletableFuture<FSDataInputStream>,
FutureDataInputStreamBuilder> {
```
Here the types of arguments/subclasses are over 80 chars wide, splitting
this stuff makes it harder to parse.
And, just like the limits on method length and #of arguments to a method or
constructor, it is only a hint.
Which is good when you are writing methods which take builders from the AWS
SDK as arguments:
```java
private <BuilderT extends S3BaseClientBuilder<BuilderT, ClientT>, ClientT>
void configureEndpointAndRegion(
BuilderT builder, S3ClientCreationParameters parameters, Configuration
conf) {
```
Although wider lines may appear to go back on the "side by side" reviewing
problem, it is considered moot because
* displays have got wider
* github is a better review tool which does word wrap
* vs code and intellij are great for reviews too.
Adopting a wider column count should not be an excuse to overboard with
functional programming code to show off how you learned Haskell, Standard ML or
OCaml
```java
getOptionalKeys().stream().filter(!key -> key.startsWith(headerPrefix) &&
key.length() > prefixLen).forEach(key -> headers.put(key.substring(prefixLen),
options.get(key)));
```
why so? makes debugging and tracking down where an NPE came from wrong. A
single clause per line is much better as you can set breakpoints, isolate stack
traces etc.
```java
getOptionalKeys().stream()
.filter(key -> key.startsWith(headerPrefix) && key.length() >
prefixLen)
.forEach(key -> headers.put(key.substring(prefixLen),
options.get(key)));
```
Accordingly, I'd worry less about line length and more about modern java
code issues
* best practices for lambda expressions
* effective use of optional<>, java17 type inference, ...
* how to design for shading, e.g. making sure you never return *any* type
from someone else's library in your APIs in case you want to move to shaded
libraries, change the implementation etc.
* how to design for maintenance. We've been maintaining hadoop code for 15+
years. What could we have done better?
* design for testability.
* should we embrace runtime exceptions so we can use java streaming APIs, or
do we have to reimplement our own functional libraries. It's too late to change
old code, hence org.apache.hadoop.util.functional, but new APIs?
For the last 2 I've really embraced interface/implementation rather than
things which require mockito abuse for changing behaviour. Hadoop avoided
interfaces in the past because its impossible to add new methods without
breaking external code. but with java8 and the ability to define default
implementations, that changed.
So here's my proposal
# stop worrying about line length. Change the rule, but even if not, +1
patches which break it if it makes the code clearer
# come and discuss on hadoop-common how we could do a style guide for the
new world, based on others' work
# look at the style rules of apache Scala projects, so we can learn from
them especially on type inference (all args/return values must declare types)
# embrace AssertJ if you haven't already.
if you can't get consensus, move this out of github issues into mailing
lists. And focus on what is best for the readability and maintainability of
modern code, not stuff we wrote for java 5.
(BTW: all those code snippets are from hadoop trunk, not all my work)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
