Among others, one of our checks is that there should not be empty block of code, like catch block.
And then we had our first rule violation. For testing of course we are using JUnit. Standard design of JUnit 3.8 test to verify that certain part of code throws exception looks like this:
try {
performExceptionThrowingOperation(...)
fail();
} catch(ExpectedException excepiton) {
// expected exception
}
Well as you can see catch block is empty, there is no java code in it. And of course checkstyle complained about this.
I see two options here:
- leave code as it is and do not fix checkstyle violation
- add nonsense line of code like assertTrue(true)
But now I am getting to the point of my thoughts. As code grows, there will be more and more places where maybe it will be strange (e.g. adding assertTrue(true)) to fix checkstyle violation and not possible to solve it with simple additional configuration of checkstyle rule.
Question is: should we leave checkstyle violation as it is or add strange code of assertTrue(true) style?
My opinion is that we should add strange code and fix checkstyle violation. If we start to leave checkstyle violations open, number of opened checkstyle violations will grow with time and checkstyle violations warnings will loose their intention warning about bad code design. Because if there is long existing list of false checkstyle warnings we may miss the new, but important one. And then, instead of fixing the bug immediately, maybe we will fix it in later testing phases. From my point of view, it is better to have some strange but harmless code lines than to miss one important bug because of long list of existing checkstyle warnings.
If somebody will ever read this :) it would be nice to hear your opinion.
1 comment:
exception = null; works for me.
Post a Comment