pondělí 24. října 2016

Jednoduchá složitost nebo složitá jednoduchost

Známe to všichni. Kód, který jsme dříve napsali, byl dobře čitelný a lehce upravitelný. Ten samý kód dneska? Nakynuté třídy, dlouhé metody, vysoká komplexita a zvyšující se náklady na požadované změny.

Všude slýcháme, že musíme psát kód jednoduchý, dobře čitelný, s nízkou komplexitou. Dobře, ale co to vlastně v praxi znamená? Zjistil jsem, že my vývojáři si pod "jednoduchostí kódu" často představujeme dost rozdílné věci.

Pojďme se společně podívat na několik řešení stejného problému a přemýšlet nad tím, co si vlastně pod "jednoduchým kódem" představujeme.

Řešíme "problém života"

Jako ukázkový problém nám poslouží hra Game of Life, kterou zpopularizovaly akce typu Coderetreat. Zrovna minulou sobotu byl Global Day of Coderetreat 2016, takže pro mnohé účastníky čerstvá zkušenost.

Hra definuje několik základních pravidel pro výpočet stavu buňky v příští generaci:

  • Any live cell with fewer than two live neighbours dies, as if caused by under-population. (Under-population rule)
  • Any live cell with two or three live neighbours lives on to the next generation. (Survival rule)
  • Any live cell with more than three live neighbours dies, as if by over-population. (Over-population rule)
  • Any dead cell with exactly three live neighbours becomes a live cell, as if by reproduction. (Reproduction rule)

Výpočet je tedy funkcí aktuálního stavu buňky a počtu jejich živých sousedů. Pravidla nám trochu zatajují situace (Dead,x), kde x in {0,1,2,4,5,6,7,8}. U těch se předpokládá příští stav Dead. Bude pro nás srozumitelnější, když si pro tyto případy zavedeme další "defaultní" pravidlo No-reproduction rule.

Kód je sice v C#, ale to by pro vývojáře ze spřátelených platforem neměl být žádný problém. Jde tady přeci o společné principy čitelnosti, komplexity a OOP.

Testy až na prvním místě

Jak už to u Test-First přístupu bývá, testy nám pomohou lépe pochopit problém a pokrýt celou množinu možných situací. Tento článek ale není o TDD přístupu ani o návrhu struktury testovacích případů. Dovolím si proto zjednodušený zápis finální množiny testů do jednoho parametrického jednotkového testu:

public class NextGenerationStateCalculatorTests
{
    [TestCase(CellState.Live, 0, CellState.Dead, TestName = "Under-population rule: (Live,0) => Dead")]
    [TestCase(CellState.Live, 1, CellState.Dead, TestName = "Under-population rule: (Live,1) => Dead")]
    [TestCase(CellState.Live, 2, CellState.Live, TestName = "Survival rule: (Live,2) => Live")]
    [TestCase(CellState.Live, 3, CellState.Live, TestName = "Survival rule: (Live,3) => Live")]
    [TestCase(CellState.Live, 4, CellState.Dead, TestName = "Over-population rule: (Live,4) => Dead")]
    [TestCase(CellState.Live, 5, CellState.Dead, TestName = "Over-population rule: (Live,5) => Dead")]
    [TestCase(CellState.Live, 6, CellState.Dead, TestName = "Over-population rule: (Live,6) => Dead")]
    [TestCase(CellState.Live, 7, CellState.Dead, TestName = "Over-population rule: (Live,7) => Dead")]
    [TestCase(CellState.Live, 8, CellState.Dead, TestName = "Over-population rule: (Live,8) => Dead")]
    [TestCase(CellState.Dead, 3, CellState.Live, TestName = "Reproduction rule: (Dead,3) => Live")]
    [TestCase(CellState.Dead, 0, CellState.Dead, TestName = "No-reproduction rule: (Dead,0) => Dead")]
    [TestCase(CellState.Dead, 1, CellState.Dead, TestName = "No-reproduction rule: (Dead,1) => Dead")]
    [TestCase(CellState.Dead, 2, CellState.Dead, TestName = "No-reproduction rule: (Dead,2) => Dead")]
    [TestCase(CellState.Dead, 4, CellState.Dead, TestName = "No-reproduction rule: (Dead,4) => Dead")]
    [TestCase(CellState.Dead, 5, CellState.Dead, TestName = "No-reproduction rule: (Dead,5) => Dead")]
    [TestCase(CellState.Dead, 6, CellState.Dead, TestName = "No-reproduction rule: (Dead,6) => Dead")]
    [TestCase(CellState.Dead, 7, CellState.Dead, TestName = "No-reproduction rule: (Dead,7) => Dead")]
    [TestCase(CellState.Dead, 8, CellState.Dead, TestName = "No-reproduction rule: (Dead,8) => Dead")]
    public void Calculate__For_Given_CellState_And_Neighbours__Should_Return_Expected_CellState(CellState cellState, int numberOfLiveNeighbours, CellState expectedNextGenerationCellState)
    {
        var nextGenerationCellState = new NextGenerationStateCalculator().Calculate(cellState, numberOfLiveNeighbours);
 
        Assert.AreEqual(expectedNextGenerationCellState, nextGenerationCellState);
    }
}
 
