Poznámky z code review - opět DRY

O tom, že programátor by se měl vyhnout opakování stejného kódu, jsem psal už několikrát - třeba naposledy v druhém článku téhle série Poznámky z code review 2  a párkrát i na starém blogu na serveru Vývojář.  V praxi se pořád ale setkávám s nedodržením zdánlivě jednoduchého pravidla DRY a tak je tenhle příspěvek zase o tom, jak se zbytečně neopakovat - tentokrát u Action metod Controlleru v ASP.NET MVC

U našich projektů používáme obvykle trošku komplikovanější architekturu a tak web aplikace složí jen jako front-end a veškeré rozhodující operace se odehrávají v tzv. střední vrstvě dostupné pro web aplikaci jako  webservices.

A tak v jednom určitém kontrolleru vzniklo několik metod, které se prakticky držely stejného vzorce - otestovat, zde při zpracovaní došlo k nějakému problému a data nemohla být zpracována, protože třeba již existovala apod. (FaultException blok), popřípadě zda nedošlo k nějaké jiné chybě v komunikaci (Exception blok):

[HttpPost]
[ExportModelStateResultToTempData]
public ActionResult WebOperation(int param1, int param2)
{
    try
    {
        this.MiddleService.Operation(param1, param2);
    }
    catch (FaultException ex)
    {
        var description = GetDescriptionFromResources(ex.Code.Name);
        if (string.IsNullOrEmpty(description))
        {
            TraceEvent(TraceEventType.Warning, 81005, "Failed to  find description  for fault:{0}", OS.GetExceptionAsXmlString(ex));
        }
        ModelState.AddModelError(string.Empty, description ?? ex.Message);
    }
    catch (Exception ex)
    {
        TraceEvent(TraceEventType.Error, 81002, "Failed to finish WebOperation:{0}", OS.GetExceptionAsXmlString(ex));
        ModelState.AddModelError(string.Empty, "Failed to  finish WebOperation.");
    }

    return RedirectToAction("GetWebOperation", new { param1 = param1});
}

A všechny tyto metody se podobaly skoro jako vejce vejci, lišily se samozřejmě v bloku try, někdy obsahovaly více volání či více rozhodování. Pokud jste placeni za řádek, je to určitě skvělé, ale čitelnost kódu se zhoršuje.

Stačí ale například přidat tuto privátní metodu:

private void TryCatchOperation(Action action, string failureMessage)
{
    try
    {
        action();
    }
    catch (FaultException ex)
    {
        catch (FaultException ex)
    {
        var description = GetDescriptionFromResources(ex.Code.Name);
        if (string.IsNullOrEmpty(description))
        {
            TraceEvent(TraceEventType.Warning, 81005, "Failed to  find description  for fault:{0}", OS.GetExceptionAsXmlString(ex));
        }
        ModelState.AddModelError(string.Empty, description ?? ex.Message);
    }
    catch (Exception ex)
    {
        TraceEvent(TraceEventType.Error, 81002, failureMessage + " Exception: {0}", OS.GetExceptionAsXmlString(ex));
        ModelState.AddModelError(string.Empty, failureMessage);
    }
}

a všechny takové metody se mohou upravit a zjednodušit:

[HttpPost]
[ExportModelStateResultToTempData]
public ActionResult WebOperation(int param1, int param2)
{
    this.TryCatchOperation(
        () => this.MiddleService.Operation(param1, param2),
        "Failed to  finish WebOperation.");

    return RedirectToAction("GetWebOperation", new { param1 = param1});
}

Poznámka - atribut ExportModelStateResultToTempData je popsán zde