Pred pár týždňami sme s kolegami rozširovali starší kód v Jave o ďalšiu funkcionalitu. Vyzeralo to na jednoduchú úlohu…

Po počiatočnom zhliadnutí bolo ale jasné, že ďalšie rozširovanie bez refaktoringu, zlepšenia čitateľnosti a optimalizácie kódu nebude možné. Našťastie existovali unit testy, a tak sme sa mohli pustiť do zefektívnenia existujúceho kódu.

Pri refaktoringu je vždy dôležité zachovať pomer medzi časovou náročnosťou úprav a pridanou hodnotou, ktorú nám zmeny prinesú. Detailnejšie úpravy a vylepšenia môžu prísť na rad pri veľkých zmenách. Úpravy nám zlepšia možnosti debugovania, pomôžu skrátiť dobu implementácie a zjednodušia aj úpravy unit testov. Ukážkové kódy sú inšpirované tými, ktoré sme refaktorovali.

Kedy kód refaktorovat?

Indikátorom či kód je potrebné zrefaktorovať sú situácie, kedy sa v kóde prestávame orientovať. Najčastejšie problémy a ich riešenia uvádzam v tabuľke.

Problém Riešenie
Veľký počet riadkov v triede (viac ako 1000). Podozrenie, že trieda zlučuje funkcionalitu, ktorá nemá byť spolu. Napríklad zlúčenie DAO objektov a DTO objektov.
Veľký počet riadkov v metóde (viac ako 150) Rozdelenie metódy na menšie časti/viaceré metódy.
Verejné atribúty triedy Zmeniť na privátne a vytvoriť get/set metódy atribútom.
Duplicita kódu Vytvorenie samostatnej parametrizovanej metódy pre kód, pri duplicite kódu v príbuzných triedach spoločný kód presunúť do vyššej triedy (predok daných tried), alebo do ich abstraktnej triedy.
Pozri bod č.4.
Programovanie voči konkrétnym triedam, typ návratových hodnôt nie je interface triedy Programovanie voči interfacom s generickou kontrolou typov. Pozri bod č.3.
Ošetrovanie výnimiek (použitie printStackTrace, System.out.println, Throwable). Použitie štandardného logovania napr. Log4J.
Namiesto Throwable používať konkrétne výnimky.
Výnimky(Exception) a chyby (Error) spracovávať osobitne.
Vo všeobecnosti veľký počet „if“ a „switch“ príkazov Využitie dedičnosti a špecializácie tried.
Absencia/nedostatočný popis metód a tried, veľa komentárov v kóde Doplnenie a automatické generovanie popisov.
Komentáre v kóde redukovať a nechať len pri zložitých biznis procesoch.
Príliš veľa komentárov môže signalizovať zlú, alebo komplikovanú implementáciu.
Pozri bod č.5.
Java warnings – upozorňuje na možné problémové miesta Opraviť problémové miesta.
Rôzne formáty kódu, pomenovanie konštánt, premenných,… Zjednotiť formát kódu podľa predpripravených šablón, ktoré si môžeme upraviť (Eclipse). Zjednotenie pomenovania.
Pozri bod č.1.
Konštanty definované priamo v kóde Nahradenie konštánt definovaných v kóde konštantami definovaných v triede prípadne nahradenie enumeráciou.
Pozri bod č.2.

V nasledovných bodoch by som ukázal najčastejšie refaktorovacie metódy, ktoré nám pomohli sprehľadniť a zefektívniť existujúci kód.

1. Pomenovania premenných, konštánt, metód a tried

V nasledujúcom príklade ukážem nevhodné definovanie a pomenovávanie. Týmto chybám sa treba vyhnúť už pri počiatočnom písaní kódu.

Po zhliadnutí kódu je viditeľné, že pre ďalšiu manipuláciu a rozširovanie sú potrebné rýchle a jednoduché úpravy.

public class Address {

  public String street="";
  public String psc="";
  public String city="";
  public String cisloDomu="";

  public void naplnAdresu(String inStreet, String inPsc, String inCisloDomu, String inCity) {
    street = inStreet;
    psc=inPsc;
    cisloDomu=inCisloDomu;
    city=inCity;
  }
...
}