public enum CellState
{
    Live,
    Dead
}

Ve výsledku tedy máme celkem 2 [součastný stav buňky - Live, Dead] x 9 [počet sousedů 0 .. 8] = 18 testů. Mohli bychom případně udělat rozpad struktury testů podle jednotlivých pravidel. Mohli bychom vylepšit pojmenování. A další. Tím se ale nechci zdržovat. Pojďme řešit implementaci.

A. První verze - ta "nejrychlejší"

Už si nepamatuju, jak jsem na svém prvním Coderetreatu pravidla implementoval já. Často ale vidím, že programátoři napíšou něco takového:

public class NextGenerationStateCalculatorA
{
    public CellState Calculate(CellState cellState, int numberOfLiveNeighbours)
    {
        if (cellState == CellState.Live && (numberOfLiveNeighbours == 2 || numberOfLiveNeighbours == 3)
            || (cellState == CellState.Dead && numberOfLiveNeighbours == 3)) return CellState.Live;
 
        return CellState.Dead;
    }
}

Je to výsledek přístupu "Žij nebo Zemři". Náš pohled na problém je omezen na to, že stačí určit, kdy bude buňka v příští generaci Live. Pro všechny ostatní případy je Dead.

Problémem tohoto přístupu je ignorování narůstající složitosti podmínky pro Live. Případné další pravidlo by při tomto přístupu podmínku ještě více znepřehlednilo. Implementace podmínky také nenese žádnou informaci o tom, jaká pravidla zahrnuje. Není zde žádné mapování kódu na požadavky. Cyklomatická složitost (Cyclomatic Complexity) této implementace je 6. Takže docela dost.

Kód je relativně krátký, ale je snadné jej pochopit a rozšířit? Zkusíme vylepšit expresivitu kódu ...

B. Pomůžeme si pojmenováním proměnných

public class NextGenerationStateCalculatorB
{
    public CellState Calculate(CellState cellState, int numberOfLiveNeighbours)
    {
        var useSurvivalRule = cellState == CellState.Live && (numberOfLiveNeighbours == 2 || numberOfLiveNeighbours == 3);
        var useReproductionRule = cellState == CellState.Dead && numberOfLiveNeighbours == 3;
 
        return useSurvivalRule || useReproductionRule ? CellState.Live : CellState.Dead;
    }
}

V kódu se nám už objevuje terminologie prvních pravidel. Konkrétně Survival a Reproduction. Zavedené lokální proměnné nám tak přidávají do kódu částečnou vazbu na naše zadání. Ostatní pravidla ale v kódu explicitně uvedena nejsou.

C. Strukturujeme kód podle jednotlivých pravidel

