Clean Code en Harbour
## Introduction
Principios de ingeniería de software, del libro de Robert C. Martin [ Código limpio ](https://www.amazon.com/Clean-Code-Handbook-Software-Craftsmanship/dp/0132350882), adaptado para Harbout. 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](https://github.com/jupeter/clean-code-php)
Basado en la traducción de [Francisco Bórquez](https://github.com/fikoborquez/clean-code-php#usar-encapsulaci%C3%B3n-de-objetos)
## Variables
### Use nombres de variables significativos y fáciles de pronunciar.
**Mal:**
```xbase
$cdt = DateTime();
```
**Bien:**
```xbase
$currentDateTime = DateTime();
```
### Usa el mismo vocabulario para el mismo tipo de variable
**Mal:**
```xbase
getUserInfo()
getUserData()
getUserRecord()
getUserProfile()
```
**Bien:**
```xbase
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:**
```xbase
// Que significa realmente el dos
fileHandle = fOpen( “fopen.prg”, 2)
```
**Bien:**
```xbase
fileHandle = fOpen( “fopen.prg”, FO_READWRITE)
```
### Use los nombres para las búsqueda (parte 2)
**Mal:**
```xbase
// What the heck is 4 for?
if (user:access == 4)
// do …
end if
```
**Bien:**
```xbase
#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:**
```xbase
programsDirectory := Directory( “*.prg” )
aeval( programsDirectory, {|x| sendFile( x[1], x[2] ) } )
```
**Bien:**
```xbase
programsDirectory := Directory( “*.prg” )
aeval( programsDirectory, {|programFile| sendFile( programFile[F_NAME], programFile[F_SIZE] ) } )
```
### Evite anidar demasiado y regrese pronto (parte 1)
Demasiadas declaraciones ‘if’ no pueden hacer que su código sea difícil de seguir.
Hay autores que llegana a decir que si tu funcion tiene, muchos else (incluso solo uno), debe ser reescrita.
Explícito es mejor que implícito.
**Mal:**
```xbase
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:**
```xbase
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:**
```xbase
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:**
```xbase
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:**
```xbase
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:**
```xbase
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 la variable.
**Mal:**
```xbase
CLASS Car
DATA carMake
DATA carModel
DATA carColor
//…
ENDCLASS
```
**Bien:**
```xbase
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 combinatoria
donde 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 dos
argumentos entonces su función está tratando de hacer demasiado. En los casos en que no lo es, la mayoría
del tiempo, un objeto de nivel superior bastará como argumento.
**Mal:**
```xbase
function createMenu(title, body, buttonText, cancellable)
// …
return
```
**Bien:**
```xbase
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 aislar
una función para una sola acción, se pueden refactorizar fácilmente y su código leerá mucho
limpio. Si no quita nada más de esta guía que no sea esto, estará adelante
de muchos desarrolladores.
**Mal:**
```xbase
function emailClients(clients)
foreach client in clients
clientRecord = clientModel():find(client)
if clientRecord:isActive()
email(client)
end if
next
return
```
**Bien:**
```xbase
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:**
```xbase
CLASS Email
//…
METHOD handle()
mail( ::to, ::subject, ::body )
RETURN nil
ENDCLASS
message := Email(…):New()
// ¿Que es esto?
message:handle()
```
**Bien:**
```xbase
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:**
```xbase
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.
```xbase
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:**
```xbase
function createFile(name, temp)
if (temp) {
fcreate( ‘./temp/’ + name)
else
fcreate( name )
end if
return
```
**Bien:**
```xbase
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:**
```xbase
// 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:**
```xbase
function splitIntoFirstAndLastName( name )
return hb_atokens( name, ‘ ‘ )
name := ‘Manuel Calero’
newName := splitIntoFirstAndLastName(name)
? ( hb_valtoexp( name ) ) // ‘Manuel Calero’
? ( hb_valtoexp( newName ) ) // {‘Manuel’, ‘Calero’}
```
### Don’t write to global functions
Llenar de funciones globales es una mala práctica en muchos lenguajes porque puedes chocar con otra librería.
**Mal:**
```xbase
function config()
return { ‘foo’ => ‘bar’ }
```
**Bien:**
```xbase
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`
```xbase
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](https://en.wikipedia.org/wiki/Singleton_pattern). Citando a Brian Button:
1. 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].(https://en.wikipedia.org/wiki/Code_smell).
2. Violan el [principio de la responsabilidad única]: en virtud del hecho de que ellos controlan su propia creación y ciclo de vida.
3. 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](http://misko.hevery.com/about/) ha realizado unas reflexiones interesantes sobre el [origen del problema](http://misko.hevery.com/2008/08/25/root-cause-of-singletons/).
**Mal:**
```xbase
CLASS DBConnection
CLASSDATA instance
METHOD New( dsn )
// …
RETURN ( self )
METHOD getInstance() INLINE ( if( empty( ::oInstance ), ::oInstance := ::New(), ), ::oInstance )
// …
ENDCLASS
singleton := DBConnection():getInstance()
```
**Bien:**
```xbase
CLASS DBConnection
METHOD New( dsn )
// …
RETURN ( self )
ENDCLASS
```
Crea una instancia de la clase `DBConnection` y configúrala con DSN.
```xbase
connection := DBConnection():New( dsn )
```
Y ahora debes usar la instancia de `DBConnection` en tu aplicación.
### Encapsular condicionales
**Mal:**
```xbase
if ( article:state == ‘published’ )
// …
end if
```
**Bien:**
```xbase
if ( article:isPublished() )
// …
end if
```
### Evitar condicionales negativos
**Mal:**
```xbase
function isNodeNotPresent( node )
// …
if ( !isDOMNodeNotPresent( node ) )
// …
```
**Bien:**
```xbase
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:**
```xbase
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:**
```xbase
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:**
```xbase
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:**
```xbase
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 del! Siempre estará a salvo en tu versión histórica si aún lo necesitas.
**Mal:**
```xbase
CLASS oldRequestModule( url )
// …
ENDCLASS
CLASS newRequestModule( url )
// …
ENDCLASS
request := newRequestModule():New( requestUrl )
inventoryTracker(‘apples’, request, ‘www.inventory-awesome.io')
```
**Bien:**
```xbase
CLASS RequestModule( url )
// …
ENDCLASS
request := RequestModule():New( requestUrl )
inventoryTracker(‘apples’, request, ‘www.inventory-awesome.io')
```
