www.rozumim.cz

Code smell: výpočet opačné hodnoty

Code smell je konstrukce v kódu, ve které se skvěle daří chybám.

Na jednu takovou jsem zrovna v pátek narazil a samozřejmě i s tou chybou. Nazval jsem si ji pro sebe výpočet opačné hodnoty. Její součástí je duplicate code, jen se zde neprovádí operace shodné, ale opačné.

To je asi i příčina, proč nebylo nikomu divné, že o pár řádek níže vznikl znovu v podstatě stejný kód. Vždyť se na tom druhém místě přece dělá se zjištěnými hodnotami pravý opak!

Příklad

Volání EditaceDokumentu.ulozDokumenty() má uložit dokumenty v určitém pořadí. Nejdříve jeden druh dokumentů, potom druhý druh a nakonec všechny ostatní (už bez ohledu na druh).

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
public class EditaceDokumentu {
  
  private Set<Dokument> dokumenty;

  // další fieldy a metody
  // (...)
  
  public void ulozDokumenty() {
    
    ulozDokumenty(dokumenty, urciDruhUkladanyJakoPrvni());
    
    ulozDokumenty(dokumenty, urciDruhUkladanyJakoDruhy());
    
    ulozOstatniDokumenty(dokumenty);
  }
  
  private DokumentDruh urciDruhUkladanyJakoPrvni() {
    
    // rozhodne, jaký druh *je* ukládaný jako první
    // (...)
  }
  
  private DokumentDruh urciDruhUkladanyJakoDruhy() {
    
    // rozhodne, jaký druh *je* ukládaný jako druhý
    // (...)
  }
  
  private void ulozDokumenty(Set<Dokument> dokumenty, DokumentDruh druh) {

    // uloží všechny dokumenty z kolekce 'dokumenty', které jsou druhu 'druh'
    // (...)
  }
  
  private void ulozOstatniDokumenty(Set<Dokument> dokumenty) {
    
    // uloží všechny dokumenty z kolekce 'dokumenty', u kazdého nejdříve
    // zkontroluje zda *není* prvním nebo druhým již uloženým druhem a to
    // pomocí *vlastní*, duplicitní, logiky
    
    // zde byla samozřejmě chyba a jeden druh dokumentů se díky ní 
    // za určitých podmínek neuložil vůbec
    
    // (...)
  }
}

V kódu se nejdříve relativně složitým způsobem určí první druh ze všech možných (EditaceDokumentu.urciDruhUkladanyJakoPrvni()), potom druhý (EditaceDokumentu.urciDruhUkladanyJakoDruhy()) a poté se, znovu složitě, určují tyto již uložené druhy (EditaceDokumentu.ulozOstatniDokumenty()), aby se neuložily podruhé. A tam je ideální místo pro chybu.

Opravená vezre

Dva nejdříve zjištěné druhy dokumentů znovu použiji při ukládání ostatních dokumentů.

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
public class EditaceDokumentu {
  
  private Set<Dokument> dokumenty;

  // další fieldy a metody
  // (...)
  
  public void ulozDokumenty() {
  
    DokumentDruh prvniDruh = urciDruhUkladanyJakoPrvni();
    ulozDokumenty(dokumenty, prvniDruh);
    
    DokumentDruh druhyDruh = urciDruhUkladanyJakoDruhy()
    ulozDokumenty(dokumenty, druhyDruh);
    
    ulozOstatniDokumenty(dokumenty, Sets.newHashSet(prvniDruh, druhyDruh));
  }
  
  private DokumentDruh urciDruhUkladanyJakoPrvni() {
    
    // stejné
    // (...)
  }
  
  private DokumentDruh urciDruhUkladanyJakoDruhy() {
    
    // stejné
    // (...)
  }
  
  private void ulozDokumenty(Set<Dokument> dokumenty, DokumentDruh druh) {

    // stejné
    // (...)
  }
  
  private void ulozOstatniDokumenty(Set<Dokument> dokumenty, 
                                    Set<DokumentDruh> kromeDruhu) {
    
    // uloží všechny dokumenty z kolekce 'dokumenty', vynechá ty, které mají
    // druh z kolekce 'kromeDruhu'
    // (...)
  }
}

Závěr

Až budete příště potřebovat určit opačnou (negativní) hodnotu, zamyslete se, zda by nešla prostě použít ta již známá (pozitivní).

15. 12. 2013, kategorie: it
comments powered by Disqus