Top 3 tips for cleaner code – and why source code comments are like salt

In his book “Clean Code” Robert C. Martin has a section called “Explain yourself in Code”.

While refactoring one of our larger projects  I stumbled over the perfect example to illustrate that rule.

@Testvoid testCapabilities() {// Test if EXASOL dialect really has all capabilitiesfinal ExasolSqlDialect dialect =new ExasolSqlDialect(DialectTestData.getExasolDialectContext());final Capabilities caps = dialect.getCapabilities();assertEquals(PredicateCapability.values().length,caps.getPredicateCapabilities().size());}

As you can see in the snippet above the author of the JUnit test case felt inclined to explain what the code really does. And the reason for this is that it doesn’t do what the method name suggests.

There is a lot wrong with those few lines of code:

1. Obviously we’re not testing any capabilities. We test if an SQL dialect supports all existing capabilities.
2. When you look at the JUnit test log and the test fails, you can’t tell why.
3. The test only checks predicate capabilities, so the comment lies.
4. If someone fixes the code to match the method name, again the comment lies.

Fix the code instead of explaining it

The fix is quite simple: pick a proper name for the test method. Then delete the comment.

Remember you can’t break anything since test methods in JUnit are called via reflection, so you’re free to change their names as you please without creating backward compatibility issues.

After that we use JUnit5’s assertAll to combine multiple sub-tests into one. All of them are now executed, even if one sub-test fails

import static org.hamcrest.collection.IsIterableContainingInAnyOrder.containsInAnyOrder;import static org.junit.Assert.assertThat;import static org.junit.jupiter.api.Assertions.assertAll;// ...@Testvoid testExasolSqlDialectSupportsAllCapabilities() {final ExasolSqlDialect dialect =new ExasolSqlDialect(DialectTestData.getExasolDialectContext());final Capabilities capabilities = dialect.getCapabilities();assertAll(() -> assertThat(capabilities.getMainCapabilities(),containsInAnyOrder(MainCapability.values())),() -> assertThat(capabilities.getLiteralCapabilities(),containsInAnyOrder(LiteralCapability.values())),() -> assertThat(capabilities.getPredicateCapabilities(),containsInAnyOrder(PredicateCapability.values())),() -> assertThat(capabilities.getScalarFunctionCapabilities(),containsInAnyOrder(ScalarFunctionCapability.values())),() -> assertThat(capabilities.getAggregateFunctionCapabilities(),containsInAnyOrder(AggregateFunctionCapability.values())));}

While I was at it, I also improved the quality of the debug output and the readability of the assertion, by using a dedicated Hamcrest collection matcher.

Additionally, I turned the abbreviation “cap” into a real word. We don’t live in the old Basic 1.0 days anymore where you had to be careful with the length of your identifiers for performance reasons. Also,the abbreviation breaks the naming rule for collections. If you really feel you must abbreviate it, at least call it “caps”.

Don’t mandate code comments

About every company I have worked in or for initially had a mandatory amount of code comments in their coding guideline. Usually it was 30%.

Even if that is a painfully stupid idea, most companies like it because it is easy to measure.

Both in my statistics and measurement engineering classes our professors would always say: “You get what you measure.”

Thinking about mandating 50% comments in your companies code? Be prepared for gems like “This if-clause decides whether or not to convert names to upper case.”

Where comments are actually useful

Code comments should appear where they have a meaning – and when they’re  useful to the users of your code. For instance, you should spend special care in documenting the API of libraries that you create. Best case API users know after a short look at your Loading...JavaDoc, PerlDoc or Doxygen documentation how to use the API without having to read the code.

If on the other hand someone wants to work on the internal details of your code, you better make that code readable instead of trying to explain it in comments.

Source code comments are like salt

Salt can refine a dish — or make it unpalatable. Especially if you use too much or put it into the wrong dish. There is a reason why you usually don’t find strawberry and sea salt yogurt in a supermarket.

Source code comments are very much the same: use them scarcely and in the right place and you enhance the user experience of your APIs. Scatter them all over the place and have other developers curse you under their breath when they are extending your code.