Effective Groovy with CodeNarc 0.12
The new release of everyone’s favorite static anaylsis engine for Groovy is here. Time to download CodeNarc 0.12 while it’s hot. And in case this is your first time, CodeNarc analyzes your Groovy code and looks for potential problems and errors, creating a nice report or flagging the problem in your IDE. It’s like FindBugs, Lint, or PMD, but for Groovy.
So what’s new in 0.12? Performance improvements, 35 new analysis rules, and a new logo. It’s supposed to be an armadillo, but no one can tell. We’re calling it the CodeNarc Grue, as in “You are in a maze of twisty little groovy scripts, all alike.” Anyway, here’s the rundown:
Effective Groovy
A big focus of 0.12 is making sure you write groovy Groovy. Time to unlearn your Java and discover a new way of coding. Before reading the rules, you may want to take the quiz and see if you can spot what can be improved.
UnnecessaryCallForLastElement – Java developers get the last element of a list like this: “list.get(list.size()-1)”. In Groovy you can just call “list.last()” or “list[-1]“.
UnnecessaryCollectCall – The collect method is useful, but once you learn the spread operator then you use it a lot less. Constructs list “list.collect { it.toString() }” are better written as “list*.toString()”.
UnnecessaryGetter – Simple getters can be written as property access. So “strings.getProperty()” becomes “strings.property”
UnnecessaryNullCheck – One of my favorites. Null checks can be replaced with the null safe dereference. So the code “(prop != null && prop.isValid())” can be rewritten as “(prop?.isValid())”.
ConfusingTernary – Groovy’s conciseness means ternary and elvis operators fit better into the language, and feel more natural. The format is “condition ? true-clause : false-clause”. If your condition is a negative expression, then the ternary is pointlessly confusing. For instance, prefer “(prop.isActive() ? ‘alive’: ‘dead’)” to the backwards construct “(!prop.isActive() ? ‘dead’: ‘alive’)”.
One last quiz before we go. Spot the error?
UnnecessaryObjectReferences – Yep, long strings of setters and getters is standard Java. You don’t need to follow suit in Groovy. This code:
def person = new Person()
person.firstName = 'Hamlet'
person.lastName = "D'Arcy"
person.employer = 'Canoo'
person.street = 'Kirschgaraten 5'
person.city = 'Basel'
person.zipCode = '4051'Is better written as:
def person = new Person().with {
firstName = 'Hamlet'
lastName = "D'Arcy"
employer = 'Canoo'
street = 'Kirschgaraten 5'
city = 'Basel'
zipCode = '4051'
}
I See (More) Dead Code
The previous version introduced dead code analysis: flagging code that is meaningless or cannot be reached. It’s been expanded in three directions with version 0.13. Before reading on, you may want to take the Dead Code Quiz and see how you do.
UnnecessaryCatchBlock – If your catch block does nothing but throw the original exception, then you should delete the catch block. This happens when you either forget to add an error logging statement or forget to back out some temporary testing code.
UnnecessaryNullCheckBeforeInstanceOf – An instanceof check on null always returns false. So constructs like “if (x != null && x instanceof Class)” can always be simplified to just “if (x instanceof Class)”.
UnusedPrivateMethodParameter – If a method parameter to a private method is not referenced from the body of the method, then you should delete it and clean up the API.
Cleaner Junit Tests
Some new rules were introduced to make your JUnit (ie. GroovyTestCase) based tests cleaner and conciser. Take the quiz or just read on.
UseAssertFalseInsteadOfNegation – The code “assertEquals(false, property)” should be rewritten as “assertFalse(property)”. It’s clearer that way.
UseAssertTrueInsteadOfNegation – The inverse of the previous rule… rewrite “assertEquals(true, property)” as “assertTrue(property)”
JUnitTestMethodWithoutAssert – Ever write a test method and forget to make asserts? I have. A test method without assertions
Better Designed Objects
I read somewhere that there are 13 different distinct types of OO inheritence. Different books promote different ones as best practices. Should you use abstract classes? abstract methods? final classes? Our should you bag it all and learn to program functionally? If you’re stuck in OO, then these rules might help you write better obejcts.
AbstractClassWithoutAbstractMethod What’s the point of an abstract class without an abstract method? You could just give the object a protected constructor instead, which mostly achieves the same thing. Some people love this rule. Go figure.
EmptyMethodInAbstractClass – What a sec… you wrote an empty method in an abstract class? It’s clearer to make the class abstract because an empty method pretty much guarantees that the contract of the method is undiscoverable from the type of the object. Again, it’s your choice to use this rule or not.
ConstantsOnlyInterface rule – Interfaces are not a good place to put a bag of constants. Put them in a class and use static imports instead of subclassing to bring them in scope.
FinalClassWithProtectedMember – A final class cannot be subclasses. A protected method can be subclassed. It’s pointless to mix the two. Make the method @PackageScope or make the class non-final
Logging
Logging shouldn’t be hard. Yet a whole gaggle of frameworks exist to help you with it. CodeNarc 0.13 makes sure you do logging correctly with 4 new rules. Take the test or just read on.
LoggerForDifferentClass – You typically instantiate a logger using the enclosing Class name. This rule triggers if you refer to a different class, which is usually a copy and past error.
LoggerWithWrongModifiers – Loggers should be private, static, and final. If it’s not then you’ll get a nice little warning
LoggingSwallowsStacktrace – Passing an Exception object to a Logger.error(Object) method typically does not log a stack trace. Did you know that? CodeNarc will remind you.
MultipleLoggers – A class with two loggers is usually a copy and paste error
Unnecessary Instantiations
In Groovy, there is typically no need to directly instantiate BigDecimal, Double, Float, or Integer types. 0.12 has several new rules to make sure you remember. Plus this gem:
UnnecessaryInstantiationToGetClass There is no need to create an instance if you just want the Class reference. Replace code like this: “new MyObject().getClass()” with the literal “MyObject.class”
Common Errors
And now the leftovers. These new rules find common errors that catch out Groovy beginners. Don’t get caught out!
MissingNewInThrowStatement – The Groovy syntax lets you throw things that don’t compile in Java, like: “throw RuntimeException()” Hey, where is the new keyword? You forgot it, but CodeNarc won’t.
SimpleDateFormatMissingLocale – If you need to use SimpleDateFormat then specify a Locale in the constructor. It’s a best practice darn it!
CompareToWithoutComparable – If you implement a compareTo method, but don’t implement the Comparable interface, then you get funny results for the == operator in some versions of Groovy. If you implement compareTo then implement Comparable as well
CloseWithoutCloseable – Dude, please implement Closeable if you have a close method. It’s a pain if you don’t for us Java developers.
RequiredString – Every class is required to have a certain String present. For instance: a copyright statement. This rule has proven useful many times to me.
ExplicitGarbageCollection – Don’t call garbage collection explicitly. ‘Nuff said.
CatchArrayIndexOutOfBoundsException Why are you catching ArrayIndexOutOfBoundsException? Surely you can change the code to not rely on exceptions being thrown and instead proeprly use an index variable.
CatchIndexOutOfBoundsException rule See above. There is typically no need to catch this Exception type in practice.
The Future
0.13 is on it’s way. There are already 10 new rules in 0.13 (by the way, Snapshot downloads are available from Canoo). We’re going to continue working on new rules and making the project better. There is another project called Groovy Lint as well. There are only a handful of rules in GroovyLint, but it integrates into Groovyc instead of run as a separate tool. If you’re curious you may want to check it out. That’s all for now. See you in the 0.13 release!











