Pásale la fregona a ese código.

Disclaimer. Este post no está basado en Clean Code. Está basadísimo. Clean Code es un libro que considero indispensable y con leer unas cuantas páginas cambiará tu manera de ver y hacer código. Link here.

El código aportado como prueba es un generador de mapas procedurales creado por mí y puede ser consultado aquí en su forma refactorizada:


private void generarRamas()
{
int aux;
int iteracionY;
int iteracionX;
int iRama = 0;
// Genero ramas aleatoriamente basado en la cantidad de ramas y su longitud. Es todo sencillo y aleatorio.
for (int i= 0; i<cantidadRama; i++)
{
  aux = Random.Range(0, AlmacenLines.Count);
  iRama = 0;
  if (AlmacenLines[aux].getIfHorizontal())
  {
    iteracionY = (int) AlmacenLines[aux].getStartPoint().y;
    iteracionX = Random.Range(0, worldSizeMapX);

    if (Random.Range(0,100)<= 50)
    {
      while (iRama < longitudRama && iteracionY < worldSizeMapY-1 && iteracionX < worldSizeMapX-1 )
      {
        iRama++;

  ....

Si quereis seguir viendo el resto de está maravilla podeis hacerlo aquí

Empecemos fuerte. ¿Qué define a un buen programador?

Se nos pueden ocurrir muchas cosas:

  • Capacidad de organización.
  • Inventiva.
  • Rápidez a la hora de solucionar problemas.
  • Capacidad de reaccionar a imprevistos.
  • Capacidad debuggeadora.
  • Conocimientos sobre diseño. (Patrones de.)

Y así podemos estar un buen rato. Pasando por conocimiento de tecnologías, soltura con X o Y motor en el concreto caso de los videojuegos, o habilidad lógica general. Pero hay un paso importante y que a veces, se nos olvida. La limpieza en el código.

¿Por qué se nos olvida? En equipos pequeños, cuando somos uno o dos programadores se nos puede empezar a ir la cabeza y hacer menos prioritario un aspecto fundamental como es la mantenibilidad de nuestro proyecto. Y bueno, cuando volvemos a este proyecto para hacer un parche 1 mes después; queramos o no, se nos habrá olvidado buena parte de las ‘ñapas’ que cometimos y el porqué las hicimos.

Ahora bien, ¿qué podemos hacer para mantener el código limpio? Antes enseñé un trozo de código brutal. Era un código que mide muy bien la prisa que tenía el programador (yo) en ese momento y pasé un par de horas refactorizándolo. Vamos a ver porqué ese código está sucio y como lo mejoré.

 

private void generarRamas() 

Empezamos mal. Esto es más una convención que otra cosa, pero aquí el problema es que tenía mezclado en toda la clase inglés y español. Aparte, lo normal en C# sería EmpezarLasFunciones así en vez de hacerloAsí.

int aux;
int iteracionY;
int iteracionX;
int iRama = 0;

No se que coño es una iRama. El nombre aquí es algo traicionero, porque haciendo algo de esfuerzo mental puedo suponer que i viene de la convención de la variable que lleva la cuenta de algo, pero vamos, a simple vista ni idea.

 // Genero ramas aleatoriamente basado en la cantidad de ramas y su longitud. Es todo sencillo y aleatorio.</pre>
Hay otro comentario más adelante, pero que valga para los dos. Si tienes que poner un comentario es que tu código no se explica solo. Esto es cierto una inmensa mayoría de las veces, y hay gente que llega a extremos tan desquiciados como Pepe.
<pre>
 //Obtengo la variable fuerza
public int GetStrength(){
  return this.strength;
}

No seas como Pepe.

Si pones un comentario justo arriba del nombre de una función que dice exactamente lo mismo que tu comentario... Ahorrate el comentario.

for (int i= 0; i<cantidadRama; i++) {
  aux = Random.Range(0, AlmacenLines.Count);
  iRama = 0;
  if (AlmacenLines[aux].getIfHorizontal()) {
    iteracionY = (int) AlmacenLines[aux].getStartPoint().y;
    iteracionX = Random.Range(0, worldSizeMapX);
    if (Random.Range(0,100)<= 50) {
      while (iRama < longitudRama && iteracionY < worldSizeMapY-1 && iteracionX < worldSizeMapX-1 ) {
        iRama++;

1, 2, 3 y ¡4! niveles de indentación. Como norma general: si tu método va a tener más de un nivel de indentación, mete los subsecuentes niveles en métodos porque probablemente se pueda leer mejor ese código.

Estos son bastantes problemas para una muestra tan pequeña de código. ¡Y ni siquiera es todo el código de ese método! Eso lo podéis revisar arriba, con problemas más graves como duplicación de código o que todo el método ocupa unas 100 líneas.

Y esto en la versión refactorizada acabó de otra manera bastante distinta:

private void GenerateBranches() {
  for (int i = 0; i < properties.BranchQuantity; i++) {
    int aux = Random.Range(0, lines.Count);
    GenerateBranch(lines[aux]);
  }
}

Aquí podemos, de un solo vistazo, ver lo que hace este método. Genera tantas ramas como le diga la variable "properties.BranchQuantity" esto sustituye a "cantidadRama" que era anteriormente la variable de clase que controlaba esto. Vamos a echarle un vistazo a las declaraciones de las variables de clase de una y otra versión:

private int[,] mapa;

public GameObject colisionAGenerar;
GameObject father;
public int worldSizeMapX;
public int worldSizeMapY;
public int casillasLibresCentro;

public int cantidadRama;
public int longitudRama;

public int lineasGeneradas;
private List<Line> AlmacenLines;
private float nodeSize;
public bool drawGizmos;

public GameObject suelo1;
public GameObject suelo2;

Esto tiene algún que otro problema, las variables públicas son malas, porque las variables de la clase no deberían ser accesibles al resto, salvo honrosas excepciones. (Poner todo con gets y sets... es lo mismo. Te puede ayudar a la hora de debuggear que si buscas las referencias a SetX solo veas las hechas desde otras clases, pero aparte de eso estas deshonrando la encapsulación que debería tener un objeto either way)

En Unity para arreglarlo tenemos la directiva [SerializeField] que nos permite que la variable aparezca en el editor sin tener que exponerla a otras clases.

Seguimos viendo el mismo problema del 'spanglish' a parte de variables que se han quedado ahí de cuando debuggeabamos (como drawGizmos) pero el principal problema es que hay muchas variables. Y hay muchas porque no estamos agrupándolas por relación.

¿worldSizeMapX y worldSizeMapY son suficientemente independientes como para merecerse el estar separadas? Aparte, ¿pueden ser negativas? Porque esta variable la usaremos en el editor y nos dejará ponerle valores negativos. Podemos usar la directiva Range para darle un valor de (0 a 2000) pero ya estás limitando por el máximo.

CantidadRama, longitudRama, lineasGeneradas y casillasLibresCentro adolecen de lo mismo, no pueden ser negativos pero son int, se relacionan entre ellas (son propiedades relacionadas directamente con la generación del mapa) y a parte de esto no están juntas verticalmente, es decir, tienen saltos de línea entre ellas. Estos espacios relacionan.

Unity tip: A mi me gusta agrupar las variables que se van a enseñar en el editor por un lado y las privadas por otro.

Mejorando en dichas áreas, el código puede quedar así:

private SquareValue[,] map;
private GameObject father;
private List<Line>lines;
private float nodeSize;

[SerializeField]
private PositivePoint worldMapSize;

[SerializeField]
private MapSpecifications properties;

[Header("Floor, can be null")]
[SerializeField]
private GameObject floorPrefab;

[SerializeField]
private GameObject prefabColision;

Aunque por cantidad de líneas parece que no hemos hecho gran cosa, si nos fijamos podremos ver que la mayoría de las líneas extra son por atributos ([Atributo]) y ahora nuestro diseño tiene más sentido al juntar muchas variables  similares en clases encapsuladoras. Por ejemplo las variables que controlan la creación del mapa están encapsuladas en MapSpecifications  en vez de estar libres por ahí, lo cual no tendría mucho sentido porque solas no cumplen ninguna función.

El tamaño del mapa está contenido en una struct que toma 2 uint para especificar que tienen que ser variables enteras y positivas, lo cual antes no sucedía, pudiendo llevar a errores.

Volviendo al método de GenerarRamas podemos ver como queda el monolítico diseño anterior comparado con como está dividido ahora:

f43aaad26a3c8369c41542721cea6ceefafc87d39af4c70ed98091800a204921

De la imagen situada a la derecha no podemos entender prácticamente nada. No cabe en la pantalla, y leyendo solo el nombre de la función sabemos lo que hace pero no cómo  lo hace.  Leyendo solo los nombres de como queda después del refactor entendemos lo que hace y podemos hacernos una buena idea de cómo lo hace.

Sabiendo estas cosas podemos ver y mejorar código de otras personas, al hacerlo más legible y mantenible. Para coger otro ejemplo menos largo, podemos ver el código de ejemplo de Unity en el tutorial de como implementar IAP.

 private static IStoreController m_StoreController; // The Unity Purchasing system.
 private static IExtensionProvider m_StoreExtensionProvider; // The store-specific Purchasing subsystems.

 public static string kProductIDConsumable = "consumable";
 public static string kProductIDNonConsumable = "nonconsumable";
 public static string kProductIDSubscription = "subscription";

 private static string kProductNameAppleSubscription = "com.unity3d.subscription.new";
 private static string kProductNameGooglePlaySubscription = "com.unity3d.subscription.original";

Primera línea, IStoreController; hasta aquí bien, prefijo chachi y StoreController. Luego me pone un comentario que dice Unity Purchasing System. Lo cual sería un nombre maravilloso para esta variable, en vez de decir dos veces que es el storeController. Esto se aplica a varias cosas, si tienes un string que define un nombre, no le llames "nameString" porque esa información extra no sirve de nada. Así mismo poner List<Line> lineList, es mala idea.

Con los IDE's actuales poner prefijos no es una buena idea. Supongamos que "m_" viene de mobile y "k" de constante. Pero es que no debería tener que aprenderme varias convenciones de nombres para poder entender un código. Si comienzas a escribir "k" en un IDE y tienes muchas constantes con ese prefijo, te tendrás que aprender la verdadera primera letra igualmente para que el IDE haga su trabajo. Si tienes pocas constantes, es bastante normal acordarse del nombre de todas ellas, porque deberían ser suficientemente importantes como para merecerse su valor constante.  (Y si no se va a cambiar en tiempo de ejecución (porque son constantes) márcala como readonly) .

En resumen, manteniendo unas reglas claras y teniendo en la cabeza que haciendo código mantenible nos ayudamos a nosotros y a los demás, haremos del mundo un lugar mejor.

Responder

Introduce tus datos o haz clic en un icono para iniciar sesión:

Logo de WordPress.com

Estás comentando usando tu cuenta de WordPress.com. Cerrar sesión / Cambiar )

Imagen de Twitter

Estás comentando usando tu cuenta de Twitter. Cerrar sesión / Cambiar )

Foto de Facebook

Estás comentando usando tu cuenta de Facebook. Cerrar sesión / Cambiar )

Google+ photo

Estás comentando usando tu cuenta de Google+. Cerrar sesión / Cambiar )

Conectando a %s

Crea un sitio web o blog en WordPress.com

Subir ↑

A %d blogueros les gusta esto: