State of the Java Style Guidelines document

classic Classic list List threaded Threaded
1 message Options
Reply | Threaded
Open this post in threaded view
|

State of the Java Style Guidelines document

Andreas Lundblad-2
Current status: The page has been rewritten as a patch for the web repo.
Formatting is in line with the other pages on o.j.n. It's v6 with a few
rewordings, among others those suggested by Lars Francke. (Grammar and typos
IIRC.)

This was done before I left Oracle but it never got the final sign-off due
to some last minute opinions on the content. Since then I've reached out on
four occasions trying to get the ball rolling without success. I imagine
people have been busy with more important stuff (like finalizing Java 9) and
I'm now hoping this will get some attention. Personally I have no problem
spending time on this.

@Mike, sorry for not getting back to you on all items. I think many of your
remarks have been addressed. I'll respond to two of them below. If there's
anything else you feel have been left behind, please let me know.

*** UTF-8 vs ASCII ***

I initially had UTF-8, but changed to ASCII. Currently the effort
focuses on documenting existing practices, rather than updating to new
practices, and ASCII is the current standard (at least it was when at the
time of writing).

From Jon Gibbons:

> [...]
> But ASCII is still the rule for JDK code, partly for historical reasons and
> partly (I suspect) because there is not enough motivation to investigate
> whether the entire JDK code ecosystem can cope with anything more than
> ASCII.
> [...]

From Joe Darcy:

> [...]
> I double checked the relevant makefile (make/common/SetupJavaCompilers.gmk)
> and the JDK build does indeed specify to javac
>
>    -encoding ascii
> [...]

I hope this explains the current formulation.


*** Opening brace on a separate line ***

One could argue both ways regarding readability. I don't think one
alternative is, objectively speaking, more readable than the other.

I know java.nio uses this for instance, and v1 (index-v1.html) mentioned
this as an alternative, but there were complains about this and it was
dropped in v2.

To make an objective assessment I analyzed the existing code and found that
the style recommended in the current draft is used in 74% of all cases where
class declarations or methods are wrapped. (Pasting the source code I used
for the analysis at the bottom of this mail.)

That's across all OpenJDK projects however. Formatting is presumably much
more consistent within each separate project.

This has in fact been a bit of a blocker. At this point I think that Lars'
suggestion is the best (only?) way forward. One general document covering
the guidelines that all projects agree on, and for projects that find it
necessary, additional pages in the project spaces, refining what's needed.

I'd suggest that the general document tries to stick to the intersection of
what all projects find acceptable since it it might be confusing if project
specific style guides need to opt out from stuff. Also, if a case is not
covered, it's often clear from surrounding code what the preferred formatting
is. The general document can include notes like "For further details, see
project specific documentation" where needed.

OpenJDK is huge with a lot of legacy code. I think this a natural approach.

-- Andreas

-------------------

Code to analyze placement of braces in wrapped method headers:



import java.io.IOException;
import java.nio.file.*;
import java.nio.file.attribute.BasicFileAttributes;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import static java.nio.charset.StandardCharsets.UTF_8;
import static java.nio.file.FileVisitResult.CONTINUE;
import static java.util.regex.Pattern.DOTALL;
import static java.util.regex.Pattern.MULTILINE;

public class Scratch {

    static String id = "\\p{Alpha}\\p{Alnum}*";
    static String clsHdr = "^(public )?class (.*?)\\{";
    static String mthHdr = "^ *"             // beginning of line + indentation
                         + "(public|private|protected|static|final|
)*" // method modifiers
                         + id + "\\s+" + id  // return type - method identifier
                         + "\\([^\\)]*\\)"   // arguments
                         + "[^;{]*"          // throws declarations
                         + "\\{$";           // opening brace
    static Pattern CLASS_HDR = Pattern.compile(clsHdr, MULTILINE | DOTALL);
    static Pattern METHOD_HDR = Pattern.compile(mthHdr, MULTILINE | DOTALL);

    static int fileCount = 0;
    static int knrCounter = 0;  // Counter for '{' at the end of the same line
    static int mrCounter = 0;   // Counter for '{' on a line by itself

    public static void main(String[] args) throws IOException {
        analyze(Paths.get("/home/aioobe/work/oracle/repo/dev-tip"));
        System.err.println("knrCounter: " + knrCounter);
        System.err.println("mrCounter: " + mrCounter);
        System.err.printf("k&r: %.2f%%", 100.0 * knrCounter /
(knrCounter + mrCounter));
    }

    static void analyze(Path root) throws IOException {
        Files.walkFileTree(root, new SimpleFileVisitor<Path>() {
            @Override
            public FileVisitResult visitFile(Path file,
BasicFileAttributes attrs)
                    throws IOException {
                if (file.toString().contains("test/java/util/Formatter/")) {
                    return CONTINUE;
                }
                if (file.toString().endsWith(".java")) {
                    fileCount++;
                    if (fileCount % 1000 == 0) {
                        System.err.printf("%.2f%%%n", 100.0 *
fileCount / 38016);
                    }
                    String sourceCode = new
String(Files.readAllBytes(file), UTF_8);
                    checkHeaders(sourceCode);
                }
                return CONTINUE;
            }
        });
    }

    // Traverse all method and class headers in given source code.
    static void checkHeaders(String sourceCode) {
        Matcher m = METHOD_HDR.matcher(sourceCode);
        while (m.find()) {
            analyzeHeader(m.group());
        }

        m = CLASS_HDR.matcher(sourceCode);
        while (m.find()) {
            analyzeHeader(m.group());
        }
    }

    // Check content between "class" and "{" or "methodName()" and "{"
    // and increment mrCounter or knrCounter as appropriate.
    static void analyzeHeader(String header) {
        String[] lines = header.split("\n");
        if (lines.length == 1) {
            // No wrapping
            return;
        }

        if (lines.length == 2 && lines[1].trim().equals("{")) {
            // Allman style:
            // void method()
            // {
            //     ...
            // }
            return;
        }

        if (lines[lines.length - 1].trim().equals("{")) {
            // Last line is a single "{"
            mrCounter++;
        } else {
            // Last line is more than just "{"
            knrCounter++;
        }
    }
}