Alexey Sergeev said,
January 28, 2011 @ 8:19 am
def person = new Person()
person.firstName = ‘Hamlet’
person.lastName = “D’Arcy”
person.employer = ‘Canoo’
person.street = ‘Kirschgaraten 5′
person.city = ‘Basel’
person.zipCode = ’4051′
This needs to be written in the following form:
def person = new Person(
firstName: ‘Hamlet’,
lastName: “D’Arcy”,
employer: ‘Canoo’,
street: ‘Kirschgaraten 5′,
city: ‘Basel’,
zipCode = ’4051′
)
So you don’t need additional closure ‘with’.
Hamlet said,
January 28, 2011 @ 8:49 am
@Alexey That only works if Person were implemented in Groovy and has a Map constructor. If your domain model is implemented in Java then you need the closure.
Alexey Sergeev said,
January 28, 2011 @ 10:12 am
@Hamlet I don’t know where you found this information.
But just do the simple test as you’ll see that you are wrong.
Here is an example that perfectly works::
Pen.java
***********
public class Pen {
private String color;
public String getColor() {
return color;
}
public void setColor(String color) {
this.color = color;
}
}
PenScript.groovy
********************
def pen = new Pen(color: “red”)
println pen.color
Everything works. Java class that has no constructors at all.
An Army of Solipsists » Blog Archive » This Week in Grails (2011-04) said,
January 31, 2011 @ 2:03 am
[...] Effective Groovy with CodeNarc 0.12 [...]
Hamlet said,
February 25, 2011 @ 4:07 pm
sorry, your comment hit our spam filter.
You are correct.