public class NextGenerationStateCalculatorC
{
    public CellState Calculate(CellState cellState, int numberOfLiveNeighbours)
    {
        if (cellState == CellState.Live && numberOfLiveNeighbours < 2) return CellState.Dead;
        if (cellState == CellState.Live && (numberOfLiveNeighbours == 2 || numberOfLiveNeighbours == 3)) return CellState.Live;
        if (cellState == CellState.Live && numberOfLiveNeighbours > 3) return CellState.Dead;
        if (cellState == CellState.Dead && numberOfLiveNeighbours == 3) return CellState.Live;
 
        return CellState.Dead;
    }
}

Toto řešení nám sice zvedlo LOC, ale jeho struktura už více kopíruje jednotlivá pravidla. Opět nám však chybí informace o názvu jednotlivých pravidel a CC nám skočila na 10.

D. Extrahujme podmínky pro jednotlivá pravidla

public class NextGenerationStateCalculatorD
{
    public CellState Calculate(CellState cellState, int numberOfLiveNeighbours)
    {
        if (UseUnderPopulationRule(cellState, numberOfLiveNeighbours)) return CellState.Dead;
        if (UseSurviveRule(cellState, numberOfLiveNeighbours)) return CellState.Live;
        if (UseOverPopulationRule(cellState, numberOfLiveNeighbours)) return CellState.Dead;
        if (UseReproductionRule(cellState, numberOfLiveNeighbours)) return CellState.Live;
 
        return CellState.Dead;
    }
 
    private bool UseUnderPopulationRule(CellState cellState, int numberOfLiveNeighbours)
    {
        return cellState == CellState.Live && numberOfLiveNeighbours < 2;
    }
 
    private bool UseSurviveRule(CellState cellState, int numberOfLiveNeighbours)
    {
        return cellState == CellState.Live && (numberOfLiveNeighbours == 2 || numberOfLiveNeighbours == 3);
    }
 
    private bool UseOverPopulationRule(CellState cellState, int numberOfLiveNeighbours)
    {
        return cellState == CellState.Live && numberOfLiveNeighbours > 3;
    }
 
    private bool UseReproductionRule(CellState cellState, int numberOfLiveNeighbours)
    {
        return cellState == CellState.Dead && numberOfLiveNeighbours == 3;
    }
}

Extrahovali jsme podmínky pravidel do pojmenovaných privátních metod. Čitelnost se tím trochu zlepšila a snížili jsme CC hlavní metody na 5. Pořád mi ale vadí, že podmínky a výsledný stav jednotlivých pravidel nejsou logicky lépe seskupeny. Takto se nám v kódu 4-krát opakuje stejný pattern:

if (XRule(cellState, numberOfLiveNeighbours)) return CellState.?;

E. Sbližujeme podmínky pravidel a výsledné stavy

delegate bool RulePredicate(CellState cellState, int numberOfLiveNeighbours);
 
public class NextGenerationStateCalculatorE
{
    public CellState Calculate(CellState cellState, int numberOfLiveNeighbours)
    {
        var rules = new Dictionary<RulePredicateCellState>
        {
            { UnderPopulationRulePredicate, CellState.Dead },
            { SurviveRulePredicate, CellState.Live },
            { OverPopulationRulePredicate, CellState.Dead },
            { ReproductionRulePredicate, CellState.Live },
            { NoReproductionRulePredicate, CellState.Dead }
        };
 
        var ruleKeyToApply = rules.Keys.First(rulePredicate => rulePredicate(cellState, numberOfLiveNeighbours));
        var nextGenerationState = rules[ruleKeyToApply];
 
        return nextGenerationState;
    }
 
    private bool UnderPopulationRulePredicate(CellState cellState, int numberOfLiveNeighbours)
    {
        return cellState == CellState.Live && numberOfLiveNeighbours < 2;
    }
 
    private bool SurviveRulePredicate(CellState cellState, int numberOfLiveNeighbours)
    {
        return cellState == CellState.Live && (numberOfLiveNeighbours == 2 || numberOfLiveNeighbours == 3);
    }
 
    private bool OverPopulationRulePredicate(CellState cellState, int numberOfLiveNeighbours)
    {
        return cellState == CellState.Live && numberOfLiveNeighbours > 3;
    }
 
    private bool ReproductionRulePredicate(CellState cellState, int numberOfLiveNeighbours)
    {
        return cellState == CellState.Dead && numberOfLiveNeighbours == 3;
    }
 
    private bool NoReproductionRulePredicate(CellState cellState, int numberOfLiveNeighbours)
    {
        return true;
    }
}

Zadefinovali jsme delegát RulePredicate, který popisuje, jak mají vypadat metody vyhodnocující podmínku pro pravidlo. Dictionary rules mapuje tyto podmínky na výsledné stavy. Máme tak poměrně hezké přiřazení pojmenované podmínky a výsledného stavu pro jednotlivá pravidla. Za tento "funkcionální přístup" jsme odměněni hezkou jedničkou z CC.

Moc se mi ale nelíbí způsob procházení obecného Dictionary, kdy odkazujeme Keys abychom přistoupili k predikátům jednotlivých pravidel. Abstrakce pravidla stále není moc dobrá.

F. Tak dobře, zapouzdříme pravidla

public class NextGenerationStateCalculatorF
{
    private readonly RuleBase[] _rules;
 
    public NextGenerationStateCalculatorF()
    {
        _rules = new RuleBase[]
        {
            new UnderPopulationRule(),
            new SurvivalRule(),
            new OverPopulationRule(),
            new ReproductionRule(),
            new NoReproductionRule()
        };
    }
 
    public CellState Calculate(CellState cellState, int numberOfLiveNeighbours)
    {
        var ruleToApply = _rules.First(rule => rule.Predicate(cellState, numberOfLiveNeighbours));
        var nextGenerationState = ruleToApply.NextGenerationCellState;
 
        return nextGenerationState;
    }
 
    private abstract class RuleBase
    {
        public RulePredicate Predicate { get; }
        public CellState NextGenerationCellState { get; }
 
        protected RuleBase(RulePredicate predicate, CellState nextGenerationCellState)
        {
            Predicate = predicate;
            NextGenerationCellState = nextGenerationCellState;
        }
    }
 
    private class UnderPopulationRule : RuleBase
    {
        public UnderPopulationRule()
            : base((state, neighbours) => state == CellState.Live && neighbours < 2, CellState.Dead)
        { }
    }
 
    private class SurvivalRule : RuleBase
    {
        public SurvivalRule()
            : base((state, neighbours) => state == CellState.Live && (neighbours == 2 || neighbours == 3), CellState.Live)
        { }
    }
 
    private class OverPopulationRule : RuleBase
    {
        public OverPopulationRule()
            : base((state, neighbours) => state == CellState.Live && neighbours > 3, CellState.Dead)
        { }
    }
 
    private class ReproductionRule : RuleBase
    {
        public ReproductionRule()
            : base((state, neighbours) => state == CellState.Dead && neighbours == 3, CellState.Live)
        { }
    }
 
    private class NoReproductionRule : RuleBase
    {
        public NoReproductionRule()
            : base((state, neighbours) => trueCellState.Dead)
        { }
    }
}

Hlavní metoda prohledá pole pravidel inicializovaných v konstruktoru třídy. Vybraného pravidla se pak zeptá na příští stav. CC = 1. Při implementaci jednotlivých pravidel jsme si pomohli bázovou třídou RuleBase. Implementace konkrétního pravidla se nám pak omezí pouze na předání predikátu a nového stavu přes volání bázového konstruktoru.

