Las peores prácticas (The worst practices) en lunes, septiembre 01, 2008

Muchos de nosotros llevamos desde días, meses, incluso años viendo código fuente, arreglando código fuente, o buscando un bug. Así que no es novedad habernos topado con más de un código que nos ha dejado literalmente boquiabiertos (sea el nuestro o el de otra persona).

Es por ello que me di el trabajo de pensar día a día qué códigos hice mal, qué códigos arreglé y qué bugs encontré.

Así que enumeremos, de menor a mayor, uno a uno con un nombre característico.

1. ¿Qué significa ese número?

public class DoesntHaveConstant {

public double getIGV(double precio){

return precio*0.19;

}

}

Muchas veces tenemos la manía de colocar código de esta forma, de repente diciéndonos “luego lo coloco en una constante” y ese “luego” nunca llega. ¿Por qué está mal? Me preguntaron muchas veces, y no existe una sola razón, sino varias: “si alguien que no eres tú ve ese código, no sabrá qué significa 0.19”, “¿Cuántas veces se repite ese 0.19 en todo el programa?¿Y si alguna vez cambia, le hago replace?” y la más importante “Si ese valor quiere ser modificado en el tiempo por un administrador y no un programador?”. Lo peor de todo que un día escuché a alguien responder a la última pregunta con un “Le enseño a programar pes”. Osea PLOOOOOOOOOOOP XD. La solución es algo simple, usar constantes y en caso de que se pueda modificar el valor de forma manual por alguien que no sabe programar, usar un properties o sino una interfaz para modificar este valor =).

2. ¿Qué hace esto?¿x1, x2 y x3?¿Versiones?

public class WhatIsThis {

private int x1;

private int x2;

private int x3;

public int doSomething(){

return x1*x2 + x3;

}

}

A la clase le iba a poner “WTFIsThis”, pero mejor no =P. Es obvia la razón de por qué está mal, pero por si es que alguien no se ha dado cuenta la diré: “¿Alguien sabe para qué sirve doSomething?”. Está bien que muchas veces venga un método con ese nombre en los libros de los cuales uno estudia, pero no se debería de crear una clase que no tiene un nombre descriptivo, que tiene variables que no te dicen nada y peor aun, un método que te dice que hace algo, pero……no sabes qué es. La solución es muy simple: nomenclatura. Hay que aprender a nombrar a las clases, variables, métodos y demás.

3. ¿Esos no son datos de una persona?

public class PersonaDAO {

public void save(String pNombre, String sNombre,

String apePaterno, String apeMaterno, String telefono, int edad, String email, Date fechaNacimiento, String direccion){

//Here is the code

}

}

Esto es algo más complejo de ver, pues uno muchas veces está acostumbrado a trabajar de la forma “funciona y listo”, sin abstraernos un poco a la POO. Es decir, si lo que va grabar el DAO, es un objeto Persona, y conociendo que…..un objeto Persona puede contener todos esos datos, por qué necesitamos pasar toooooooooooodos los datos en lugar de pasar solo un objeto de la clase Persona, debidamente llenado, y grabarlo directamente a la BD. La solución ya fue implícitamente mencionada.

4. Sin comentarios…..

if (accion.equalsIgnoreCase(CReclamo.RECLAMO_ACCION_GRABAR)){

// nothing

}else if (accion.equalsIgnoreCase(CReclamo.RECLAMO_ACCION_ENVIAR)){

SirectMovimientoSolReclamo movSolicitudVerificacion1 = generaMovimiento(CReclamo.ESTADO_RECLAMO_VERIFICADO1, usuarioResponsable, idTipoAprobacion);

movSolicitudVerificacion1.setSirectSolicitudReclamo(reclamo);

listaMovimiento.add(movSolicitudVerificacion1);

}

No señores, no estoy bromeando, este código es de un programa real que utiliza actualmente una empresa (obviamente no mencionaré el nombre). Extraje una sección del código para mostrarles las cosas que uno puede encontrar en cada código y quedar como la imagen de Batman del comienzo. Ojo, la parte que dice “//nothing” no significa que yo lo haya eliminado y haya puesto eso, NO, así es tal como está en el código fuente. La pregunta que me hago es……si no vas a hacer nada, para qué le pones un “if”?. Para esto, a mi parecer, existen 2 soluciones: Primera, Colocar “if” solo para situaciones que ameritan una acción, que ameritan que al procesar no hagas una condición que no te lleve a nada, en caso te digan que coloques un “if” para que sea más entendible el código, colócale un comentario y listo, es mucho mejor tener un comentario que ocupe algo de líneas, en lugar de hacer trabajar al procesador en una condición que no hace NADA. Segunda solución: despide a ese programador XD.

