Yassine Badache Yassine Badache - 1 year ago 363
Java Question

Sonar "useless assignment to local variable" workaround?

I am working towards improving my code, and I came across this issue from Sonar:

Remove this useless assignment to local variable "uiRequest"


Fact is, it is not useless, as I am using it just after in the code:

// I am supposed to remove this
UiRequest uiRequest = null;

if("Party".equals(vauban.getName())) {
uiRequest = contextBuilder.buildContext(vauban);
} else {
// Maybe I could work my way around here ?
throw new NamingException(
String.format(
"Hey %s, change your name to %s, thanks",
vauban.getName(), "Vauban"));
}

// Set the generated Id in the result of context builder
MyOwnService response = callService(uiRequest, vauban);

return response;


Sonar still tells me that "uiRequest" is useless, why ? It is not, as I don't want it to reach the code if it is null. I tried initializing it (
uiRequest = new UiRequest()
) but it keeps telling me that it is useless.

Anyone got an idea about why Sonar behaves like this / how to correct this ?

Answer Source

Your issue simplifies to:

Foo x = null;

if(a()) {
     x = b();
} else {
     throw new Exception();
}

c(x);

By the time you reach c(x), either x has been assigned b() or an exception has been thrown (so you don't reach c(x)). So whatever you assigned at first, is redundant.

You can fix this in two ways:

Firstly just removing the = null, leaving Foo x; - Java is clever enough to realise that all routes to c(x) involve an assignment, so this will still compile.

Better yet, move c(x) into the block:

if(a()) {
     Foo x = b();
     c(x);
} else {
     throw new Exception();
}

This is logically equivalent, neater, and reduces the scope of x. Reducing scope is a good thing. Of course, if you need x in the wider scope, you can't do this.

One more variation, also logically equivalent:

if(! a()) {
   throw new Exception();
}

Foo x = b();
c(x);

... which responds well to "extract-method" and "inline" refactorings:

throwForInvalidA(...);
c(b());

Use whichever communicates your intent best.

Recommended from our users: Dynamic Network Monitoring from WhatsUp Gold from IPSwitch. Free Download