Každé pravidlo je nyní reprezentováno dobře pojmenovanou třídou. Hlavní třída pouze definuje pořadí těchto pravidel a umí se doptat na výsledný stav buňky v příští generaci. Celková komplexita problému je rozložena do více tříd s nízkou komplexitou. Přestože je kód delší, můžeme jej stále považovat za jednoduchý.

Jak tedy psát ten kód?

Možná si říkáte, kam jsme se to v tom vylepšování až dostali. Proč tak složitě na tak jednoduchý problém? Proč vytvářet tolik tříd?

Je to trénink. Test toho, jak vnímáme komplexitu kódu a které charakteristiky nás vlastně zajímají. Reálné problémy jsou samozřejmě složitější a správná redukce komplexity přinese výraznější benefity. A příjemnější práci pro programátory.

Přemýšlejme nad každým větvením programu, zda-li se náhodou nejedná o signál pro použití polymorfismu a separaci na úrovni tříd. Nízká CC by měla být našim základním požadavkem.

Preferujme, aby v našem kódu byla snadno dohledatelná struktura reálného problému, tedy našich (správně formulovaných) požadavků.

Nejrychlejší / nejkratší řešení často nebývá nejlepší. Zvažujme různé možnosti a přístupy. Psát "složitý kód" bývá jednoduché, zato psát "jednoduchý kód" často složité.

Strach, lenost nebo neschopnost problém dekomponovat do více jednodušších tříd často vede k hromadám špaget. Nic proti špagetám, mohl bych je mít pořád. Ale v kódu se bez nich rád obejdu! :)

2 komentáře:

  1. Ahoj,

    co říkáš na další zamyšlení nad kódem? Jednotlivé implementace `RuleBase` nijak nespecifikují jeho chování, jde podle mě o zneužití dědičnosti. Přijde mi, že potřebujeme jen pravidlo konfigurovat, ale neměnit jeho chování (ponechme stranou, že žádné pořádné chování nemá).

    Co kdyby existovala `Rule`, která bude mít stejnou funkcionalitu jako `RuleBase`, jen nebude abstraktní? Tím by šla sama o sobě i otestovat.

    Jednotlivá pravidla pak může tvořit továrna. Kód nebude tratit na přehlednosti, nebudeme zneužívat dědičnost a snížíme počet tříd.

    Výsledek by mohl vypadat cca takto:

    _rules = new RuleBase[]
    {
    ruleFactory.createUnderPopulationRule(),
    ruleFactory.createSurvivalRule(),
    ...
    }

    class RuleFactory {

    public createUnderPopulationRule(): Rule {
    return new Rule((state, neighbours) => state == CellState.Dead && neighbours == 3, CellState.Live);
    }

    ...
    }

    Svaťa

    OdpovědětVymazat
    Odpovědi
    1. Ahoj Svaťo,

      díky za komentář. Fajn řešení. Měl bych k němu pár připomínek na zamyšlení:
      - Zdá se mi, že factory toho ví příliš hodně. Obsahuje totiž definici všech pravidel. Možná tak porušuje SRP ze SOLID.
      - OCP ze SOLIDu nám říká, že každá případná změna by měla mít co nejmenší dopad na stávající implementaci. Měla by ji ideálně pouze rozšiřovat. Tento princip se často zajišťuje právě separací do samostatných tříd. Přidat nové pravidlo ve tvém případě znamená rozšířit rozhranní RuleFactory o metodu createNewRule() a naimplementovat ji. Při mém řešení F by vznikla nová třída NewRule a NextGenerationStateCalculatorF by jen přidal instanci této třídy do _rules.

      Pokud by pravidla v F byla definována jako disjunktní (museli bychom upravit implementaci NoReproductionRule), pak by na pořadí nezáleželo a mohli bychom využít IoC container a dependency injection:

      public NextGenerationStateCalculatorF(IEnumerable rules)
      {
      _rules = rules;
      }

      V takovém případě bychom změny při přidání nového pravidla provedli jenom v příslušných XRule třídách. A v NextGenerationStateCalculatorF by k žádné změně nedošlo.

      Vymazat