Poznámky z code review 3

Další nedostatkem, se kterým se setkávám, je buď neznalost používaných technologiích a nebo nevyužívání jejich možností. To lze určitě pochopit, pokud kodér nepracuje s danou technologii určitý čas a nebo je jasné, že není expertem. Code review by ho  v takovém případě mělo upozornit na jiné možnosti a tedy tak vlastně přispívá k jeho odbornému růstu - a nebo může sloužit k vyvolání diskuze, který přístup je lepší a proč.

Například mi byl předán takovýto Razor kód:

@model OurModel

@{
   var mainClass = "main";
   var hasProperty = SomePropertyProvider.HasProperty("Name");

   if (Model.IsReadOnly)
   {
      mainClass += " readonly";
   }

   if(Model.Values.Any(v=> IsWrong(v))
   {
      mainClass += " error";
   }

   var alertId = Guid.NewGuid().ToString();
}
   
<div class="@mainClass">
    <table class="Header">
        <tr>
            <td class="modelType">
.....

Je jasné, že se kodér snažil sestavit  jména stylu - a dopustil se chyb - například v bloku kódu střídá několik věcí - jednak sestavuje jména stylů, mezitím ale provádí ale volání s touto akcí nijak nespjatých. Správně by měl blok vypadat asi takto:

@model OurModel

@{
   var alertId = Guid.NewGuid().ToString();
   var hasProperty = SomePropertyProvider.HasProperty("Name");

   var mainClass = "main";

   if (Model.IsReadOnly)
   {
      mainClass += " readonly";
   }

   if(Model.Values.Any(v=> IsWrong(v))
   {
      mainClass += " error";
   }
}
   
<div class="@mainClass">
    <table class="Header">
        <tr>
            <td class="modelType">
.....

To už vypadá lépe, ale stále to není ono. Kombinování řetězců výše uvedeným způsobem není příliš štastné a i když uvedený příklad by asi nevedl k velké paměťové zátěži, je lepší uvedené "sčítání" řetězců nepoužívat. Jinou možnost nabízí MVC helper:

@model OurModel

@helper GetMainClasses()
{
   @:message

   if(Model.IsReadOnly())
   {
      @:readonly
   }

   if(Model.Values.Any(v=> IsWrong(v))
   {
      @:error
   }
}
   
<div class="@GetMainClasses()">
    <table class="Header">
        <tr>
            <td class="modelType">
.....

a nebo při rozepsání na více helperů:

@model OurModel

@functions
{
    string IsReadonly()
    {
        if (Model.IsReadOnly)
            return "readonly";
        
        return string.Empty;
    }
    
    string IsInError()
    {
        if(Model.Values.Any(v=> IsWrong(v))
            return "error";

        return string.Empty;       
    }
}
   
<div class="message @IsReadonly() @IsInError()">
    <table class="Header">
        <tr>
            <td class="modelType">
.....

a nebo alternativně:

@model OurModel

@helper IsInError()
{
    @(Model.Values.Any(v=> IsWrong(v) ? "error" : string.Empty)
}

@helper IsReadonly()
{
    @(Model.IsReadOnly() ? "readonly" : string.Empty)
}
   
<div class="message @IsReadonly() @IsInError()">
    <table class="Header">
        <tr>
            <td class="modelType">
.....

popřípadě in-line kódem:

@model OurModel

<div class="message @(Model.IsReadOnly() ? "" : string.Empty) @(Model.Values.Any(v=> IsWrong(v) ? "" : string.Empty)">
    <table class="Header">
        <tr>
            <td class="modelType">
.....

a nebo pomocí funkce:

@model OurModel

@functions
{
   string AddClass(bool add, string name)
   {
      return add ? name : null;
   }
}

<div class="message @(AddClass(Model.IsReadOnly(), "readonly")) @(AddClass(Model.Values.Any(v=> IsWrong(v), "error"))">
    <table class="Header">
        <tr>
            <td class="modelType">
.....

Chápu, že na daný příklad a řešení mohou být různé názory, kód nebyl vysloveně špatný a nefunkční, ale jak jsem již napsal, lze code review pojmout i jako možnost vzdělávání se či prostor k diskuzi. Helpery z uvedených příkladů lze přesunout na konec stránky a mít tak  na začátku téměř čisté HTML. Čitelnost ukázek v VS je lepší, nějak mám problém vkládat ukázky, která mají odpovídající barevné zvýraznění.