Co je špatně?
Duplicitní kód se sice rychle a snadno generuje (podobně jako diplomky politiků), ale později se ve zlém vrátí tím spíš, čím víc "klonů" existuje. Scénář katastrofy je obvykle rutinní:- Uživatel nahlásí chybu nebo je třeba implementovat novou vlastnost
- Chyba se opraví - čili klon se liší od svých pravzorů
- Jiný uživatel reklamuje chybu v jiném klonu
- Chyba se opraví - obvykle jinak než v prvním klonu.
- Ostatní klony jsou neopravené. Trpí stejným problémem? Netrpí jím? Jak to, že fungují správně? Je část klonovaného kódu v jejich případě zbytečná? Nebo nefungují správně, jen ještě nikdo v jejich případě chybu nereklamoval?
- Rozdíly narůstají, chyby někde mizí, jinde zůstávají nebo se dokonce mění. Technický dluh roste exponenciálně.
Konfigurace CPD, Copy And Paste Detector
CPD je součástí PMD maven pluginu, čili když už, nejspíš budete používat oba. Kontrolám PMD samotného se ale vyhnu, to si najděte jinde ;)
CPD porovnává soubory a hledá v nich opakující se sekvence "tokenů". Algoritmus lze trochu konfigurovat, viz dokumentace pluginu.
<plugin>
<artifactId>maven-pmd-plugin</artifactId>
<version>3.11.0</version>
<executions>
<execution>
<id>cpd</id>
<goals>
<goal>cpd</goal>
</goals>
<phase>verify</phase>
<configuration>
<skip>${cpd.skip}</skip>
<minimumTokens>100</minimumTokens>
<excludes>
<exclude>**/en/**/*</exclude>
<exclude>**/generated/**/*</exclude>
</excludes>
</configuration>
</execution>
<execution>
<id>cpd-check</id>
<goals>
<goal>cpd-check</goal>
</goals>
<phase>verify</phase>
<configuration>
<skip>${cpd.skip}</skip>
<printFailingErrors>true</printFailingErrors>
<verbose>true</verbose>
</configuration>
</execution>
</executions>
Pokud plugin najde dvě stejné sekvence delší jak 100 tokenů, shodí build a vypíše nalezené duplicity. Ty pak budete muset řešit.
Řešení
Fuj: Přehodím řádky
Ano, z pohledu CPD jste z toho venku, ale po pravdě ... vážně myslíte, že jste se problému zbavili? Ve skutečnosti jste se zbavili jen jeho hlášení, problém trvá. Navíc jste si ještě zhoršili orientaci ve třídě.Fuj: Zvýším minimumTokens
... a zbavím se hlášení. Problém trvá, ale co hůř, ono to často ani nezabere, protože těch duplicit je víc, byť i jen na málo řádcích. Pointa je právě v tom, co vlastně je ten "token". Ta stovka je docela rozumná, proto je taky jako default.Exkluze
Ta funguje stoprocentně. Není ovšem určená k ignorování skutečných problémů, nýbrž k ignorování falešných hlášení problémů. Může být nalezená duplicita, skutečná duplicita, falešným hlášením? Je třeba si uvědomit, že cílem není zbavit se jakýchkoliv duplicit, ale zbavit se hrozících problémů s duplicitami, zlepšit architekturu, uklidit.Do exkluze tudíž typicky dávám jednak kód generovaný, se kterým nic nenadělám, jednak třeba JPA entity, které mohou mít velmi podobné vlastnosti, přestože vzájemně nijak nesouvisí. To, že se něco stejně jmenuje, nemusí znamenat, že je to duplicitní - a CPD s oblibou hlásí stejné čtyři settery a gettery, jdoucí za sebou.
Problém může být, pokud nemáte žádnou konvenci, jak třeba JPA entity odlišit od jiných tříd. Hodně štěstí ;-)
Vyvedení do samostatné metody ve stejné třídě
Samozřejmost, pokud je duplicitní kód jediné třídy. Nebo ne? Pokud na 10 řádkách máme 8 lokálních proměnných, tohle nebude smysluplná cesta. Můžeme je zkombinovat do jednoho výpočtu, jenže ... to zásadně zhorší čitelnost, takže taky ne. Takže jiná možnost.Vyvedení do společného rodiče
Je to možnost, ale je třeba zvážit, zda vyváděná funkcionalita do rodiče opravdu patří. Pokud ostatním potomkům rodiče je tato funkcionalita cizí a nemají s ní nic společného, nedělejte to. Jedině že byste stvořili dalšího "mezirodiče", ale pokud mu nedokážete dát smysluplný název, budu se opakovat - nedělejte to; příliš vysoká hierarchie rodičů a potomků je další problém, čili je tu riziko, že odstraněním jednoho problému vytvoříte jiný.Vyvedení do "utility" nebo "cizí" třídy
Utilitky, helpery, atp., jsou víceméně neobjektová věc, na druhou stranu netrpí problémy s dědičností, dobře se na ně píšou testy, takže tuhle možnost vůbec nezatracujte.Podobně je možné, že víte o třídě, kam tento kód vlastně i logicky patří a lze ho odsud sdílet na potřebná místa. Předpokladem je, že vyvedenému kódu dokážete vytvořit nějaké pěkné smysluplné API. Ale to se vlastně týká každého "vyvádění".
Vyvedení do default metody interface
Óóó, jak snadné a efektivní!Jenže to má háček - nese to riziko. Implementace z abstraktního rodiče, implementující stejnou metodu, má vyšší prioritu než jakýkoliv interface!!!
Vážně si rozmyslete, jestli vyváděný kód může opravdu sloužit jen jako pouhý "default". V případě kolizí pak musíte metodu stejně přetížit a z těla volat Iface.super.metoda(), jinak řečeno musíte říct, kterou z dostupných implementací chcete používat.
Pokud máte ale v rodiči metodu implementovanou, complier vám nic neřekne a neporadí, předpokládá, že to vážně chcete.
Silver bullet: přečtěte si to
Překvápko: někdy není třeba duplicitní blok "vyvádět", stačí oba duplicitní bloky naprosto stejně zjednodušit. Třeba odstranit 10 opakovaných přetypování. Nebo jen něco, co se na obou místech otravně opakuje, vyvést ven a sdílet. Výsledek? Ubyde tokenů, jak prosté, už to nevypadá jako hloupý copy and paste, jen něco, co dělá stejnou věc, a to už není známka copy and paste.Svět je zase v pořádku :-)
Žádné komentáře:
Okomentovat