Application Checker: mejores practicas de programación?

Si no has trabajado en un ISV hay muchas posibilidades de que nunca te hayas preocupado de las Best Practices de Dynamics (BP), o quizá sí. Yo no he trabajado para un ISV pero cuando empecé a trabajar con AX me dieron el documento de BP de desarrollo y he intentado seguir la mayoría cuando programo.

Pero las BPs se podían ignorar y no seguirlas sin problema. Esto es por lo que Microsoft va a publicar…

Application Checker

Application Checker es una herraminta que pretende cambiar esto. Va a forzar algunas reglas que nuestro código tendrá que cumplir, de otra forma no va a compilar (y quizá ni se despliegue en los entornos).

Tuvimos un pequeño avance en el pasado MBAS, en la sesión “X++ programming with quality” de Dave Froslie y Peter Villadsen. Desafortunadamente la sesión no se grabó.

App checker usa BaseX, una herramienta de análisis XML, y alimenta a Socratex, que usará Microsoft para auditar la calidad del código. No sé si Socratex será publicado y no recuerdo si se dijo nada durante la charla.

El conjunto de reglas se puede encontrar en elproyecto de GitHub de Application Checker y todavía es WIP. Creo que hay muuuuchas cosas a decidir antes de que esto sea GA, y algunas reglas me preocupan y asustan 😛

Tipos de reglas

Hay distintos tipos de reglas, algunas provocarán errores y otras warnings. Por ejemplo:

ExtensionsWithoutPrefix.xq: esta regla provocará un error evitando que tu código compile. Comprueba que una clase de extensión que acabe en _Extension y el atributo ExtensionOf. En caso de que así sea debe tener un prefijo. Por ej.: si extendemos la clase CustPostInvoice no se puede llamar CustPostInvoice_Extension, necesita un prefijo como CustPostInvoiceAAS_Extension.

SelectForUpdateAbsent.xq: esta regla lanzará un warning. Si hay una cláusula forUpdate en un select y no se llama a doUpdate, update, delete, doDelete o write posteriormente nos lo hará saber.

A día de hoy hay 21 reglas en el proyecto de GitHub. Se puede contribuir al proyecto, o puedes crear tus propias reglas sin mandarlas al proyecto y forzarlas en las máquinas de desarrollo con añadirla a la carpeta local de reglas. Yo crearía una que hiciera que después de un if/while/for/switch sea obligatorio dejar el espacio en blanco, pero eso sólo es cosa de mi TOC al escribir/leer código.

Pruébalo en tu código

Podemos usar Application Checker en nuestras VMs de desarrollo desde, creo, el PU26. Sólo tenemos que instalar JRE y BaseX en la máquina y seleccionar el check al hacer una full build.

Algunos ejemplos

ComplexityIndentationCombined.xq

Esta consulta comprueba la (agarráos a la silla)This query checks the (wait for it…) complejidad ciclomática de los métodos. Lo intentaré explicar… la complejidad ciclomática es una métrica de calidad de software, y es el número de caminos independientes que puede seguir el código. Dependiendo del número de ifs, whiles, switches, etc… el código puede tener distintas salidas por distintos caminos, es es lo que calcula la complejidad.

Tomando este ejemplo, que es muy sencillo para demostrarlo, vemos la gran cantidad de caminos que podría seguir:

class AASAppCheckerDemo
{            
    public void complexMethod()
    {
        int a, b, c, d, e, f, g, h, i;

        if (a)
        {
            if (d)
            {				
                if (d)
                {
                }
                else if (e)
                {
                }
                else if(f)
                {
                }                
            }
            else if (e)
            {
                if (d)
                {
                }
                else if (e)
                {
                }
                else if(f)
                {
                }                
            }
            else if(f)
            {                
                if (d)
                {
                }
                else if (e)
                {
                }
                else if(f)
                {
                }
            }
        }
        else if (b)
        {
            if (d)
            {
                if (d)
                {
                }
                else if (e)
                {
                }
                else if(f)
                {
                }
            }
            else if (e)
            {
                if (d)
                {
                }
                else if (e)
                {
                }
                else if(f)
                {
                }                
            }
            else if(f)
            {               
                if (d)
                {
                }
                else if (e)
                {
                }
                else if(f)
                {
                }                
            }
        }
        else if(c)
        {
            if (d)
            {                
                if (d)
                {
                }
                else if (e)
                {
                }
                else if(f)
                {
                }                
            }
            else if (e)
            {                
                if (d)
                {
                }
                else if (e)
                {
                }
                else if(f)
                {
                }                
            }
            else if(f)
            {
                if (d)
                {
                }
                else if (e)
                {
                }
                else if(f)
                {
                }
            }
        }        
    }

}

