Posted by: Camilo Torres | August 22, 2013

Meaningless code every now an then…

java code

java code with a meaningless condition

I found this piece of Java code, it is only a conditional:

if (banda.getField55().containsKey(“5F34”)
&& (!(banda.getElementValue(“5F34”).equals(“4E50”) && BcdLib
.hexStr_ascii(banda.getElementValue(“5F34”))
.equalsIgnoreCase(“NP”)))) {

In the first part of the condition evaluates if some field contains a value (a key). I think it is OK because this first part prevents that the access to that ‘key’ in the second part of the conditional raises a null pointer exception tipical to Java, this is to prevent that the access to an inexistent field throws an error.

Let’s get rid of the first part and stay only with the second part of the conditional:

(!(banda.getElementValue(“5F34”).equals(“4E50”) &&
BcdLib.hexStr_ascii(banda.getElementValue(“5F34”)).equalsIgnoreCase(“NP”)))

I think this always returns true and makes this part of the condition useless. Worst it introduces confussion to the code because the reader believes this code really does something usefull. This part of the condition is a negation, so if it always returns true, the inner part must always return false, let’s see if that happens:

banda.getElementValue(“5F34”).equals(“4E50”) &&
BcdLib.hexStr_ascii(banda.getElementValue(“5F34”)).equalsIgnoreCase(“NP”)

Then for this two part condition to be true, both conditions must be true (it’s an ‘and’ conditional). But here they are evaluating if one single value is equal to two different values at the same time, such condition can’t even happen, so this part of the sentence always return false. This proves the point that this part of the condition is meaningless.

This lead me to think that the original developer has no much idea of the rules he was trying to write into his code. O may be this is an error in the condition which must be an ‘or’ instead of an ‘and’? There is no way of knowing the answer right now.

There are other subtle issues in this code: Why one access the key through a getField55 in the first place, then access the same key through the banda object directly? Also, they are using this magic strings with hex values like 5F34, 4E50 and this NP string, What is the meaning of those values? Well, this is just a small part of a medium system made of spagetti code, so no surprise to me finding this kind of things, may be I start to write about those bits of bad codes or mistakes…


Leave a Reply

Fill in your details below or click an icon to log in:

WordPress.com Logo

You are commenting using your WordPress.com account. Log Out / Change )

Twitter picture

You are commenting using your Twitter account. Log Out / Change )

Facebook photo

You are commenting using your Facebook account. Log Out / Change )

Google+ photo

You are commenting using your Google+ account. Log Out / Change )

Connecting to %s

Categories

%d bloggers like this: