Refactorización de Código

22 de octubre de 2010 • 6 min de lectura

En un proyecto reciente, me asignaron la tarea de refactorizar grandes partes de un sistema web. Está escrito en C#. Con el tiempo, algunos de los archivos code-behind habían crecido a 4000 líneas. El objetivo era reducir este número a un nivel más manejable.

En las próximas publicaciones, he tomado fragmentos de código que refactoricé y explicaré mis pensamientos y cómo llegué a la solución.

El primer fragmento de código:

    string tmp = Request.QueryString["st"];
    _varStartRecNum = tmp;
    if ((tmp != null) & (!Page.IsPostBack))
    {
        _varStartRecNum = tmp;
        postBack = true;
    }

    tmp = Request.QueryString["det"];
    if ((tmp != null) & (!Page.IsPostBack))
    {
        _varDetailsRecNum = tmp;
        postBack = true;
    }

    tmp = Request.QueryString["return"];
    if ((tmp != null) & (!Page.IsPostBack))
    {
        postBack = true;
    }

    tmp = Request.QueryString["searchnow"];
    if ((tmp != null) & (!Page.IsPostBack))
    {
        Session["selectedTab"] = "mtf";
        Session["sessionDSProviders"] = null;
        Session["mtfs"] = null;
    }

    tmp = Request.QueryString["displaywalking"];
    if (tmp == "true")
    {
        dispMtf = false;
        postBack = true;
    }

    tmp = Request.QueryString["sb"];

    if ((tmp != null) & (!Page.IsPostBack))
    {
        _varSortBy = tmp;
        postBack = true;
        switch (_varSortBy)
        {
            case "Distance":
            case "Drive time":
                ddlSortBy.SelectedIndex = 0;
                break;
            case "Name":
                ddlSortBy.SelectedIndex = 1;
                break;
            case "Gender":
                ddlSortBy.SelectedIndex = 2;
                break;
            case "Clinic":
                ddlSortBy.SelectedIndex = 3;
                break;
            case "City":
                ddlSortBy.SelectedIndex = 4;
                break;
            case "Description":
                ddlSortBy.SelectedIndex = 5;
                break;
        }
    }

El fragmento de código anterior es una colección de declaraciones if, que son una evaluación y una ejecución. En mi primer intento, traté de usar la misma evaluación para todas las declaraciones if, pero luego me di cuenta de que una era diferente. Al no entender la intención del código, me veo obligado a preservar la lógica textualmente.

Evaluación if diferente:

tmp = Request.QueryString["displaywalking"];
if (tmp == "true")
{
    dispMtf = false;
    postBack = true;
}

La declaración switch me preocupaba. La condición para entrar en la declaración switch es la misma que las otras. Decidí proceder y preocuparme por la declaración switch más tarde.

El código usa la misma variable, la variable ‘tmp’, para recuperar diferentes valores de consulta. El valor se sobrescribe con cada recuperación de valor de consulta. Para mayor claridad, creo una variable para cada valor de consulta:

string st = Request.QueryString["st"];
string det = Request.QueryString["det"];
string @return = Request.QueryString["return"];
string searchNow = Request.QueryString["searchnow"];
string displayWaling = Request.QueryString["displaywalking"];
string sb = Request.QueryString["sb"];

El siguiente paso fue aislar la evaluación y la expresión mientras las mantenía asociadas entre sí. Si una evaluación es verdadera, quiero ejecutar su expresión correspondiente. Creé una clase que representaba la asociación.

private class Evaluate
{

    public Func Evaluation { get; set; }

    public Action Expression { get; set; }
}

Ahora puedo crear una evaluación, y si es verdadera, puedo ejecutar su expresión.

El siguiente problema fue cómo usar la clase anterior con todas las declaraciones if. Me preocupaba que las expresiones pudieran volverse difíciles de manejar en una colección. Todo el propósito era crear una solución concisa y escalable. La solución existente no era ninguna de las dos.

