Koderefaktorering

På et nylig prosjekt fikk jeg i oppgave å refaktorere store deler av et websystem. Det er skrevet i C#. Over tid hadde noen av code-behind-filene vokst til 4000 linjer. Målet var å få dette tallet ned til et mer håndterbart nivå.

I de neste innleggene har jeg tatt kodeutdrag som jeg refaktorerte og vil forklare tankene mine og hvordan jeg kom frem til løsningen.

Det første kodeutdraget:

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

Kodeutdraget ovenfor er en samling av if-setninger, som er en evaluering og en utførelse. I mitt første forsøk prøvde jeg å bruke samme evaluering for alle if-setninger, men så innså jeg at en var annerledes. Uten å forstå hensikten med koden er jeg tvunget til å bevare logikken ordrett.

Annerledes if-evaluering:

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

Switch-setningen bekymret meg. Betingelsen for å gå inn i switch-setningen er den samme som de andre. Jeg bestemte meg for å fortsette og bekymre meg for switch-setningen senere.

Koden bruker samme variabel, ‘tmp’-variabelen, for å hente forskjellige query-verdier. Verdien overskrives med hver query-verdi-henting. For klarhet opprettet jeg en variabel for hver query-verdi:

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

Neste steg var å isolere evalueringen og uttrykket mens jeg holdt dem assosiert med hverandre. Hvis en evaluering er sann, vil jeg utføre det tilsvarende uttrykket. Jeg opprettet en klasse som representerte assosiasjonen.

private class Evaluate
{

    public Func Evaluation { get; set; }

    public Action Expression { get; set; }
}

Nå kan jeg opprette en evaluering, og hvis den er sann, kan jeg utføre uttrykket.

Neste problem var hvordan jeg skulle bruke klassen ovenfor med alle if-setningene. Jeg var bekymret for at uttrykkene kunne bli uhåndterlige i en samling. Hele hensikten var å skape en konsis skalerbar løsning. Den eksisterende løsningen var ingen av delene.

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

Det viste seg bedre enn jeg forventet. En ulempe med løsningen min er at hvis du ikke vet hvordan du bruker delegater, vil du være i trøbbel når det kommer til å vedlikeholde koden ovenfor.

Den siste hindringen var switch-setningen. Den kom ikke til å passe elegant inn i min anonyme samling, men det trengte den heller ikke:

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

Ved å innkapsle den i en metode kunne jeg referere til metoden i uttrykket. Det fungerte veldig fint.

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

Den siste komponenten er å iterere over samlingen:

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

Den komplette løsningen:

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

Til slutt liker jeg denne løsningen bedre enn originalen. En av ulempene er nivået den er skrevet på. Jeg ønsket å skape en enklere løsning som enhver utvikler kunne vedlikeholde. Det er ikke noe vanskelig med koden ovenfor; jeg oppretter en samling og itererer over den. Forvirringen kommer inn med evalueringen og uttrykkene. Det er ikke et nybegynnertema.