Buď explicitní

TLDR: Čím obecnější kód píšeme, tím méně informací v kódu zůstává. Snažte se jasně a přesně zapsat myšlenku do kódu a nenechte čtenáře hádat.

Podivný bugfix

// Kód je zjednodušený pro ukázku
public string GetWebsiteUrl(
  string shopId,
  string? itemUrl)
{
  return $"{shopId}/website/{itemUrl}/";
}

Pokud je itemUrl = null, tak vygenerovaná URL bude vypadat třeba takto: cz/website//.

Někdo tuto chybu opravil následovně:

public string GetWebsiteUrl(
  string shopId,
  string? itemUrl)
{
  return Regex.Replace($"{shopId}/website/{itemUrl}/", @"/+", @"/");
}

Tento regex najde všechny výskyty // a nahradí je jedním /, což je technicky vzato správné řešení. Problém je, že jsme ztratili informaci o tom, proč regex existuje.

Pokud se na tento kód podíváte za rok, tak si budete říkat: “Z jakého důvodu byl regex přidán?”

• Ošetřuje situaci, kdy je itemUrl = null?
• itemUrl nebo shopId může být / a regex je potřeba pro ošetření této situace?
• itemUrl nebo shopId může mít formát xxx//xxx nebo /xxx nebo xxx/ a je potřeba tyto situace ošetřit?

Kdybychom chybu opravili následujícím způsobem:

public string GetWebsiteUrl(
  string shopId,
  string? itemUrl)
{
  var itemSanitized = itemUrl == null ? "" : $"{itemUrl}/";
  return $"{shopId}/website/{itemSanitized}";
}

tak je naprosto jasné, co se děje, a není ztracena žádná informace.

Co když se regex jednou bude hodit?

Mohli byste namítnout - co když itemUrl nebo shopId opravdu jednou bude obsahovat / a regex by předešel chybě?

V tomto případě máte 2 rozumné možnosti:

• Ošetřit už někde na vstupu, že itemUrl a shopId neobsahuje ’/’.
• Vytvořit pro itemUrl a shopId speciální typ, který kontroluje, že neobsahují ’/’, a pak předávat tento typ. Jinak řečeno, použít Value object pattern.

Rozumným řešením určitě není, aby nějaká náhodná metoda GetWebsiteUrl dělala sanitaci vstupů a tiše zakrývala chybu, která vznikla už když jsme umožnili vytvořit nevalidní itemUrl nebo shopId.

Obecné pravidlo

Čím obecnější kód píšeme, tím méně informací v kódu zůstává. Snažte se jasně a přesně zapsat myšlenku do kódu a nenechte čtenáře hádat.

Ukažme si ještě jeden příklad.

Barcode

Zákazníci vkládají čárové kódy. Podle toho, ke kterému partnerovi jsou přiřazeni, je potřeba udělat transformaci.

Například:

Každý partner má definovanou regex masku, podle které se informace extrahuje. Implementace vypadala následovně:

internal static string? ExtractByMask(string barcode, string? mask)
{
  if (mask == null)
  {
    return null;
  }

  var test = Regex.Match(barcode, mask);
  if (test.Success)
  {
      return test.Value;
  }
  else
  {
      return null;
  }
}

Metoda pak byla volána takto:

public string ProcessBarcode(
  string barcode,
  Partner partner)
{
  var maskedBarcode = ExtractByMask(barcode, partner.BarcodeMask);
  if(maskedBarcode == null)
  {
    return "Načten špatný čárový kód"; // Zjednodušeno pro ukázku
  }

  // Pokračujeme ve zpracování s maskedBarcode
  // ...
}

Tento kód obsahuje jednu zvláštnost - pokud je partner.BarcodeMask null, tak se uživateli vrátí “Načten špatný čárový kód”. To je trochu divné… Jen proto, že někdo zapomněl nastavit masku, tak uživatel dostane špatný čárový kód?

Očekával bych, že kód bude obsahovat alespoň nějaký log chyby:

public string ProcessBarcode(
  string barcode,
  Partner partner)
{
  if (partner.BarcodeMask == null)
  {
    logger.LogError("Partner {partnerId} nemá nastavenou masku pro čárový kód.", partner.Id);
    return "Načten špatný čárový kód"; // Zjednodušeno pro ukázku
  }

  var maskedBarcode = ExtractByMask(barcode, partner.BarcodeMask);

  if (maskedBarcode == null)
  {
      return "Načten špatný čárový kód"; // Zjednodušeno pro ukázku
  }

  // Pokračujeme ve zpracování
  // ...
}

Možná je můj předpoklad ale špatný… Je možné, že chybějící maska je v této aplikaci běžný stav a nemá význam ho logovat.

Ať už je to jak chce, tak dává smysl, aby byl autor kódu explicitní a jasně nám ukázal, s jakými situacemi počítá. Například tímto způsobem:

public string ProcessBarcode(
  string barcode,
  Partner partner)
{
  if (partner.BarcodeMask == null)
  {
    return "Načten špatný čárový kód"; // Zjednodušeno pro ukázku
  }

  var maskedBarcode = ExtractByMask(barcode, partner.BarcodeMask);
  if (maskedBarcode == null)
  {
      return "Načten špatný čárový kód"; // Zjednodušeno pro ukázku
  }

  // Pokračujeme ve zpracování
  // ...
}

Závěr

Přesně a jasně definujte vaše myšlenky tak, aby čtenář nebyl na pochybách a nemusel hádat, které problémy jste se snažili vyřešit a zda jste některé situace nezapomněli ošetřit.