Viditeľné je premiešanie rôznojazyčných názvov premenných, nevhodná inicializácia premenných, viditeľnosť premenných mimo triedu, pristupovanie priamo k premenným mimo get/set metód a mnohé ďalšie.
Pri jazyku je dobré dohodnúť sa vopred aký sa bude používať na celom projekte. V tomto prípade to bola slovenčina.

Tu je časť triedy po refaktoringu.

public class Adresa {

  private String ulica;
  private String cisloDomu;
  private String mesto;
  private String psc;

  public void Adresa(String ulica, String cisloDomu, String mesto, String psc) {
    this.ulica = ulica;
    this.psc=psc;
    this.cisloDomu=cisloDomu;
    this.mesto=mesto;
  }
...
}

Pri menách metód sa používajú obdobné pravidlá ako pri atribútoch a premenných. Meno má byť krátke, výstižné (bližšie detaily zistíme v popise metódy), používať jednotný jazyk a metodiku pomenovania. V angličtine sú to napríklad prefixy get/set/is/has. Pokiaľ je meno metódy príliš dlhé, je možné, že metóda zastrešuje rôznu funkcionalitu a je indikátorom rozdelenia metódy na viaceré časti. Napríklad „vypocitajAZobrazVysledkyistiky”.

2. Extraktovanie, konštanty a enumerácia

Na rýchle sprehľadnenie kódu, identifikácie využitia pevne definovaných konštánt v celom našom kóde, ich ďalších zmien je vhodné tieto konštanty nahradiť globálnymi konštantami alebo enumeráciou.

public class Attachment extends AbstractSQLObject {
...
  public static ArrayList getAttachments(final int gfNr) throws AdapterException {
    ArrayList attachmentList = new ArrayList();
    ResultSet rs = null;
    Statement stmt = null;
    try {
      String sql = "select GFNR, FILEDATA, VERSION, DESCRIPTION, FILENAME, FILETYPE from " + getTableNameString()+" where GFNR=" + gfNr;
      stmt = DBConnectionFactory.getInstance().getSQLStatment(sql);
    
      rs = stmt.getResultSet();

      while (rs.next()) {
        Attachment attachment = new Attachment();

        attachment.setVersion(rs.getInt("VERSION"));

        if (0 == attachment.getVersion()) {
          attachment.setFileData(Base64.decode(new String(rs.getBytes("FILEDATA"))));
        } else {
          attachment.setFileData(rs.getBytes("FILEDATA"));
        }

        attachment.setDescription(rs.getString("DESCRIPTION"));
        attachment.setFileName(rs.getString("FILENAME"));
        attachment.setFileType(rs.getString("FILETYPE"));
        attachment.setGfnr(rs.getString("GFNR"));
        attachmentList.add(attachment);
      }
    } catch (Exception e) {
      throw new AdapterException("Error while reading SQL-Object!", e);
    } finally {
      Tools.closeResultSet(rs, stmt);
    }
    return attachmentList;
  }

Pri rozširovaní objektu prílohy(Attachment) sme nahradili mená stĺpcov v databáze globálnymi konštantami a podobne sme zadefinovali aj verzie prílohy.
Označenia verzií mohli byť definované aj enumerátorom, avšak nakoľko boli použité len v rámci tejto triedy, nechali sme ich ako konštanty. Ďalší dôvod bol, že verzie prílohy boli len dve a síce 0 a 1.

Po úprave:

public class Attachment extends AbstractSQLObject {
  private static final String FILEDATA = "FILEDATA";
  private static final String VERSION = "VERSION";
  private static final String GFNR = "GFNR";
  private static final String FILETYPE = "FILETYPE";
  private static final String FILENAME = "FILENAME";
  private static final String DESCRIPTION = "DESCRIPTION";

