Refactorisation de Code

Sur un projet récent, j’ai été chargé de refactoriser de grandes parties d’un système web. Il est écrit en C#. Au fil du temps, certains des fichiers code-behind avaient atteint 4000 lignes. L’objectif était de réduire ce nombre à un niveau plus maintenable.

Au cours des prochains articles, j’ai pris des extraits de code que j’ai refactorisés et j’expliquerai mes réflexions et comment je suis arrivé à la solution.

Le premier extrait de code :

    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;
        }
    }

L’extrait de code ci-dessus est une collection d’instructions if, qui sont une évaluation et une exécution. Dans ma première tentative, j’ai essayé d’utiliser la même évaluation pour toutes les instructions if, mais j’ai ensuite réalisé qu’une était différente. Ne comprenant pas l’intention du code, je suis forcé de préserver la logique mot pour mot.

Évaluation if différente :

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

L’instruction switch m’inquiétait. La condition pour entrer dans l’instruction switch est la même que les autres. J’ai décidé de procéder et de m’inquiéter de l’instruction switch plus tard.

Le code utilise la même variable, la variable ‘tmp’, pour récupérer différentes valeurs de requête. La valeur est écrasée à chaque récupération de valeur de requête. Pour plus de clarté, je crée une variable pour chaque valeur de requête :

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"];

L’étape suivante était d’isoler l’évaluation et l’expression tout en les gardant associées l’une à l’autre. Si une évaluation est vraie, je veux exécuter son expression correspondante. J’ai créé une classe qui représentait l’association.

private class Evaluate
{

    public Func Evaluation { get; set; }

    public Action Expression { get; set; }
}

Maintenant je peux créer une évaluation, et si elle est vraie, je peux exécuter son expression.

Le problème suivant était comment utiliser la classe ci-dessus avec toutes les instructions if. J’étais inquiet que les expressions puissent devenir difficiles à gérer dans une collection. Le but entier était de créer une solution concise et évolutive. La solution existante n’était ni l’une ni l’autre.

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);}}, 
               };

Cela s’est avéré mieux que prévu. Un inconvénient de ma solution est que, si vous ne savez pas comment utiliser les délégués, vous serez dans l’embarras quand il s’agira de maintenir le code ci-dessus.

Le dernier obstacle était l’instruction switch. Elle n’allait pas s’intégrer gracieusement dans ma collection anonyme, mais elle n’en avait pas besoin :

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;
    }
}

En l’encapsulant dans une méthode, j’ai pu référencer la méthode dans l’expression. Cela a très bien fonctionné.

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

Le dernier composant est l’itération sur la collection :

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

La solution complète :

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();
    }
}

Au final, j’aime mieux cette solution que l’originale. Un des inconvénients est le niveau auquel elle est écrite. Je voulais créer une solution plus simple que n’importe quel développeur pourrait maintenir. Il n’y a rien de difficile dans le code ci-dessus ; je crée une collection et j’itère dessus. La confusion vient avec l’évaluation et les expressions. Ce n’est pas un sujet pour débutants.