Obviamente la 2da solución es una broma, pues en momentos que uno se encuentra con cosas así, sean de uno o de otros, no vale la pena enojarse, sino reírse y reírse hasta más no decir. Porque recuerden algo, todos fuimos alguna vez terneras.

Espero haya sido de su gusto este artículo.

Y me despido haciéndoles la pregunta: “¿Cuál fue el peor código que has visto en tu vida?”

Y esto va solo para la gente que lee mi blog, pronto haré un tipo de concurso donde sortearé un libro original de ASP.NET. Así que escriban con todo para saber quiénes realmente desean participar de este concurso. En estos días decidiré si será solo para la gente relacionada con el tema o no. Gracias por todo a todos =)

//*******************************
//English
//*******************************

Many of us have worked for days, months, even years looking a lot of source code, fixing source code, or maybe finding bugs. So, We have faced a lot of ugly source code that we finished with this face:

That’s the reason of i was thinking this days about my uglys source code, the ugly source code that I fixed and some bug that I found.

So, let’s enumérate, from less to more, one by one with a caracteristic name:

1. ¿What’s the meaning of that number?

public class DoesntHaveConstant {

public double getIGV(double precio){

return precio*0.19;

}

}

Many times we write this kind of code, maybe thinking “I will change it after that…”, but that never happen. “Why is this bad?” That is the question that someone asked me, and there is no just one reason: “Just a little bit of people will understand the meaning of 0.19”, “How many times did you repeat that 0.19 in the application? If in the future that value needs to be changed, how can I do that? Replace?”, and the most important “If that value needs to be changed in the future for someone who don’t know about how to program?”. The worst of this is that someday I heard someone answering the last question “I can teach him/her how to program”. PLOOOOOOP XD. The solution is simple, you need to use Constants or maybe properties to modify the value manually.

2. ¿What is this?¿x1, x2 y x3?¿Versions?

public class WhatIsThis {

private int x1;

private int x2;

private int x3;

public int doSomething(){

return x1*x2 + x3;

}

}

The reason is obvious, but if you didn’t see it, it’s: “Does anyone know what is the functionality of doSomething?” Ok we’ve seen this method in books, but you shouldn’t use that kind of names to a class, a variable, method or other things. The solution is very easy, put a descriptive name to your classes, variables, methods and others.

3. ¿Is that the description of a Person?

public class PersonDAO {

public void save(String pNombre, String sNombre,

String apePaterno, String apeMaterno, String telefono, int edad, String email, Date fechaNacimiento, String direccion){

//Here is the code

}

}

This is something complex to see, because many of us a lot of times work of the way “if this works, so it’s ok”, without to take the abstraction of POO. So, if the DAO will save a Person object, and you know that a Person object contains those parameters, why do we need to pass a lot of parameters rather than just one Person object.

4. No comments…..

if (accion.equalsIgnoreCase(CReclamo.RECLAMO_ACCION_GRABAR)){

// nothing

}else if (accion.equalsIgnoreCase(CReclamo.RECLAMO_ACCION_ENVIAR)){

SirectMovimientoSolReclamo movSolicitudVerificacion1 = generaMovimiento(CReclamo.ESTADO_RECLAMO_VERIFICADO1, usuarioResponsable, idTipoAprobacion);

movSolicitudVerificacion1.setSirectSolicitudReclamo(reclamo);

listaMovimiento.add(movSolicitudVerificacion1);

}

No, I’m not kidding, this is a real source code of a real program that works in an enterprise. I copied this section of the code because I wanted to show the kind of code that can leave you like the batman image xD. Hey, if you don’t understand, that part of the code that says “//nothing”, that doesn’t mean that I have deleted some code and put that, NO, that’s the original way of the source code. So, why do you need to put a conditional that does …..NOTHING???. There are two solutions: First, just put a conditional when you really need it. Second, fired him/her XD

Obviously i was kidding in the second solution, because in that kind of moments you just need a laugh, come on don’t be angry for that, just a lot of laughs. Because remember, we have failed sometime.

I hope you liked this article.

And my final question for you is: “What’s the worst code that you have ever seen?”

cya :3

Shinji