En App checker el error aparece cuando la complejidad es más de 30. He usado el analizador de complejidad Lizard para calcular la complejidad del método y es de 49.

La regla también comprueba la profundidad de indentación, fallando si es mayor de 2. Al final el propósito de estas reglas es que troceemos métodos largos en partes más pequeñas, lo que también ayudará a que nuestro código sea más extensible, como hizo Microsoft con las clases Data Provider de los informes.

BalancedTtsStatement.xq

Con esta regla tengo sentimientos encontrados. La regla comprueba que los ttsbegin y ttscommit de un método estén dentro del mismo ámbito. Así que esto ya no será posible:

public void ttsCheck()
{
    StagingTable stagingTable;

    try
    {
        while select forupdate stagingTable
            where !stagingTable.Processed
        {
            ttsbegin;                
            boolean ret = this.doThings();

            if (ret)
            {
                stagingTable.Processed = true;
                stagingTable.update();
                ttscommit;
            }
            else
            {
                ttsabort;

                ttsbegin;
                stagingTable.Processed	= false;
                stagingTable.ErrorMsg	= 'An error ocurred, see log.';
                stagingTable.update();
                ttscommit;
            }
        }
    }
    catch (Exception::Error)
    {
        ttsabort;

        ttsbegin;
        stagingTable.Processed = false;
        stagingTable.update();
        ttscommit;
    }
    catch
    {
        ttsabort;

        ttsbegin;
        stagingTable.Processed = false;
        stagingTable.update();
        ttscommit;
    }
}

private boolean doThings()
{
    return true;
}

Imagina que has desarrollado una integración con una aplicación externa que introduce datos en una tabla intermedia y que luego se procesan esos datos de forma secuencial. No quieres que si algo falla haya un throw para que el proceso siga con el siguiente registro, así que llamas a ttsabort, guardas el error y continúas. Si esto no es posible… cómo deberemos hacerlo? Crear un lote que genere una tarea por cada línea a procesar?

Además, los modelos del estándar están llenos de ttscommit dentro de ifs.

RecursiveMethods.xq

Esta regla bloqueará el uso de recursividad en métodos estáticos. No tengo muy claro por qué. Application checker debería ser una forma de promover buenas prácticas, no prohibir algunos patrones. Si alguien mete un método recursivo en producción y no se llega nunca a la condición de salida… hola testeo?

Algunas reflexiones finales

¿Va a forzar esto a programar mejor? Lo dudo mucho, pero seguramente no sea el propósito de App Checker. Durante milenios los humanos hemos encontrado la forma de saltarnos las reglas, leyes y todo tipo de restricciones, y esto no será una excepción.

¿Nos ayudará? ¡Joder sí! Pero la mejor forma de asegurarnos de la calidad del código es promoviendo las buenas prácticas en nuestros equipos, a través de formación interna o revisiones de código. E incluso así si a alguien le importa el código limpio un pepino seguirá escribiendo código terrible. Puede que funcione pero no será bonito.

Finalmente, algunas reglas no me convencen. Como lo de evitar la recursividad o el tema de los tts. Tendremos que esperar a ver qué reglas se lanzan finalmente y cómo se va a implantar Application checker en el ALM de MSDyn365FO, bloqueando (o no) los despliegues de código o si se va a incluir en el proceso de build.

Deja un comentario

Tu dirección de correo electrónico no será publicada. Los campos obligatorios están marcados con *

Este sitio usa Akismet para reducir el spam. Aprende cómo se procesan los datos de tus comentarios.