var eval = new[]
               {
                   new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(st) && !IsPostBack), Expression = () => { _varStartRecNum = st;postBack = true; }},
                   new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(det) && !IsPostBack), Expression = () => { _varStartRecNum = det;postBack = true; }}, 
                   new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(@return) && !IsPostBack), Expression = () => {postBack = true; }}, 
                   new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(searchNow) && !IsPostBack), Expression = () => {Session["selectedTab"] = "mtf";Session["sessionDSProviders"] = null; Session["mtfs"] = null;}}, 
                   new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(displayWaling)), Expression = () => {dispMtf = false; postBack = true;}}, 
                   new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(sb) && !IsPostBack), Expression = () => {_varSortBy = sb;postBack = true; SetSort(_varSortBy);}}, 
               };

Resultó mejor de lo que esperaba. Una desventaja de mi solución es que, si no sabes cómo usar delegados, estarás perdido cuando se trate de mantener el código anterior.

El último obstáculo fue la declaración switch. No iba a encajar elegantemente en mi colección anónima, pero entonces no necesitaba hacerlo:

private void SetSort(string sortBy)
{
    switch (sortBy)
    {
        case "Distance":
        case "Drive time":
            ddlSortBy.SelectedIndex = 0;
            break;
        case "Name":
            ddlSortBy.SelectedIndex = 1;
            break;
        case "Gender":
            ddlSortBy.SelectedIndex = 2;
            break;
        case "Clinic":
            ddlSortBy.SelectedIndex = 3;
            break;
        case "City":
            ddlSortBy.SelectedIndex = 4;
            break;
        case "Description":
            ddlSortBy.SelectedIndex = 5;
            break;
    }
}

Al encapsularlo en un método, pude referenciar el método en la expresión. Funcionó muy bien.

new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(sb) && !IsPostBack), Expression = () => {_varSortBy = sb;postBack = true; SetSort(_varSortBy);}

El último componente es iterar sobre la colección:

foreach (var evaluate in eval.Where(evaluate => evaluate.Evaluation()))
{
    evaluate.Expression();
}

La solución completa:

private class Evaluate
{
    public Func Evaluation { get; set; }

    public Action Expression { get; set; }
}

private void SetSort(string sortBy)
{
    switch (sortBy)
    {
        case "Distance":
        case "Drive time":
            ddlSortBy.SelectedIndex = 0;
            break;
        case "Name":
            ddlSortBy.SelectedIndex = 1;
            break;
        case "Gender":
            ddlSortBy.SelectedIndex = 2;
            break;
        case "Clinic":
            ddlSortBy.SelectedIndex = 3;
            break;
        case "City":
            ddlSortBy.SelectedIndex = 4;
            break;
        case "Description":
            ddlSortBy.SelectedIndex = 5;
            break;
    }
}

private void EvaluateQueryParameters()
{
    string st = Request.QueryString["st"];
    string det = Request.QueryString["det"];
    string @return = Request.QueryString["return"];
    string searchNow = Request.QueryString["searchnow"];
    string displayWaling = Request.QueryString["displaywalking"];
    string sb = Request.QueryString["sb"];

    var eval = new[]
                   {
                       new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(st) && !IsPostBack), Expression = () => { _varStartRecNum = st;postBack = true; }},
                       new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(det) && !IsPostBack), Expression = () => { _varStartRecNum = det;postBack = true; }}, 
                       new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(@return) && !IsPostBack), Expression = () => {postBack = true; }}, 
                       new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(searchNow) && !IsPostBack), Expression = () => {Session["selectedTab"] = "mtf";Session["sessionDSProviders"] = null; Session["mtfs"] = null;}}, 
                       new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(displayWaling)), Expression = () => {dispMtf = false; postBack = true;}}, 
                       new Evaluate {Evaluation = () => (!string.IsNullOrEmpty(sb) && !IsPostBack), Expression = () => {_varSortBy = sb;postBack = true; SetSort(_varSortBy);}}, 
                   };

    foreach (var evaluate in eval.Where(evaluate => evaluate.Evaluation()))
    {
        evaluate.Expression();
    }
}

Al final, me gusta más esta solución que la original. Una de las desventajas es el nivel en el que está escrita. Quería crear una solución más simple que cualquier desarrollador pudiera mantener. No hay nada difícil sobre el código anterior; estoy creando una colección e iterando sobre ella. La confusión viene con la evaluación y las expresiones. No es un tema para principiantes.