Archive for October, 2012

Through the eyes of sonar: Immutable Objects.

Sonar is an open platform to manage code quality. Sonar is combining various java quality tools and collect these possible defects as violations. Let’s see how sometimes we misuse immutable objects.

Immutable Objects

I’m continuing the “Through the eyes of sonar” series with all these immutable objects you use all the times.
Immutable objects are simply objects whose state (the object’s data) cannot change after construction.

Immutable objects greatly simplify your program, since they :
— are simple to construct, test, and use
— are automatically thread-safe and have no synchronization issues
— do not need a copy constructor
— do not need an implementation of clone
— allow hashCode to use lazy initialization, and to cache its return value
— do not need to be copied defensively when used as a field
— make good Map keys and Set elements (these objects must not change state while in the collection)
— have their class invariant established once upon construction, and it never needs to be checked again
— always have “failure atomicity” (a term used by Joshua Bloch) : if an immutable object throws an exception, it’s never left in an undesirable or indeterminate state

Examples of immutable objects
— from the JDK include String, Integer, BigDecimal, Boolean, etc.
— from joda time : DateMidgnight, DateTime,…

More on this topic : ImmutableObjects


Integer Instantiation

Avoid instantiating Integer objects. Call Integer.valueOf() instead. This allows the class to give you back an existing instance if an object was already created for this number. Note this optimization that it’s more for small values of integer and depends on the jre implementation.

 return new Integer(rowCount);

Prefer :

 return Integer.valueOf(rowCount);


