8 de septiembre de 2007

Como se degrada el código

A veces nos encontramos con mal código difícil de explicar. ¿Por que alguien comete errores tan tontos?. Menos mal que con la proliferación de sistemas de control de versión podemos investigar en el historial del código y encontrar el porqué. Como en el ejemplo de hoy.

Partimos de una función bien construida y perfectamente normal. Simplemente calcula un nombre de fichero a partir de un identificador numérico y carga el contenido del fichero. Hasta aquí todo normal.
void Load( int id ){
char filename[ MAX_FILENAME_SIZE ];
CalculaNombre( filename, id );

LoadAsset( filename, false );
}

Podemos ver que la función que carga el fichero tiene un parámetro booleano. En la primera modificación, se tiene que mirar cierta condición usando el identificador como parámetro. Si se cumple la condición se carga el fichero con el parámetro booleano con el valor true, si no se cumple la condición pues usamos el valor false.

void Load( int id ){
char filename[ MAX_FILENAME_SIZE ];
CalculaNombre( filename, id );

if( DeterminadaCondicion( id ) ){
LoadAsset( filename, true );
}else{
LoadAsset( filename, false );
}
}

Vamos con la última modificación registrada en el historial. Se decide que si no se cumple la condición anteriormente mencionada, pues el valor pasado a la función que cargará los datos sera true.

void Load( int id ){
char filename[ MAX_FILENAME_SIZE ];
CalculaNombre( filename, id );

if( DeterminadaCondicion( id ) ){
LoadAsset( filename, true );
}else{
LoadAsset( filename, true );
}
}

Como podéis ver, hemos acabado con una función que usa una sentencia condicional innecesaria, código repetido, una llamada a función redundante. Un mal código, resultado de un par de cambios descuidados.

Mi solución

La sentencia condicional ya era innecesaria en la primera modificación. Se puede usar el valor devuelto como parámetro de la función de carga. De esta manera se obtiene un código más claro.

void Load( int id ){
char filename[ MAX_FILENAME_SIZE ];
CalculaNombre( filename, id );

LoadAsset( filename, DeterminadaCondicion( id ) );
}

Con el paso anterior es más difícil cometer el error del código duplicado. Simplemente nos damos cuenta que el parámetro de la función de carga va a tomar siempre el valor true. Y así lo escribimos, llegando al código final, mucho más correcto que el original.

void Function( int id ){
char filename[ MAX_FILENAME_SIZE ];
CalculaNombre( filename, id );

LoadAsset( filename, true );
}

Conclusión. Dos lecciones podemos aprender hoy, cuando se hacen cambios en el código hay que pensar un poco en lo que se hace, y lo otro que podemos aprender, lo útiles que son los sistemas de control de versiones.