Reformat source code for consistent use of whitespace.

[heuermh]

Fixes #147.

I created a code style for Intellij IDEA (checked in at etc/IntelliJ IDEA Code Style.xml) and ran Reformat Code. All source code changes should be whitespace and braces only.

[coveralls]

Coverage decreased (-0.8%) to 62.13% when pulling 9293243 on heuermh:reformat-source-code into 710cfc8 on HadoopGenomics:master.

[fnothaft]

+1!

[tomwhite]

Thanks for doing this @heuermh!

The code style and particularly the whitespace is particularly inconsistent in this codebase, so I'm +1 on fixing it. I think we should do it in a major release, though, since it's quite a disruptive change.

Also, I think we should enforce the style by using checkstyle or similar. I started writing a PR a while ago to do this, it's incomplete, but you can see the idea here: https://github.com/HadoopGenomics/Hadoop-BAM/compare/master...tomwhite:checkstyle?expand=1

What do you think?

[heuermh]

Also, I think we should enforce the style by using checkstyle or similar.

I agree, and as an example have it configured here
https://github.com/heuermh/dishevelled/tree/master/compress/src/build/conf
https://github.com/heuermh/dishevelled/blob/master/parent/pom.xml#L315

We would need to sync the configuration in the source code formatter with that enforced by checkstyle.

ADAM formats source on the fly as part of the build, I think that might be more trouble than it is worth.

[tomwhite]

Great!

ADAM formats source on the fly as part of the build, I think that might be more trouble than it is worth.

Agreed.

[fnothaft]

+1

[coveralls]

Coverage decreased (-0.8%) to 62.539% when pulling 3a534d0 on heuermh:reformat-source-code into e4224ab on HadoopGenomics:master.

[tomwhite]

Rather than choosing our own formatting preferences, I would rather choose an existing style guide to use. The Google Java Style Guide looks like the one that is most used these days (the old Sun one has not been updated since 1999).

At first I thought we should use Checkstyle to enforce the style, but after playing with the Google checkstyle settings I found that they produce lots (hundreds) of warnings even for source code that has been formatted to adhere to the Google Java Style Guide. For example, it produces lots of warnings about there not being blank lines between import groups even though the guide says there shouldn't be blank lines.

So I think it would be preferable to use a tool that checks if the source is formatted correctly according to the Google Java Style Guide, and fail the build if it's not. This is such a tool: https://github.com/coveo/fmt-maven-plugin, and it's easy to integrate with the Maven build. The source can also be manually reformatted using mvn fmt:format if needed.

I've produce a patch to do this here: #182. Please take a look!

[heuermh]

The Google Java Style Guide looks fairly reasonable to me.

The only thing I don't like is the missing newline after }, I prefer

return () -> {
  while (condition()) {
    method();
  }
};

return new MyClass() {
  @Override public void method() {
    if (condition()) {
      try {
        something();
      }
      catch (ProblemException e) {
        recover();
      }
    }
    else if (otherCondition()) {
      somethingElse();
    }
    else {
      lastThing();
    }
  }
};

to

return () -> {
  while (condition()) {
    method();
  }
};

return new MyClass() {
  @Override public void method() {
    if (condition()) {
      try {
        something();
      } catch (ProblemException e) {
        recover();
      }
    } else if (otherCondition()) {
      somethingElse();
    } else {
      lastThing();
    }
  }
};

I could of course could be outvoted. :)

[fnothaft]

+1 to using the Google Java style guide.

[heuermh]

Closing as WontFix