Esta entrada se publicó originalmente en Harbour Magazine, mi publicación sobre el lenguaje de programación Harbour.
Este es un artículo de Manuel Calero Solís para Harbour Magazine.
Introducción
Principios de ingeniería de software, del libro de Robert C. Martin Código limpio , adaptado para Harbour. Esta no es una guía de estilo, es una guía para producir software legible, reutilizable y refactorizable. No todos los principios de este documento deben seguirse estrictamente, y aún menos serán universalmente acordados. Estas son pautas y nada más, pero están codificadas durante muchos años de experiencia colectiva por los autores de Clean Code.
Inspirado en clean-code-php
Basado en la traducción de Francisco Bórquez
Variables
Use nombres de variables significativos y fáciles de pronunciar.
Mal:
$cdt = DateTime();
Bien:
$currentDateTime = DateTime();
Usa el mismo vocabulario para el mismo tipo de variable
Mal:
getUserInfo()
getUserData()
getUserRecord()
getUserProfile()
Bien:
getUser()
Use los nombres para las búsqueda (parte 1)
Es importante que el código que escribimos sea legible y pueda buscarse. Al no nombrar variables que terminan siendo significativas para comprender nuestro programa, molestamos a los futuros lectores de nuestro código. Hagamos que los nombres sean faciles de buscar y entender.
Mal:
// Que significa realmente el dos
fileHandle = fOpen( "fopen.prg", 2)
Bien:
fileHandle = fOpen( "fopen.prg", FO_READWRITE)
Use los nombres para las búsqueda (parte 2)
Mal:
// What the heck is 4 for?
if (user:access == 4)
// do ...
end if
Bien:
#define ACCESS_READ = 1
#define ACCESS_CREATE = 2
#define ACCESS_UPDATE = 4
#define ACCESS_DELETE = 8
?
if (user:access == ACCESS_UPDATE)
// do ...
end if
Usar variables explicativas
Mal:
programsDirectory := Directory( "*.prg" )
?
aeval( programsDirectory, {|x| sendFile( x[1], x[2] ) } )
Bien:
programsDirectory := Directory( "*.prg" )
?
aeval( programsDirectory, {|programFile| sendFile( programFile[F_NAME], programFile[F_SIZE] ) } )
Evite anidar demasiado y regrese pronto (parte 1)
Demasiadas sentencias ‘if’ pueden hacer que su código sea difícil de seguir. Hay autores que llegan a decir que si tu funcion tiene muchos else (incluso solo uno), debe ser reescrita.
Explícito es mejor que implícito.
Mal:
function isShopOpen(day)
?
if !empty(day)
if hb_isstring(day)
day = lower(day)
if (day == 'friday')
return .t.
elseif (day == 'saturday')
return .t.
elseif (day == 'sunday')
return .t.
else
return .f.
end if
else
return .f.
end if
else
return .f.
end if
?
return .f.
Bien:
function isShopOpen(day)
local openingDays := {'friday', 'saturday', 'sunday'}
?
if empty(day)
return .f.
end if
?
return ( ascan( openingDays, lower(day) ) == 0 )
Evite anidar demasiado y regrese pronto (parte 2)
Mal:
function fibonacci( n )
?
if (n < 50)
if (n != 0)
if (n != 1)
return fibonacci(n - 1) + fibonacci(n - 2)
else
return 1
end if
else
return 0
end if
else
return 'Not supported'
end if
?
return n
Bien:
function fibonacci( n )
?
if (n == 0 .or. n == 1)
return n
end if
?
if (n > 50)
return 'Not supported'
end if
?
return fibonacci(n - 1) + fibonacci(n - 2)
?
return n
?
Evitar el mapeo mental
No obligue al lector de su código a traducir lo que significa la variable. Explícito es mejor que implícito.
Mal:
local li
local l := {'Austin', 'New York', 'San Francisco'}
?
for (i := 1 to len( l ))
li = l[i]
doStuff()
doSomeOtherStuff()
// ...
// ...
// ...
// Espera, ¿qué es `li`?
dispatch(li)
next
Bien:
?
local locations := {'Austin', 'New York', 'San Francisco'}
?
foreach location in location
doStuff()
doSomeOtherStuff()
// ...
// ...
// ...
dispatch(location)
next
No agregue contexto innecesario
Si su nombre de clase / objeto le dice algo, no lo repita en su nombre de variable.
Mal:
CLASS Car
?
DATA carMake
DATA carModel
DATA carColor
?
//...
ENDCLASS
Bien:
CLASS Car
?
DATA make
DATA model
DATA color
?
//...
ENDCLASS
Funciones
Parametros de función (lo ideal es 2 o menos)
Limitar la cantidad de parámetros de función es increíblemente importante porque hace que probarla sea más fácil. Tener más de tres conduce a una explosión combinatoriadonde tienes que probar toneladas de casos diferentes con cada argumento por separado.
Cero argumentos es el caso ideal. Uno o dos argumentos están bien, y se deben evitar tres.Algo más que eso debe consolidarse. Por lo general, si tiene más de dosargumentos entonces su función está tratando de hacer demasiado. En los casos en que no lo es, la mayoríadel tiempo, un objeto de nivel superior bastará como argumento.
Mal:
function createMenu(title, body, buttonText, cancellable)
?
// ...
return
Bien:
class MenuConfig
?
DATA title
DATA body
DATA buttonText
DATA cancellable INIT .f.
?
end class
?
config := MenuConfig():New()
config:title := 'Foo'
config:body := 'Bar'
config:buttonText := 'Baz'
config:cancellable := .t.
?
function createMenu( config )
// ...
return
Las funciones deberían hacer una cosa
Esta es, con mucho, la regla más importante en ingeniería de software. Cuando las funciones hacen másnde una cosa, son más difíciles de componer, probar y razonar. Cuando puedes aislaruna función para una sola acción, se pueden refactorizar fácilmente y su código leerá mucholimpio. Si no quita nada más de esta guía que no sea esto, estará adelantede muchos desarrolladores.
Mal:
function emailClients(clients)
?
foreach client in clients
clientRecord = clientModel():find(client)
if clientRecord:isActive()
email(client)
end if
next
return
Bien:
function emailClients(clients)
?
local activeClients := activeClients( clients )
aeval( activeClients, {|client| email( client ) } )
?
return nil
?
function activeClients( clients )
?
local activeClients := {}
aeval( clients, {|client| if( isClientActive( client ), aadd( activeClients, client ), ) } )
?
return ( activeClients )
?
function isClientActive( client )
?
if clientModel():find( client )
return ( clientRecord:isActive() )
end if
?
return ( .f. )
Los nombres de las funciones deberían decir lo que hacen
Mal:
CLASS Email
?
//...
?
METHOD handle()
mail( ::to, ::subject, ::body )
RETURN nil
?
ENDCLASS
?
message := Email(...):New()
// ¿Que es esto?
message:handle()
Bien:
CLASS Email
?
//...
?
METHOD send()
mail( ::to, ::subject, ::body)
?
RETURN nil
?
ENDCLASS
?
message := Email(...):New()
// Limpio y obvio
message:send()
Las funciones deben tener sólo un nivel de abstracción
Cuando tienes más de un nivel de abstracción usualmente es porque tu función está haciendo demasiado. Separarlas en funciones lleva a la reutilización.
Mal:
function ExecutorStaments( tokenizedStatement )
?
local statement
local statements := hb_atokens( tokenizedStatement, "," )
?
for each statement in statements
if SQLConexion():Parse( statement )
SQLConexion():Execute( statement )
// ...
end if
next
?
return
Bien:
Lo mejor es crear una clase que tenga dentro dependencias a otras clases.
CLASS Tokenizer
?
METHOD new()
?
METHOD tokenizer() inline ( hb_atokens( tokens, "," ) )
?
end CLASS
?
CLASS Conexion
?
data oConexion
?
METHOD new()
?
METHOD Parse( statement ) inline ( ::oConexion:Parse( statement ) )
?
METHOD Execute( statement ) inline ( ::oConexion:Execute( statement ) )
?
ENDCLASS
?
CLASS ExecutorStaments
?
DATA tokenizer
DATA conexion
?
METHOD new()
::tokenizer := Tokenizer():New()
::conexion := Conexion():New()
RETURN ( self )
?
METHOD Execute( statement )
local statement
local statements := ::tokenizer:tokenizer( statement )
?
for each statement in statements
if ::conexion:Parse( statement )
::conexion:Execute( statement )
end if
next
?
RETURN ( self )
?
ENDCLASS
No usar logicos como parámetros de funciones
Los valores logicos le dicen al usuario que la función hace más de una cosa. Las funciones deben hacer sólo una. Divide tus funciones si ellas siguen diferentes caminos basados en un valor booleano.
Mal:
function createFile(name, temp)
?
if (temp) {
fcreate( './temp/' + name)
else
fcreate( name )
end if
?
return
Bien:
function createFile(name, temp)
?
fcreate( './temp/' + name)
return
?
function createTempFile(name)
?
fcreate( './temp/' + name)
return
Evitar efectos secundarios
Una función produce un efecto secundario si hace algo más que tomar un valor y devolver otro. Un efecto secundario puede ser escribir en un archivo, modificar alguna variable global, o accidentalmente darle todo tu dinero a un extraño.
Ahora, ocasionalmente necesitaras los efectos secundarios en un programa. Como los ejemplos anteriores, necesitarás escribir en un archivo. Lo que quieres hacer en esos casos es centralizar donde realizarlos. No tengas muchas funciones y clases que escriban un archivo en particular. Ten un servicio que lo haga. Uno y sólo uno.
El punto principal es evitar trampas comunes como compartir estados entre objetos sin alguna estructura, usar tipos de datos mutables que puedan ser escritos por cualquiera, y no centralizar donde el efecto paralelo ocurre. Si puedes hacerlo, serás más feliz que la mayoría de los demás programadores.
Mal:
// Variable global referenciada por la siguiente función.
// Si tenemos otra función que use el mismo nombre, ahora será un arreglo y podría romperla.
?
memvar name
?
function splitIntoFirstAndLastName()
?
name = hb_atokens( name, ' ' )
?
return nil
?
name = 'Manuel Calero'
?
splitIntoFirstAndLastName()
?
? ( hb_valtoexp( name ) ) // {'Manuel', 'Calero'}
Bien:
function splitIntoFirstAndLastName( name )
?
return hb_atokens( name, ' ' )
?
name := 'Manuel Calero'
newName := splitIntoFirstAndLastName(name)
?
? ( hb_valtoexp( name ) ) // 'Manuel Calero'
? ( hb_valtoexp( newName ) ) // {'Manuel', 'Calero'}
No escribas funciones globales
Llenar de funciones globales es una mala práctica en muchos lenguajes porque puedes chocar con otra librería.
Mal:
function config()
?
return { 'foo' => 'bar' }
Bien:
CLASS Configuration
?
DATA configuration INIT {}
?
METHOD New( configuration )
::configuration := configuration
?
RETURN ( self )
?
METHOD get( key )
if hhaskey( ::configuration, key )
RETURN ( hget( ::configuration, key ) )
end if
?
RETURN ( nil )
?
ENDCLASS
Crea la variable configuration con una instancia de la clase Configuration
configuration := Configuration():new( {'foo' => 'bar'} )
Y ahora puedes usar una instancia de la clase Configuration en tu aplicación.
No usar el patrón Singleton
Singleton es un anti-patrón. Citando a Brian Button:
- Son usados generalmente como una instancia global, ¿Por qué eso es malo? Porque escondes las dependencias de tu aplicación en tu código, en lugar de exponerlas. Hacer algo global para evitar pasarlo es una hediondez de código.
- Violan el principio de la responsabilidad única]: en virtud del hecho de que ellos controlan su propia creación y ciclo de vida.
- Inherentemente causan que el código esté estrechamente acoplado. Esto hace que muchas veces sean difíciles de probar, aunque en Harbour no tenemos bancos de pruebas.
- Llevan su estado al ciclo de vida de la aplicación. Otro golpe a las pruebas porque puedes terminar con una situación donde las pruebas necesitan ser ordenadas lo cual es un gran no para las pruebas unitarias. ¿Por qué? Porque cada prueba unitaria debe hacerse independiente de la otra. Misko Hevery ha realizado unas reflexiones interesantes sobre el origen del problema.
Mal:
CLASS DBConnection
?
CLASSDATA instance
?
METHOD New( dsn )
// ...
RETURN ( self )
?
METHOD getInstance() INLINE ( if( empty( ::oInstance ), ::oInstance := ::New(), ), ::oInstance )
?
// ...
ENDCLASS
?
singleton := DBConnection():getInstance()
Bien:
CLASS DBConnection
?
METHOD New( dsn )
// ...
RETURN ( self )
?
ENDCLASS
Crea una instancia de la clase DBConnection y configúrala con DSN.
connection := DBConnection():New( dsn )
Y ahora debes usar la instancia de DBConnection en tu aplicación.
Encapsular condicionales
Mal:
if ( article:state == 'published' )
// ...
end if
Bien:
if ( article:isPublished() )
// ...
end if
Evitar condicionales negativos
Mal:
function isNodeNotPresent( node )
?
// ...
?
?
if ( !isDOMNodeNotPresent( node ) )
?
// ...
?
Bien:
function isNodePresent( node )
?
// ...
?
if (isNodePresent( node ) )
?
// ...
?
Evitar condicionales
Esta parece ser una tarea imposible. Al escuchar esto por primera vez, la mayoría de la gente dice, “¿cómo se supone que haré algo sin una declaración if?» La respuesta es que la mayoría de las veces puedes usar polimorfismo para lograr el mismo resultado.
La segunda pregunta usualmente es, “bien, eso es genial, ¿pero cómo puedo hacerlo?” La respuesta es un concepto de código limpio que ya hemos aprendido: una función debe hacer sólo una cosa. Cuando tienes clases y funciones que usan declaraciones if, estás diciéndole al usuario que tu función hace más de una cosa. Recuerda, hacer sólo una cosa.
Mal:
CLASS Airplane
?
// ...
?
METHOD getCruisingAltitude()
SWITCH ::type
CASE '777':
RETURN ::getMaxAltitude() - ::getPassengerCount()
CASE 'Air Force One':
RETURN ::getMaxAltitude()
CASE 'Cessna':
RETURN ::getMaxAltitude() - ::getFuelExpenditure()
END
?
RETURN ( 0 )
?
ENDCLASS
Bien:
CLASS Airplane
?
// ...
?
METHOD getCruisingAltitude() VIRTUAL
?
ENDCLASS
?
CLASS Boeing777 FROM Airplane
?
// ...
?
METHOD getCruisingAltitude()
RETURN ::getMaxAltitude() - ::getPassengerCount()
ENDCLASS
?
CLASS AirForceOne implements Airplane
// ...
?
METHOD getCruisingAltitude()
RETURN ::getMaxAltitude()
ENDCLASS
?
CLASS Cessna implements Airplane
?
// ...
?
METHOD getCruisingAltitude()
RETURN ::getMaxAltitude() - ::getFuelExpenditure()
ENDCLASS
Evitar revisión de tipo
Harbour es un lenguaje no tipado, lo que quiere decir que tus funciones pueden tener cualquier tipo de argumento. Algunas veces habrás sentido esta libertad y te habrás tentado a hacer revisión de tipo en tus funciones. Hay muchas maneras de evitar tener que hacerlo.
Mal:
METHOD travelToTexas( vehicle )
?
if vehicle:IsDerivedFrom( 'Bicycle' )
vehicle:pedalTo( Location():New( 'texas' ) )
elseif vehicle:IsDerivedFrom( 'Car' )
vehicle:driveTo( Location():New( 'texas' ) )
end if
RETURN ( nil )
Bien:
METHOD travelToTexas( vehicle )
?
vehicle:travelTo( Location():New( 'texas' ) )
RETURN ( nil )
Quitar código muerto
El código muerto es tan malo como el código duplicado. No hay motivos para mantenerlo en tu código fuente. Si no está siendo llamado, ¡deshazte de él! Siempre estará a salvo en tu versión histórica si aún lo necesitas.
Mal:
CLASS oldRequestModule( url )
?
// ...
?
ENDCLASS
?
CLASS newRequestModule( url )
?
// ...
?
ENDCLASS
?
request := newRequestModule():New( requestUrl )
?
inventoryTracker('apples', request, 'www.inventory-awesome.io')
?
Bien:
CLASS RequestModule( url )
?
// ...
?
ENDCLASS
?
request := RequestModule():New( requestUrl )
?
inventoryTracker('apples', request, 'www.inventory-awesome.io')