Why we don’t use Doubles for Financial Calculations

    package com.bloodredsun;

    public class DoubleOperation {

        public static void main(String[] args) {

            double t1 = 10.266d;
            double t2 = 10.0d;

            //Outputs 0.266

            double h1 = 100.266d;
            double h2 = 100.0d;

            //Outputs 0.26600000000000534

Ouch! That is not what we want but it is the classic behavior of doubles. The inability to represent some decimals in the IEEE-754 format (as binary fractions) causes this. If we want correct precision the answer is to use BigDecimals but we have to remember to use Strings in the constructors or you end up with the same issues that you were trying to avoid.

    package com.bloodredsun;

    import java.math.BigDecimal;

    public class BigDecimalOperation {

            public static void main(String[] args) {

            BigDecimal t1 = new BigDecimal("10.266");
            BigDecimal t2 = new BigDecimal("10.0");

            //Outputs 0.266

            BigDecimal h1 = new BigDecimal("100.266");
            BigDecimal h2 = new BigDecimal("100.0");

            //Outputs 0.266

Notice the usage of the string constructor ! You will read more about it in the the next paragraph about java.math.BigDecimal.


Avoid Decimal Literals In Big Decimal Constructor

As the BigDecimal class is immutable, all its methods modifying its value return a new BigDecimal object. The newly created object contains the value modified.

  BigDecimal dec = new BigDecimal("123.54");
  BigDecimal dec2 = new BigDecimal(123.54);
  System.out.println("dec value = " + dec);
  System.out.println("dec2 value = " + dec2);

Result displayed in the console:

 dec value = 123.54
 dec2 value = 123.5400000000000062527760746888816356658935546875

As Java doesn’t parse and represent these two numbers in the same way, the result is different.
For this reason, when you use a BigDecimal object, create it with the constructor using a String in argument.

  BigDecimal dec = new BigDecimal(123.54);
  System.out.println("dec value = " + dec);
  BigDecimal dec2 = dec.setScale(2, BigDecimal.ROUND_HALF_UP);
  System.out.println("dec2 value = " + dec2);

Result displayed in the console:

  dec value = 123.5400000000000062527760746888816356658935546875
  dec2 value = 123.54

Useless Operation On Immutable

As the setScale(newScale, roundingMode) method is used on an immutable object, it returns a new BigDecimal object with the modified value. If you don’t put the result of this method in another BigDecimal object, you will not able to work on this new value.
Remember that the example above is valid for all methods involving the modification of a BigDecimal object’s value.

  BigDecimal dec = new BigDecimal("123.54");
  dec.add(new BigDecimal("1.0"));

This will display :


Fixing by reassigning the dec variable :

  BigDecimal dec = new BigDecimal("123.54");
  dec = dec.add(new BigDecimal("1.0"));
  dec = dec.add(BigDecimal.ONE);
  dec = dec.multiply(BigDecimal.TEN);

This will display :


All mathematical methods of the BigDecimal class ask you to give in argument a BigDecimal object containing the value to apply on the value of your BigDecimal object. To avoid the instantiation of a BigDecimal object just to add one or multiply your numbers by ten (as in the example), you can use the constant defined in the BigDecimal class. Use them to avoid creating multiple objects in memory.


Use Joda Time

Don’t use the mutable java built-in classes for date/time manipulation, prefer the use the immutable and fluent api of jodatime.

Useless Operation On Immutable

DateTime is also an immutable class. It suffers of the same advantages and inconveniences of the BigDecimal class.
When you use the class, don’t forgot to provide an object to receive the result of your modification or they will be be never apply (cf. the following example) :

public void testAddMonth() throws Exception {
  DateTime time = new DateTime();

Result displayed in the console :


No month added… damned.



Avoid Instantiation

The Boolean class is used instead of the primitive type in order to offer the possibility to have three values for a boolean (true, false and null). If you just need to use the classical true-false value, use the primitive type.

Use primitives if you can, wrappers if you must

  new Boolean("true"); // bad practice : this will create always new instances prefer Boolean.valueOf(...)

Prefer :


If it is possible, use the constant defined in the Boolean class instead of creating a new object.

  if (x > 10){
  else {
  Boolean.valueOf(x > 10);

The valueOf(boolean) method can receive in argument a condition which has as result true or false. As you can see in the example before, you can resolve the if-else code by using the valueOf(boolean) method.


Useless Operation On Immutable

String name = "David";
name.replaceAll("D", "d");
System.out.println("BAD   "+name);
name= name.replaceAll("D", "d");
System.out.println("GOOD  "+name);
BAD David
GOOD david

Correctness – Method ignores return value

I stumble upon this code

String key = ...

Either we forgot something to the index char returned or we can simply drop the call from the codebase.

Avoid String To String

Avoid calling toString on String

    StringBuffer buffer = new StringBuffer(4);
    if (socialState != null) {
	buffer.append(" ");

Simply :

    StringBuffer buffer = new StringBuffer(4);
    if (socialState != null) {
	buffer.append(" ");

Note also that in this code we should have used StringBuilder. Check this article :
Using StringBuffer instead of StringBuilder

Performance – Method invokes inefficient new String(String) constructor

Avoid creating a new string :

   protected String getCode(final Code code) {
     return new String(code.getValue() + " " + getLabel(code));

Simply :

 protected String getCode(final Code code) {
     return code.getValue() + " " + getLabel(code);

Correctness – “.” used for regular expression

The dot is a special character in regexp. Using String.split(regexp)… will not split

 String[] splitKeys = this.propertyDescriptor.getClassDescriptor().getName().split(".");

A simpler and safer way is to use StringUtilsor the StringTokenizer where the dot is processed as a normal character

 String[] splitKeys = StringUtils.split(this.propertyDescriptor.getClassDescriptor().getName(),".")

Inefficient String Buffering

Note that string concatenation is often badly coded… some myth tells you that StringBuffer or StringBuilder are the way to go. But one thing that most developers don’t know… is that the compiler is smarter than you 😉


will be compiled to :

 new StringBuilder("4564654").append(sdfds).append("1231321").toString()

In some cases these optimizations are not enough, specially when looping on a list of records :

strBuff.append("; " + contact.targetid);
strBuff.append("; " +;
strBuff.append("; " + task.dueDate);
strBuff.append("; " + task.objectId);
strBuff.append("; " + task.statusId);


public static final String SEPERATOR = "; "


NullSafety and trim/blank

Instead of :

  if  (anAddress.getZipCode() == null || "".equals(anAddress.getZipCode().trim()))
     return false;

Prefer the less error prone version using StringUtils :

  return StringUtils.isNotBlank(anAddress.getZipCode());


Through the eyes of sonar: Exception handling

Sonar is an open platform to manage code quality. Sonar is combining various java quality tools and collect these possible defects as violations. Let’s see some exception handling worst practices detected by these tools.

Avoid Print Stack Trace

Instead of :

catch (Exception e) {

This exception will often get logged in a separate file (System.err) making debugging more complex than it needs.
Use a logger call instead.

catch (Exception e) {
  logger.warn("failed to ...." ,e);

Note the ,e this will tell log4j/slf4j/… to log the full stacktrace. Note that you can configure sfl4j to log the root cause first

Avoid logging only exception message

catch (Exception e) {
  logger.warn("failed to ...."+e);

Use the “,” variant of the logging method. In case of null pointer… the log will looks like :

 failed to ....null

This isn’t really helpful for debugging. So please use the other method :

catch (Exception e) {
  logger.warn("failed to ...." , e);

UseCorrectExceptionLogging and other pmd rules

Preserve Stack Trace

Intead of loosing track of the InstantiationException

 } catch (InstantiationException e) {
     throw new HibernateException("Could not instantiate resultclass: " + resultClass.getName());

Preserve the stacktrace with a nested exception constructor :

 } catch (InstantiationException e) {
     throw new HibernateException("Could not instantiate resultclass: " + resultClass.getName() ,e);

Avoid Catching Throwable

A catch statement should never catch throwable since it includes errors (OutOfMemoryError,…) :

 catch (Throwable t) {
  log.warn("unmonitored problem",t);
  success = false;

So do this only in rare case (dictated by an api) where severe problem should catched and logged. If you are forced to do so ensure that the error is logged !

Prefer runtime exception, avoid signature that declare throws Exception

It is unclear which exceptions can be thrown from the methods. It might be difficult to document and understand the vague interfaces.

public Set getChildrenNames(Fqn arg0) throws Exception {

Use a class derived from RuntimeException. This also often force the user of your API to catch these exception and as you can saw it’s no easy to get right !
Checked exceptions I love you, but you have to go

A history of exceptions — Exceptions and how they should be handled always end in heated debates between Java developers. It isn’t surprising that Hibernate has some noteworthy history as well. Until Hibernate 3.x, all exceptions thrown by Hibernate were checked exceptions, so every Hibernate API forced the developer to catch and handle exceptions. This strategy was influenced by JDBC , which also throws only checked exceptions. However, it soon became clear that this doesn’t make sense, because all exceptions thrown by Hibernate are fatal. In many cases, the best a developer can do in this situation is to clean up, display an error message, and exit the application. Therefore, starting with Hibernate 3.x, all exceptions thrown by Hibernate are subtypes of the unchecked Runtime Exception, which is usually handled in a single location in an application. This also makes any Hibernate template or wrapper API obsolete.

Avoid Instanceof Checks In Catch Clause

Each caught exception type should be handled in its own catch clause. For example SQLException a subclass DataTruncation that contains more information and in separate catch you can log the parameter that exceeded the db length, but you will probably share a common method that will handle the rollback of the transaction.

Avoid Throwing Null Pointer Exception

NPE means programmer’s mistake and if you throw NPE you are not a nice person.
NPE may be a convention in the JDK, but perhaps it should stay there too.

if (flowName == null) {
   throw NullPointerException("FlowName cannot be null.");

Throwing NPE is really confusing, prefer IllegalArgumentException :

if (flowName == null) {
   throw IllegalArgumentException("FlowName cannot be null.");

A shorter version using spring assert :

import org.springframework.util.Assert;
Assert.notNull(flowName, "FlowName cannot be null.");

Check also commons-lang Validate or Guava Preconditions

Avoid Throwing Raw Exception Types

Rather than throw a raw RuntimeException, Throwable, Exception, or Error, use a subclassed exception or error instead.

    if (StringUtils.isEmpty(props.getString("smtp.config"))) {
      throw new RuntimeException("Bad Configuration for smtp.config");

Prefer throwing specific exception like :

    if (StringUtils.isEmpty(props.getString("smtp.config"))) {
         throw new BadConfigurationException("smtp.config");

Correctness – Exception created and dropped rather than thrown

Silly mistakes. We are human after all… luckily sonar is there to spot it !

This code creates an exception (or error) object, but doesn’t do anything with it. For example, something like

    if (x < 0)
      new IllegalArgumentException("x must be nonnegative");

It was probably the intent of the programmer to throw the created exception:

    if (x < 0)
      throw new IllegalArgumentException("x must be nonnegative");

Empty Try Block

The less you have code to maintain… the less work you have. just drop this kind of code

public void run() {
	try {
	} finally {
		try {
		} catch (Exception e) {

Bad practice – Method may fail to close stream on exception

Stream from access,URLConnection,… should be closed whatever happens in a finally clause.
This will prevent also Too many open files

  File f = new File(parent.getAbsolutePath(),"TEST-"+getTestSuiteResult().getModuleName()+".xml");
    try {
	BufferedWriter bw = new BufferedWriter(new FileWriter(f));
	bw.write("<!--?xml version=\"1.0\" encoding=\"UTF-8\" ?-->" + ENDL);
	bw.write(getTestSuiteOpenTag() + ENDL);
	for (UnitTestCaseResult tc : getTestSuiteResult().getTestCaseResults()) {
 	    bw.write(getTestCaseTag(tc) + ENDL);
	bw.write(getTestSuiteEndTag() + ENDL);
    } catch (IOException e) {
	throw new RuntimeException(e);

To fix use the to finally close the writer

    File f = new File(parent.getAbsolutePath(),"TEST-"+getTestSuiteResult().getModuleName()+".xml");
    BufferedWriter bw = null;
    try {
	bw = new BufferedWriter(new FileWriter(f));
	bw.write("<!--?xml version=\"1.0\" encoding=\"UTF-8\" ?-->" + ENDL);
	bw.write(getTestSuiteOpenTag() + ENDL);
	for (UnitTestCaseResult tc : getTestSuiteResult().getTestCaseResults()) {
 	    bw.write(getTestCaseTag(tc) + ENDL);
	bw.write(getTestSuiteEndTag() + ENDL);
    } catch (IOException e) {
	throw new UnitTestCaseGenerationException(e);
    } finally {

If you are lucky to use jdk7 take a look at the The try-with-resources Statement.


Web’s fear and maven

All is well and good on your jetty or tomcat servers, then one of your client want to deploy your application in websphere application server, and trouble begins

 - JNDI lookup for datasources
 - Classloading mess
    - Verbose classloading and parent last
    - Jboss tattletale
    - Cleanup undesired dependencies
        - Maven exclusions
        - Correct scope
        - Maven war plugin : packagingExcludes
        - Patched jar
    - Keep it clean
         - maven-enforcer-plugin and friends
         - Combining Groovy-Sonar-Jenkins

Jndi lookup

If your client plan to use websphere, may be he wants to use the built-in websphere datasource, an implementation collecting various statistics about connection, prepared statement,…
You probably want to keep your jetty/tomcat compliance and if you are in webphere switch to the specific implementation (jndi datasource, jta transaction manager,…)
You can use the spring profiles to lookup your datasource via jndi instead of using dbcp or another datasource implementation.

	<bean id="dataSource" class="org.springframework.jndi.JndiObjectFactoryBean" abstract="false"
		<property name="lookupOnStartup" value="false" />
		<property name="cache" value="true" />
		<property name="proxyInterface" value="javax.sql.DataSource" />
		<property name="expectedType" value="javax.sql.DataSource" />
		<property name="jndiName" value="java:comp/env/jdbc/MyDataSource" />

If you plan to use jta transactions, and multiple datasources/queues, don’t forget to use XA transactions or tweak your transactions to avoid mixing access in a single transaction (and design it for possible data loss).
Also reduce the isolation level through the datasource property webSphereDefaultIsolationLevel (the default one is repeatable read).
If you have long running transaction like quartz job, test them extensively.

Classloading mess

We are in 2012… osgi is there since a long time and I’m still struggling with websphere and its bundled xerces.

Verbose classloading and parent last

To diagnose classloading issues like NoClassDefFound, Violation Constraint,… you can enable the verbose classloading.
To minimize the side effect of the bundled jars in websphere you setup the classloader policy of your application and module to parent last.

Jboss tattletale

I know that it’s ironic but this tool developed by JBoss will save you hours of trial and errors.
To audit your web-inf/lib, jboss tattletale is THE tool to identify :
– undesired dependencies like the one bundling javax.** classes
– duplicate jars (often due to maven relocation)
– duplicate classes


Launch mvn clean package
Then take a look at the report, you will perhaps discover duplicates classes like the one from commons-logging and use jcl-over-slf4j

or duplicate quartz jar :


and many other undesired dependencies.

Cleanup Undesired dependencies

Maven exclusions

Since maven 2.x resolves dependencies transitively, it is possible for unwanted dependencies to be included in your project’s classpath. Projects that you depend on may not have declared their set of dependencies correctly, for example. In order to address this special situation, maven 2.x has incorporated the notion of explicit dependency exclusion. Exclusions are set on a specific dependency in your POM, and are targeted at a specific groupId and artifactId. When you build your project, that artifact will not be added to your project’s classpath by way of the dependency in which the exclusion was declared.


Correct scope

For example excluding test artifact by specifying the correct scope.


Exclude jdbc drivers by defining them as provided (idem for your datasource implementation)




In extreme case… putting exclusions is just too long and boring. Configuring the maven war plugin to exclude the jar can be a faster way but remember that if this dependency breaks something in your application, it’s still there in your unit tests.


Patched jars

Some open source jars bundles multiple times the same classes, for example org.w3c.dom.UserDataHandler is bundled in xom, jaxen and many more.
This interface was also bundled in websphere and two jars in the web-inf/lib, one of them was sealed leading to java.lang.LinkageError: loading constraint violation: loader.
So I removed them from the jar and upload a xom-1.1.patched.jar to the corporate maven repository. It’s really ugly but it’s working.

Keep it clean

maven-enforcer-plugin and friends

Maven provide a default rules to enforce some rules, on[e] of them is for banneddependencies.

But there is another set of rules provided by the pedantic pom enforcer

Have you ever experienced symptoms like headaches, unfocused anger or a feeling of total resignation when looking at a Maven project where everybody adds and changes stuff just as they need? Do people call you a “POM-Nazi” if you show them how to setup proper and well organized projects?

If so, the Pedantic POM Enforcers are absolutely the thing you need!

An you have also an extra rule set @codehaus

Combining Groovy-Sonar-Jenkins

It’s quite easy to create a small groovy script that
– will check the jars in web-inf/lib against a baseline list
– failed the build or if you are less paranoid…
– send a mail to your team,
– or just contribute to a sonar manual measure

Let’s define our baseline, for some jar you want to get noticed if a different version is bundled, for your module you accept any version.
And use this baseline as a whitelist if it’s a different version or if there’s no match then it’s a new dependency -> requires to test a websphere deployment.


Then create the manual measure in sonar

You can define a manual measure

And now the groovy script to analyze the latest war file and post the manual measure to sonar and send you a mail 😉 :


//authenticated post
def postSonarMeasure = { resource,metric,val, sonarhost,token ->
	def script = "resource=${resource}&metric=${metric}&val=${val}&text=fromgroovy&description=fromgroovy";
	println script
	URL url = new URL("${sonarhost}/api/manual_measures?"+script);
	URLConnection conn = url.openConnection();
	conn.setRequestProperty ("Authorization", "Basic ${token}")
	OutputStreamWriter wr = new OutputStreamWriter(conn.getOutputStream());
	result=  conn.getInputStream().getText()
	println 'metrics created '+result;
	return result

def sonar = ''
def mavencoordinate=''
def token = 'myuser:mypassword'.bytes.encodeBase64().toString()

// <not_supported/>  
// 1. define the metrics
// 2. add a measure manually
// 3. launch an analysis
// 4. check the data through api/manual_measures
def postSonarUnverifiedWebInfJars = { value ->

def getActualContentOfWebInfLibFromLastestWar ={
	// find latest war file in target directory
	fileWar = new File("./target").listFiles().findAll(){ it.getName().endsWith('.war')}.sort() { a,b ->
		a.lastModified().compareTo b.lastModified()
	println "Checking WEB-INF/lib from "+fileWar.canonicalPath;
	//and create actuals with content
	ZipFile file = new ZipFile(fileWar)
	actuals = file.entries().collect { entry -> if (entry.getName().startsWith('WEB-INF/lib/')) return entry.getName().substring('WEB-INF/lib/'.length()) }
	actuals = actuals.findAll {it!=null && !it.equals('')}
	return actuals

def getBaseLine = {
	new File("./baseline.txt").eachLine { if (!it.trim().isEmpty())allowed.add(it) }
	return 	allowed
	actuals =getActualContentOfWebInfLibFromLastestWar();
	allowed =getBaseLine();

	println "************************************ "
	println "actuals "+actuals.size()
	println "allowed "+allowed.size()
	println "************************************ "

	unallowed = [];
	unmatched = [];

	allowedNonMatching = [];

	actuals.each { actual ->
		ok = allowed.find() { allow ->
			boolean match= (actual =~ '^'+allow)
			if (match) {
				println "matching " +actual +" "+ allow
			return match;
		if (ok==null) {
			unallowed.add("unmatched dependencies ! '${actual}' ")
			println "unmatched dependencies ! '${actual}' "
	if (!unallowed.isEmpty() || !allowedNonMatching.isEmpty) {
		def msg =  "The ${project} problem dependencies : \n"+unallowed.join('\n')+" \n add exclusions or adapt baseline.txt check if websphere deployment is ok.\nplease.\n"+actuals.join('\n');
		 ant = new AntBuilder()
		 ant.mail(mailhost:'', subject:"${project} : undesired dependencies detected !" ,tolist:''){

		println msg.toString()
	println "************************************ unused constraint from baseline.txt"
	allowedNonMatching.each {println it}
	println "*************************"
	println "************************************ append content to baseline.txt"
	unmatched.each {println it}
	println "*************************"

Enable the run of this scripts via maven plugin in a dedicated profile


or via jenkins groovy post scripts.


1 Comment