Code-Refaktorierung

In einem kürzlichen Projekt wurde ich damit beauftragt, große Teile eines Websystems zu refaktorieren. Es ist in C# geschrieben. Im Laufe der Zeit waren einige der Code-Behind-Dateien auf 4000 Zeilen angewachsen. Das Ziel war es, diese Zahl auf ein wartungsfreundlicheres Niveau zu reduzieren.

In den nächsten Beiträgen habe ich Code-Snippets genommen, die ich refaktoriert habe, und werde meine Gedanken und den Lösungsweg erklären.

Das erste Code-Snippet:

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

Das obige Code-Snippet ist eine Sammlung von if-Anweisungen, die eine Auswertung und eine Ausführung darstellen. In meinem ersten Versuch versuchte ich, dieselbe Auswertung für alle if-Anweisungen zu verwenden, aber dann stellte ich fest, dass eine anders war. Da ich die Absicht des Codes nicht verstehe, bin ich gezwungen, die Logik wörtlich zu bewahren.

Unterschiedliche if-Auswertung:

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

Die switch-Anweisung bereitete mir Sorgen. Die Bedingung für den Eintritt in die switch-Anweisung ist dieselbe wie bei den anderen. Ich beschloss fortzufahren und mir später Gedanken über die switch-Anweisung zu machen.

Der Code verwendet dieselbe Variable, die ‘tmp’-Variable, um verschiedene Query-Werte abzurufen. Der Wert wird mit jedem Query-Wert-Abruf überschrieben. Für mehr Klarheit erstelle ich eine Variable für jeden Query-Wert:

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

Der nächste Schritt war, die Auswertung und den Ausdruck zu isolieren, während sie miteinander verknüpft bleiben. Wenn eine Auswertung wahr ist, möchte ich den entsprechenden Ausdruck ausführen. Ich erstellte eine Klasse, die diese Verknüpfung repräsentiert.

private class Evaluate
{

    public Func Evaluation { get; set; }

    public Action Expression { get; set; }
}

Jetzt kann ich eine Auswertung erstellen, und wenn sie wahr ist, kann ich ihren Ausdruck ausführen.

Das nächste Problem war, wie die obige Klasse mit allen if-Anweisungen verwendet werden kann. Ich befürchtete, dass die Ausdrücke in einer Sammlung unhandlich werden könnten. Der ganze Zweck war es, eine prägnante, skalierbare Lösung zu schaffen. Die bestehende Lösung war weder das eine noch das andere.

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

Es stellte sich besser heraus als erwartet. Ein Nachteil meiner Lösung ist, dass Sie aufgeschmissen sind, wenn Sie nicht wissen, wie man Delegates verwendet, wenn es um die Wartung des obigen Codes geht.

Der letzte Stolperstein war die switch-Anweisung. Sie würde nicht elegant in meine anonyme Sammlung passen, aber das musste sie auch nicht:

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

Durch die Kapselung in eine Methode konnte ich die Methode im Ausdruck referenzieren. Es funktionierte sehr schön.

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

Die letzte Komponente ist die Iteration über die Sammlung:

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

Die vollständige Lösung:

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

Am Ende gefällt mir diese Lösung besser als das Original. Einer der Nachteile ist das Niveau, auf dem sie geschrieben ist. Ich wollte eine einfachere Lösung schaffen, die jeder Entwickler warten kann. Es gibt nichts Schwieriges an dem obigen Code; ich erstelle eine Sammlung und iteriere darüber. Die Verwirrung entsteht bei der Auswertung und den Ausdrücken. Es ist kein Anfängerthema.