Refactorizando JS (presumiblemente) productivo

Uno suele cansarse de que gran parte del material relativo a Software que uno consiga en Internet esté dirigido a newbies.
Sin embargo, hasta que los newbies no apliquemos el sabio principio RTFM, se hace necesario repetir lo mismo una y otra vez.
Digo esto con conocimiento de causa: soy un grandísimo newbie. La diferencia es que nunca desprecio un consejo y siempre escucho a los que saben más que yo.
Un par de aclaraciones antes de comenzar:

• El "presumiblemente productivo" del título pone en duda la "productividad" del código, no que esté o no disponible públicamente ya que de hecho lo está.

• Hay muchas más cuestiones que corregir del código, relacionadas con el estilo o uso de algunas mejoras traídas por las nuevas versiones de JS, pero todas estas cuestiones saldrían a la luz con el uso de un linter, así que la idea de estas entradas es que se enfoquen en cuestiones más conceptuales o relacionadas con el diseño.

Comencemos:

function msjInfo( boolean, msj ){
    if( boolean ){
        if(!$('.mensaje-mal').length > 0)$('body').append('<div class="mensaje-bar mensaje-mal"><span class="msj"></span></div>');
        $('.mensaje-mal').fadeTo("slow", 1).animate({opacity: 1.0}, 3000).fadeTo("slow", 0);
        $('.mensaje-mal .msj').html(msj);
        $('.mensaje-mal').css('margin-left', ($('body').width()/2 -  $('.mensaje-mal').width()/2) + 'px');
    }else{
        if(!$('.mensaje-bien').length > 0)$('body').append('<div class="mensaje-bar mensaje-bien"><span class="msj"></span></div>');
        $('.mensaje-bien').fadeTo("slow", 1).animate({opacity: 1.0}, 3000).fadeTo("slow", 0);
        $('.mensaje-bien .msj').html(msj);
        $('.mensaje-bien').css('margin-left', ($('body').width()/2 -  $('.mensaje-bien').width()/2) + 'px');
    }
}

Supongamos que msjInfo es el nombre adecuado para una función. Que esta función tan bien nombrada reciba un parámetro llamado boolean es molesto por varias razones:

• Code smell: parece ser un parámetro de control desde "el vamos", es como si el llamador debiera conocer la implementación de esta función para saber como llamarlo.
• Boolean es un tipo de datos no un nombre. Nos dice tanto sobre el parámetro en sí como si lo hubiesemos llamado verdadero_o_falso.

Queda claro que lo que hace boolean es determinar con que clase css se va a trabajar en la función bien nombrado. El code smell estaba en lo cierto. Quien invoca esta función tiene que saber que hace la función para cada valor que puede asumir la variable (que por suerte son solo dos).
Las 3 líneas de código que componen el bloque if y el bloque else son iguales! Salvo por la clase CSS con la que operan. Si no queremos cambiar la interfaz del método podemos hacer lo sgte:

function msjInfo (exito, msj) {
   const cssClass = exito? '.mensaje-mal' :  '.mensaje-bien';
   const div4msg = `<div class="mensaje-bar ${cssClass}"><span class="msj"></span></div>`;
   $('body').append(div4msg);
   $(cssClass).fadeTo("slow", 1).animate({opacity: 1.0}, 3000).fadeTo("slow", 0);
   $(cssClass + ' .msj').html(msj);
   $(cssClass).css('margin-left', ($('body').width()/2 - $(cssClass).width()/2) + 'px');
}

Mejor, pero sigue habiendo cosas sobre las que trabajar en esa porción. Después, un poco de asincronicidad usando ajax tenemos:

$.ajax({
        url: ...
        type: "POST",
        data: ...,
        context: document.body,
        success: function( data ){
            retorno = eval( '(' + data + ')' );
            if(retorno){
                msjInfo( true, "Completé mi mandato y voy por la reelección" );
            }else{
                msjInfo( false, "No pude completar mi mandato y debí huir en helicoptero" );
            }
        }
});

Esto es peligroso. No solo por usar eval(), sino por usarlo por un error de diseño. La documentación de jQuery, respecto a la función success(): "es la función a ser llamada si la request ha tenido éxito". Es clarísimo, si la request no tuvo éxito, la función invocada es la función error: "es la función a ser llamada si la request ha fallado".
De esto se desprenden 2 cosas: la request puede salir bien o no. Súperlógico.
Sin embargo, y sin revisar el script PHP invocado mediante ajax sabemos que eso no pasa, que para ese script la request siempre tiene éxito y por eso es necesario romper jQuery y el sentido común y hacer ese peligroso eval().
Lo correcto sería que el script PHP invocado retorne un mensaje de error (probablemente de código 500 pero podría variar) usando la función header de PHP.
Por otro lado, tenemos que la regex que valida emails es:

^[_a-z0-9-]+(.[_a-z0-9-]+)*@[a-z0-9-]+(.[a-z0-9-]+)*(.[a-z]{2,3})$

Me parece extraña e ineficiente. Podemos usar algunas herramientas muy prácticas para el análisis como:

• Un validador y debugger de regex
• Un generador de strings a partir de regex
• Un graficador de regex

Pero intentaremos ordenar a mano y ver si podemos aprender algo así.
Reemplacemos _a-z0-9- por un conjunto de caracteres que llamaremos A e ignoremos los paréntesis inútiles. La regex quedaría:

^[A]+(.[A]+)*@[A]+(.[A]+)*.[a-z]{2,3}$

Achiquemos un poco el scope y veamos solo la parte del mail que antecede al @ por si el absurdo no es suficientemente obvio aún:

^[A]+(.[A]+)* 

puede leerse como: "La línea comienza con al menos un caracter del conjunto A, seguido por un caracter cualquiera, y después al menos un caracter A. Ese caracter debe estar seguido por al menos un caracter del grupo A y esto puede ocurrir de 0 a N veces."
Lección: si no saben:

• Googlear una regex, o
• construir una regex, o
• Usar una api para validar una regex

Por favor ofusquen su código js para no terminar escribiendo a correos como "__@--.xx".

Volviendo a Javascript y terminando la presente entrada tenemos

function getGanadores( _PaginaActual ){
   PaginaActual = _PaginaActual;
   <resto del código de la función que no utiliza jamás _PaginaActual>
}

Recibir con un parámetro con un nombre y cambiarlo inmediatamente después es bastante tonto.