Decide on Allman vs K&R brace position and stay consistent

[qualidafial]

Allman:

public void foo()
{
  // do stuff
}

Kernighan & Ritchie:

public void foo() {
  // do stuff
}

The code seems to use lots of both styles, to the point that I'm not sure which one we're going for.

My vote would be K&R style braces (end of line) since the Java ecosystem at large seems to follow it.

[stevenschlansker]

Yes, braces at end of line is fine by me. We should maybe do a once over for code style before release, there's lots of some_variable and whatnot that is very un-Javalike

[brianm]

A convention was snake case for local variables :-)

It is NOT very common in Java, but fwiw, I grew to like it and still do it out of habit until I stop myself!

[hgschmie]

Heh. We always did Ning style in JDBI, why change now?

It is "allman" style for classes and methods and K&R for control statements on the account that each is better readable in their respective situations. Also, it is

if (...) {
...
}
else {
...
}

and not

if (...) {
...
} else {
...
}

on the account that the former allows you to select the full else block without cursor-left and right movement and the latter does not (hello, peeps stuck on vi :-) ) .

[qualidafial]

Whatever we do, I feel like it's time to have the checkstyle plugin start validating formatting during builds.

[stevenschlansker]

Ah of course, the bikeshed issue attracts the most commentry ;)

I also do not care, sticking with the existing style is fine, but I agree we should enforce it one way or the other.

[arteam]

I propose to switch to the K&R style which is rather simple to enforce with checkstyle and usually is set as the default for the majority of Java IDEs.

[qualidafial]

I personally like the look of Allman brace style.

However I would argue that we should switch to K&R because it's what the vast majority of Java devs use. Allman braces just add work to PRs. e.g. #474 (comment)

In fact I wouldn't mind going a step further, and just adopting Google's Java Style Guide:

• It's well documented and justification is provided for just about every rule.
• It's supported by checkstyle.
• There is a Maven plugin that will format existing code into that style.
• Google maintains code format definitions for both Eclipse and IntelliJ

[stevenschlansker]

I generally am OK with Google's style guide. However, some things are surprising (seems like they like 2 space indents? 4 seems very standard for Java) and I worry it's over-specified for what we care about. I can see us spending a fair amount of time messing around with Checkstyle and getting frustrated over anal interior spacing rules 😒

I will second that #474 showed Allman style to be very verbose in some cases. It really annoyed me on that PR in particular.

Do we have any data to back up the assertion of "Java programmers like K&R"? I found this: http://sideeffect.kr/popularconvention/#java
which seems to indicate K&R is twice as popular as Allman, and that 5% of coders are literally insane ;)

This will also cause merge pain from master to jdbi3 as we try to port features over.
I guess I lean towards "K&R style braces, Google Java Style highly encouraged but not 100% required, just make the code readable" - and we can enable whatever subset of checkers we find more useful than annoying.

[stevenschlansker]

I'll also go on the record as not really caring too much about this either way. I like readable code over debating about style rules :)

[qualidafial]

How about we table this for now. When we're ready to come back to this (toward the end of the release), I'll do a spike on switching to Google style, and then we can debate based on the observed impact.

[electrum]

We use a derivative of the Ning style (I'm guessing that's @martint style) in Airlift and Presto and is quite close to what Jdbi uses now. Our Checkstyle rules are good enough now that formatting nits are relatively rare in code review.

Having the checker run in Travis means that people see it and fix it before review. I think that, psychologically, people are much more accepting when this is automatic as part of the build. No one argues about style rules, they just notice the build failed and fix it. Highly recommended, no matter what style you choose.

IntelliJ: https://github.com/airlift/codestyle
Checkstyle: https://github.com/prestodb/presto/blob/master/src/checkstyle/checks.xml

[qualidafial]

Having looked into the google code formatter and the google code style preset in checkstyle--this is probably more trouble than it's worth.

@jdbi/contributors I'm good to close this if the rest of the team is in agreement.

[stevenschlansker]

All bikeshed colors matter :P