  // data ulozene bez BASE64
  private static final int VERSION_1 = 1;
  // data ulozene v BASE64 = povodny typ
  private static final int VERSION_0_BASE64 = 0;
...
  public static ArrayList getAttachments(final int gfNr) throws AdapterException {
    ...
    String sql = "select "+GFNR+", "+FILEDATA+", "+VERSION+", "+DESCRIPTION+", "+FILENAME+", "+FILETYPE+" from " + getTableNameString()+" where "+GFNR+"=" + gfNr;
    stmt = DBConnectionFactory.getInstance().getSQLStatment(sql);

    rs = stmt.getResultSet();

    while (rs.next()) {
      Attachment attachment = new Attachment();

      attachment.setVersion(rs.getInt(VERSION));

      if (VERSION_0_BASE64 == attachment.getVersion()) {
        attachment.setFileData(Base64.decode(new String(rs.getBytes(FILEDATA))));
      } else {
        attachment.setFileData(rs.getBytes(FILEDATA));
      }

      attachment.setDescription(rs.getString(DESCRIPTION));
      attachment.setFileName(rs.getString(FILENAME));
      attachment.setFileType(rs.getString(FILETYPE));
      attachment.setGfnr(rs.getString(GFNR));
      attachmentList.add(attachment);
...

Pri ďalších iteráciách úprav triedy boli vykonané zmeny na PrepareStatement-y, generovanie SQL kódu a mnohé ďalšie. Typ súboru bol prerobený na enumeráciu:

public enum AttachmentType {
  JPG     ("image/jpeg",               "jpg"),
  JPG_MS  ("image/pjpeg",              "jpg"), 
  JPEG    ("image/jpeg",               "jpeg"),
  JPEG_MS ("image/pjpeg",              "jpeg"),
  PDF     ("application/pdf",          "pdf"),
  RTF     ("text/richtext",            "rtf"),
  TIF     ("image/tiff",               "tif"),
  TIFF    ("image/tiff",               "tiff");

  public String contentMimeType;
  public String fileExtension;

  AttachmentType(String contentMimeType, String fileExtension) {
    this.contentMimeType = contentMimeType;
    this.fileExtension = fileExtension;
  }

  public static AttachmentType getType(String contentMimeType) {
    if (!StringTools.isNullOrEmpty(contentMimeType)) {
      for (AttachmentType attType : AttachmentType.values()) {
        if (attType.contentMimeType.equals(contentMimeType.toLowerCase())) {
          return attType;
        }
      }
    }
  ...

3. Použitie interface-u namiesto konkrétnej implementácie

Pri predchádzajúcom príklade implementácie metódy getAttachments je návratová hodnota typu ArrayList. Definovanie konkrétneho typu návratovej hodnoty zamedzuje ľahkej zmene a úpravám kódu.
V budúcnosti ak budeme potrebovať zmeniť na iný typ zoznamu, napríklad synchronizovaný, alebo úplne inú implementáciu zoznamu, musíme následne zmeniť typ v celom našom kóde.
Riešením je definovanie typu návratovej hodnoty jeho interfacom. V našom prípade List-om.

Pôvodný zdrojový kód
public static ArrayList getAttachments(...

predefinujeme na
public static List getAttachments(...

Nakoľko vieme, že naša metóda vracia len objekty typu Attachment, pridáme ešte typovú kontrolu.
public static List getAttachments(...
a upravíme aj vytváranie zoznamu
List attachmentList = ...

Programovanie na interface sa široko využíva, napríklad pri Factory triedach, alebo pri využití Spring frameworku.

4. Zamedzenie duplicity

Údržba a zmeny kódu sú náročné na čas, preto minimalizácia duplicity je nanajvýš dôležitá. Medzi ďalšie výhody treba uviesť, že akékoľvek zmeny sa vykonajú len na jednom mieste, minimalizuje sa tým riziko zabudnutia vykonania zmien na všetkých miestach. Menej kódu = menej práce s údržbou.

public class Zamestnanec {
...
  public void zobrazMzdu () {
  ...
    System.out.println("Hruba mzda:" + pocetDni * dennaMzda);
    System.out.println("Rocna hruba mzda:" + (pocetDni * dennaMzda * 12));
  }
}

Po odstránení duplicity:

double hrubaMzda = pocetDni * dennaMzda;
System.out.println("Hruba mzda:" + hrubaMzda);
System.out.println("Rocna hruba mzda:" + (hrubaMzda * 12));

Daná implementácia pokrýva prípad výpočtu mzdy pre interného zamestnanca. Avšak časom príde stav, že potrebujeme rátať mzdu aj externým pracovníkom.

public class ExternyZamestnanec {
...
  public void zobrazMzdu () {
    ...
    double hrubaMzda = pocetHodinZaMesiac * hodinovaSadza;
    System.out.println("Hruba mzda:" + hrubaMzda);
    System.out.println("Rocna hruba mzda:" + (hrubaMzda * 12));
  }
}

Rozdiel v oboch prípadoch je len vo výpočte mzdy, avšak zobrazovanie ostáva rovnaké. Extraktujeme časť zodpovednú za výpočet a vytvoríme novú metódu vypocetHrubaMzda. Triedu ExternyZamestnanec odvodíme od triedy Zamestnanec a prepíšeme výpočet mzdy a doplníme aj atribúty pre tento výpočet.

public class Zamestnanec {
...
  public void zobrazMzdu () {
    System.out.println("Hruba mzda:" + vypocetHrubaMzda());
    System.out.println("Rocna hruba mzda:" + (vypocetHrubaMzda() * 12));
  }

  protected double vypocetHrubaMzda() {
    return pocetDni * dennaMzda;
  }
}

public class ExternyZamestnanec extends Zamestnanec {
...
  @Override
  protected double vypocetHrubaMzda() {
    return pocetHodinZaMesiac * hodinovaSadza;
  }
}

V tomto bode by sme mali minimalizovanú duplicitu. Pri bežnom refaktoringu kedy nie dostatok času by to postačovalo.
Pre úplný refaktoring by som vytvoril podľa bodu č.3. interface IZamestnanec, ktorý by obsahoval všetky potrebné metódy.
Ďalej by nasledovalo vytvorenie abstraktnej triedy AbstractZamestnanec implementujúci interface IZamestnanec a tu by boli následne implementované spoločné metódy pre všetky triedy zamestnancov. V našom prípade pre interných a externých zamestnancov. Nové triedy ZamestnanecExterny a ZamestnanecInterny by boli rozšírením abstraktnej triedy s vlastnou implementáciou výpočtu hrubej mzdy a vlastnými ďalšími metódami a premennými.

Na zistenie duplicity, ale aj ďalších problémov ako prázdne try-catch bloky, nepoužité metódy, premenné a identifikovanie ďalších problémov existujú rôzne programy. Niektoré open-sourcové riešenia s popisom možno nájsť na
http://java-source.net/open-source/code-analyzers

5. Komentáre a popis

Popisovanie by však malo byť súčasťou kódu. Stačí jednoducho a krátko popísať čo daná metóda/trieda/interface/… robí. Minimálne verejné metódy triedy by mali byť popísané. V samotnom kóde by komentáre byť nemali. Kód by mal byť čitateľný a pochopiteľný aj bez neho. Pokiaľ sa však takýto komentár v kóde nachádza, tak je to skôr znak toho, že sme niečo nie vhodne implementovali.

Ukážka komentára metódy.

/**
* Return number format for export.
* 
* @return NumberFormat
* @return NullPointerException
* @author Peter Hanuliak (phanuliak)
* @date 06.03.2010
*/
public NumberFormat getNumberFormat() throws NullPointerException {
...

Popis by mal minimálne obsahovať popísanie činnosti, typ návratovej hodnoty, výnimky. Pokiaľ z komentárov generujeme dokumentáciu, môžeme navyše uviesť aj meno autora a dátum ako je uvedené v tomto príklade.

Tip: V prostredí Eclipse sa dá nastaviť podľa potreby automatické generovanie popisov.

Zhrnutie

S postupnými úpravami dokážeme aj z neprehľadného a zle napísaného kódu vypracovať čitateľný a použiteľný kód. Vývojové prostredia ponúkajú podporu na zjednodušenie týchto činností, a tak by postupný refaktoring mal byť nielen súčasťou úprav a opráv starého kódu, ale aj písania nového.

6 Příspěvků v diskuzi

  1. super clanok, jedna chybka:
    ….
    public static List getAttachments(…

    Nakoľko vieme, že naša metóda vracia len objekty typu Attachment, pridáme ešte typovú kontrolu.
    public static List getAttachments(…

    teda podla mna tam chyba

    termi

  2. hmmm, neviem preco, ale pokazilo to vlozeny komentar, takze by tam podla mna malo byt:

    public static List getAttachments(…

  3. Super clanok, osvetovy, len tak dalej.

    Nezaskodilo by pridat aj link na pouzitu literaturu – Tipujem Fowler a Martin

  4. balki:
    spominanu knihu od Martina Fowlera som davnejsie cital. Cielom clanku bolo popisat prakticke skusenosti a body, ktorych sa drzime pri refaktoringu.
    Odporucena literatura:
    Martin Fowler – Refactoring: Improving the Design of Existing Code
    Robert C. Martin – Clean Code: A Handbook of Agile Software Craftsmanship

Odpovědět