26/08/2009

Przykład tego jak nie należy pisać kodu

Home

Czasami jak patrzę na niektóre kody to zastanawiam się o czym myślał piszący je programista. Ostatnio natknąłem się na kod mniej więcej jak poniżej (zmieniłem nazwy zmiennych, metod itd.). Pomińmy jaka jest jego logika i gdzie został użyty. Proponuję natomiast przyjrzeć się przez chwilę temu fragmentowi i spróbować odpowiedzieć na pytanie co jest nie tak.
...
for (int i = 0; i < values.Length; i++)
{
  if (values[i] != null)
  {
    if (hash[values[i]] != null)
      Process(hash[values[i]]);

    if (hash[values[i]] != null)
    { 
      Process(values[i]);
      Process(values[i], hash[values[i]]);
      hash.Remove(values[i]);
    }
  }
}
...
Mnie uderzyła duża i zupełnie zbyteczna liczba odwołań do zmiennych values (tablica) i hash (Hashtable). Odwołanie do tej pierwszej występuje 8 razy, a do drugiej 5 razy! Po chwili pracy i wprowadzeniu bardzo prostych poprawek kod wygląda tak:
...
for (int i = 0; i < values.Length; i++)
{
  string val = values[i];
  if (val != null)
  {
    object obj = hash[val];
    if (obj != null)
      Process(obj);

    if (obj != null)
    {
      Process(val);
      Process(val, obj);
      hash.Remove(obj);
    }
  }
}
...
Odwołanie do zmiennych values oraz hash występuje tylko 1 raz. Czemu o tym piszę? Odpowiedź jest tak prosta jak wprowadzone poprawki: wydajność. Dodam jescze, że kod ten można jeszcze ulepszyć. W tym poście skupiam się jednak na tej jednej rzeczy.

W tym przypadku poprawiona wersja działa jakieś 3x/4x razy szybciej! Przyznam jednak, że całościowy efekt tej optymalizacji jest niezauważalny z dwóch powodów. Po pierwsze długość używanej w kodzie tablicy i liczba elementów w tablicy hashującej jest generalnie niewielka. Przeważnie nie przekracza kilkudziesięciu, może kilkuset elementów. W takim scenariuszu czas wykonania tego kodu nie przekracza milisekundy. Po drugi częstotliwość użycia tego kodu jest niewielka.

Z drugiej jednak strony programista, który napisał omawiany kod stosuje (zastosował) takie konstrukcje w wielu miejscach. Być może wywołuje jedną, ciężką, metodę wielokrotnie zamiast zapamiętać jej wynik wywołania na później. Przykładów takich można mnożyć.

Reasumując należy uczyć się na cudzych błędach i strzec się takich konstrukcji.

6 comments:

binary said...

ten 1st przyklad to chyba klasyczne ;) "spaghetti code"? :)
Wyglada jak moje pierwsze kroki w Pascalu ;D eh, to byly czasy... ;)

Michał Komorowski said...

Najgorsze jest to, że to nie był początkujący programista. Mam nadzieję, że to był po prostu jego gorszy dzień.

ferdzikamil said...

Jak umieszczasz kod w poście bo mi wywala błędy

Michał Komorowski said...

W tagach <pre>...</pre>

ferdzikamil said...

nie dziala

csyan said...

The store has been solving problems for me, the service is very patient, and it is a perfect shopping. Cheap Audemars Piguet WatchesWhen I received this watch, I thought it was a good replica watch.Replica Audemars Piguet royal oak I like this online store, this watch is simple and generous. Very satisfied with this purchase.

Post a Comment