Friday, June 27, 2014

Google Java Style - mandatory braces rules

During development of Checkstyle configuration for Google Java Style,  we found two rules that are looks like conflicting . 

In 4.1.1 Google say:
Braces are used with if, else, for, do and while statements, even when the body is empty or contains only a single statement.

in 4.1.3 Google say:
An empty block or block-like construct may be closed immediately after it is opened, with no characters or line break in between ({}), unless it is part of a multi-block statement (one that directly contains multiple blocks: if/else-if/else or try/catch/finally).

So in 4.1.1 IF-ELSE could be {}, but in 4.1.3 there is unclear "unless" that either conflict with 4.1.1 or mean smth else that it not described.

After long discussion with friends and investigation :

Rule 4.1.1 tell us to always use braces for statements that use blocks

// BAD
if () {
  ...
} else
  i++;

It must be
// GOOD
if () {
  ...
} else {
  i++;
}

// it is OK, old fashioned but ok
int index = 0;
for (; index < s.length() && s[index] != 'x'; index++) {}

// OK, side effect in condition expression is bad design , but it is not matter of style/formatting 
while ((r = in.read()) != 0) {}

So ELSE is just another structure that could have braces so it have to have braces no matter what.


Rule 4.1.3 extend 4.1.1 and demand that braces should not be empty in multi-block statements, small wording conflict in ELSE case is not intentional and if apply rules in sequence 4.1.1 and then 4.1.3 all will be fine:

// BAD
try {
  doX();
} catch (Exception e) {}

// BETTER
try {
  doX();
} catch (Exception e) {
  // Here's why I'm swallowing this exception
}

// BAD
if (expr) {} else {
  doSomething();
}

// BETTER
if (expr) {
  // Here's why I'm not doing anything here....
} else {
  doSomething();
}

No comments:

Post a Comment