java code with a meaningless condition

Encontré esta pieza de código Java, sólo es un condicional:

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

En la primera parte de la condición se evalúa si algún campo contiene un valor (una clave). Creo que está bien, porque esta primera parte impide que el acceso a la “clave” en la segunda parte de la condición genere una excepción de null pointer típica de Java, esto es para evitar que el acceso a un campo inexistente genere un error.

Vamos a deshacernos de la primera parte y quedarnos sólo con la segunda parte de la condicion:

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

Creo que siempre devuelve true por lo que esta parte de la condición es inútil. Lo peor es que introduce confusion al código porque el lector cree que este código hace algo realmente útil. Esta parte de la condición es una negación, por lo que si siempre devuelve true, la parte interior siempre debe devolver false, vamos a ver si eso ocurre:

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

Para que esta parte de dos condiciones sea verdad, ambas condiciones deben ser verdaderas (es una “y” condicional). Pero aquí están evaluando si un solo valor es igual a dos valores diferentes al mismo tiempo, tal condición no puede suceder nunca, por lo que esta parte de la frase siempre devuelve false. Esto demuestra el punto de que esta parte de la condición carece de sentido.

Esto me lleva a pensar que el desarrollador original no tenía mucha idea de las reglas que él estaba tratando de escribir en su código. O puede ser que esto es un error en la condición que debe ser un “o” en lugar de una “y”? No hay manera de saber la respuesta al día de hoy.

Hay otros problemas sutiles en el código: ¿Por qué un acceso a la clave a través de getField55, en primer lugar, a continuación, acceder a la misma clave a través del objeto banda directamente? Además, están utilizando estos valores mágicas hexadecimales como 5F34, 4E50 y ese string NP, ¿Cuál es el significado de esos valores? Bueno, esto es sólo una pequeña parte de un sistema hecho de código espagueti, así que no me sorprendió encontrar este tipo de cosas, puede ser que empiece a escribir sobre esos trozos de malos códigos o errores …

Meaningless code every now an then…

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…