[brzydkie zapachy kodu] Długa metoda

Jak nie trudno się domyśleć zapach - długa metoda, polega na tym, iż metoda jest bardzo ... długa ;) Ale co to oznacza w praktyce? Otóż to, iż np. w takiej metodzie znajdziemy pętle, różnego rodzaju obliczenia, wyświetlenia treści etc. Przykład takiej metody:

class Przyklad
{
    public void DlugaMetoda()
    {
        Console.WriteLine("Naglowek");
        Console.WriteLine("---------------------------");

        int wynik = 3 * 5;

        int a = 0;

        for (int i = 0; i < wynik; i++)
        {
            a += wynik;
        }

        if (wynik < a)
        {
            wynik = 123 + 123;
        }
        else
        {
            a = wynik;
        }


        Console.WriteLine("Wyniki:");
        Console.WriteLine("a: " + a);
        Console.WriteLine("wynik: " + wynik);

        Console.WriteLine("---------------------------");
        Console.WriteLine("Stopka");
    }
}

Rozwiązanie tego problemu jest banalne. Wystarczy stosować "wydzielenie metody". Czyli tą długa metodę dzielimy na mniejsze:

class Przyklad
{
    public void DlugaMetoda()
    {
        DrawNaglowek();

        int wynik = ObliczWynik(3, 5);

        int a = 0;

        a = ZrobCos(wynik, a);

        ZrobCosInnego(ref wynik, ref a);

        DrawWyniki(wynik, a);

        DrawStopka();
    }

    private static void ZrobCosInnego(ref int wynik, ref int a)
    {
        if (wynik < a)
        {
            wynik = 123 + 123;
        }
        else
        {
            a = wynik;
        }
    }

    private static int ZrobCos(int wynik, int a)
    {
        for (int i = 0; i < wynik; i++)
        {
            a += wynik;
        }
        return a;
    }

    private static void DrawStopka()
    {
        Console.WriteLine("---------------------------");
        Console.WriteLine("Stopka");
    }

    private static void DrawWyniki(int wynik, int a)
    {
        Console.WriteLine("Wyniki:");
        Console.WriteLine("a: " + a);
        Console.WriteLine("wynik: " + wynik);
    }

    private static void DrawNaglowek()
    {
        Console.WriteLine("Naglowek");
        Console.WriteLine("---------------------------");
    }

    private static int ObliczWynik(int a, int b)
    {
        return a * b;
    }
}

Gdzie mamy zysk?
-wprowadzanie zmian w takiej metodzie jest łatwiejsze
-ciało metody jest CZYTELNIEJSZE
-jesteśmy w stanie objąć całą metodę wzrokiem
-jeśli dobrze nazwiemy te inne metody powstałe przy "wydzieleniu", to nie będziemy musieli zaglądać do ich kodu, gdyż ich nazwa określi nam co robią

[refaktoryzacja] Tworzenie obiektów dla prostych typów danych

Często programiści boją się tworzyć (przynajmniej ja takich spotkałem) małe klasy do obsługi np. kodu pocztowego etc. Niepotrzebnie. Pokażemy to na przykładzie numeru telefonu. Jeśli logika jego obsługi będzie rozmieszczona w różnych klasach, będziemy mieć nie lada problem z zarządzaniem/obsługą/modyfikacją/pielęgnowaniem. Zobrazujmy ten problem:

class Osoba
{
    public string NumerTelefonu
    {
        get
        {
            // zwraca numer telefonu
        }
    }
}

class Centrala
{
    public string NumerTelefonuBezNumeruKierunkowego
    {
        get
        {
            // zwraca numer telefonu bez numeru kierunkowego
        }
    }
}

class Miejscowosc
{
    private Osoba osoba;

    public string NumerKierunkowy
    {
        get
        {
            // wyciagamy numer kierunkowy z numeru telefonu osoba.NumerTelefonu
        }
    }
}

Piękny syf w kodzie ;) Ciekawe gdzie teraz byśmy dodali metodę sprawdzającą poprawność numeru telefonu? Żeby uniknąć takich rzeczy wystarczy stworzyć drobną klasę zajmującą się tylko i wyłącznie obsługą tego numeru telefonu. Czyli wszystkie metody dotyczące numeru telefony przemieszczamy do klasy NumerTelefonu:

class NumerTelefonu
{
    public NumerTelefonu(string numer)
    {
        this.numer = numer;
    }

    private string numer;
    public string Numer
    {
        get
        {
            // zwraca numer telefonu
        }
    }

    public string NumerTelefonuBezNumeruKierunkowego
    {
        get
        {
            // zwraca numer telefonu bez numeru kierunkowego
        }
    }

    public string NumerKierunkowy
    {
        get
        {
            // wyciagamy numer kierunkowy z numeru telefonu osoba.NumerTelefonu
        }
    }

    public bool CzyPoprawny
    {
        get
        {
            // sprawdzenie czy numer jest poprawny
        }
    }
}

Teraz mamy wszystko w jednym miejscu. Nie musimy się zastanawiać gdzie umieścić metodę walidującą numer telefonu, oczywiście w ... NumerTelefonu, ale odkrycie ;)

[tdd] TDD. Przykładowa aplikacja cz. 4

Na razie udało nam się spławić klienta i nie będzie nam zawracał gitary, póki co oczywiście. Możemy coś sobie porobić w wolnym czasie. Zamiast chodzić po stronach i popijać piwko, ulepszymy kod aplikacji dla naszego klienta, ponieważ wiemy, że to się na OPŁACI. Na fantastycznym blogu znaleźliśmy krótkie wprowadzenie do TDD. Z wypiekami na twarzy rzucamy się na zastosowanie w praktyce tej metodyki.

Wiemy, że nasz kod nie jest jednak tak doskonały jak się nam wydawało. Czeka nas w zasadzie całkiem spora refaktoryzacja, która później zaowocuje.

Zajmijmy się najpierw uporządkowaniem naszego pliku, przeniesiemy klasy do nowych plików. Tak więc, jeśli mamy klasę piekarz, to tworzymy plik Piekarz.cs i tam przenosimy całą klasę. Robimy to zarówno dla enum, klas, interfaceów etc.



No niby jest fajnie, lecz nie do końca, jeśli będziemy pracować nad tą aplikacją i będą dochodzić kolejne klasy to zrobi nam się bałagan. Zatem zakładamy katalogi i staramy się w nich umieścić, klasy mające "coś" wspólnego ze sobą. Nasza propozycja wygląda tak:



Okey, na razie tak zostawimy.

Przed przystąpieniem do refaktoryzacji postanawiamy pokryć nasz kod testami, aby klient się nie zdziwił, że poprzednio co działało nagle przestało.

Zaczynamy od napisania testów dla obliczania wynagrodzeń. (Kto nie wie o co chodzi, zapraszam do zapoznania się z tym postem)

Utwórzmy może osobną dll'ke, która będzie zawierała testy. Nazwijmy ją PrzykladowaAplikacja.Tests. Jej struktura będzie identyczna jak struktura PrzykladowaAplikacja, czyli:



Bierzemy się za pisanie testów. Zaczniemy od napisania testów dla WynagrodzenieFactoryMethod. Tutaj będziemy sprawdzać czy metoda zwraca nam odpowiednie typy.

using System;
using System.Collections.Generic;
using System.Text;
using NUnit.Framework;
using PrzykladowaAplikacja.Global;
using PrzykladowaAplikacja.Pracownicy;

namespace PrzykladowaAplikacja.Tests.Pracownicy
{
    [TestFixture]
    public class TestWynagrodzenieFactoryMethod
    {
        [Test]
        public void TestMakePracownik()
        {
            Assert.AreEqual(typeof(Kierownik), 
WynagrodzenieFactoryMethod.MakePracownik(Enums.TypZawodu.KIEROWNIK));
        }
    }
}

Testujemy... i zaskoczenie, test nie przeszedł.
PrzykladowaAplikacja.Tests.Pracownicy.TestWynagrodzenieFactoryMethod.TestMakePracownik : Expected: PrzykladowaAplikacja.Pracownicy.Kierownik
But was: PrzykladowaAplikacja.Pracownicy.Kierownik


dziwny komunikat, ok to zróbmy taką "sztuczkę":

            Assert.AreEqual(typeof(Kierownik).ToString(),
 WynagrodzenieFactoryMethod.MakePracownik(Enums.TypZawodu.KIEROWNIK).ToString());

Testy przechodzą. No to teraz dopisujemy dla każdej klasy odpowiednie sprawdzenie. Dzięki temu klasa WynagrodzenieFactoryMethod jest już otestowana. Hmmm... ale w zasadzie poco w taki sposób napisaliśmy ten test, przecież upubliczniliśmy klasy Piekarz, Nauczyciel, Kierownik, a nie chcemy tego robić. Nie po to tworzymy obiekty przez metodę fabrykującą żeby udostępniać te klasy. Wiec jak możemy zmienić test aby te 3 klasy nie były upublicznione? Otóż bardzo prosto :). Usuńmy public w tych 3 klasach, po czym zmieńmy trochę nasz przypadek testowy:

        [Test]
        public void TestMakePracownik()
        {
            Assert.AreEqual("PrzykladowaAplikacja.Pracownicy.Kierownik", 
WynagrodzenieFactoryMethod.MakePracownik(Enums.TypZawodu.KIEROWNIK).ToString());
            Assert.AreEqual("PrzykladowaAplikacja.Pracownicy.Nauczyciel", 
WynagrodzenieFactoryMethod.MakePracownik(Enums.TypZawodu.NAUCZYCIEL).ToString());
            Assert.AreEqual("PrzykladowaAplikacja.Pracownicy.Piekarz", 
WynagrodzenieFactoryMethod.MakePracownik(Enums.TypZawodu.PIEKARZ).ToString());
        }

Teraz testy również przechodzą a na dodatek mamy ukryte klasy :) O to nam chodziło.

Weźmiemy się teraz za testy naszych metod obliczających. Napiszmy test dla najprostrzej metody wyliczającej wynagrodzenie. Niewątpliwie znajduje się ona w klasie Nauczyciel. Ciało testu:

[TestFixture]
public class TestNauczyciel
{
    [Test]
    public void TestObliczWynagrodzenie()
    {

    }
}

Dobra, to tak. Zawartość metody ObliczWynagrodzenie

  return STAWKA_GODZINOWA * przepracowaneGodziny.Dzienne;

stawka godzinowa wynosi 21. Dajmy na to ze przepracowane godzinny dzienne wynoszą 120, 21 * 120 = 2520. Napiszmy zatem test:

[Test]
public void TestObliczWynagrodzenie()
{
    PrzepracowaneGodziny pg = new PrzepracowaneGodziny(120, 0, 0);

    Assert.AreEqual(2520, 
WynagrodzenieFactoryMethod.MakePracownik(Enums.TypZawodu.NAUCZYCIEL).ObliczWynagrodzenie(pg));
}

świetnie, test przechodzi. Spróbujmy teraz udowodnić, że 120 * 21 != 2520 (taki przypadek również nam się przyda do testu) użyjemy do tego AreNotEqual.

            Assert.AreNotEqual(2520, 
WynagrodzenieFactoryMethod.MakePracownik(Enums.TypZawodu.NAUCZYCIEL).ObliczWynagrodzenie(pg));

testy znów przechodzą, więc tą klasę mamy już pokrytą testami. Po zmianach w tej metodzie będziemy od razu wiedzieli czy coś namieszaliśmy czy też nie.

Tworząc testy dla Kierownika odkrywamy, że... metoda ObliczWynagrodzenie źle działa! No to klient się wkurzy a nam się dostanie. Gdybyśmy pierwsze napisali test a potem dopisali do niego kod, to z pewnością uniknęlibyśmy tego problemu. Gdzie znajduje się błąd? Zacznijmy od wymagań jakie były: Jeśli kierownicy przepracują więcej niż 160, to ich stawka od tej chwili rośnie 2x.
Napiszmy do tego test.

[Test]
public void TestObliczWynagrodzenie()
{
    pg = new PrzepracowaneGodziny(161, 0, 0);
    Assert.AreEqual(8100, WynagrodzenieFactoryMethod.MakePracownik(Enums.TypZawodu.KIEROWNIK).ObliczWynagrodzenie(pg));
}

PrzykladowaAplikacja.Tests.Pracownicy.TestKierownik.TestObliczWynagrodzenie : Expected: 8100
But was: 161.0f

rozjazd jest baardzo duży, miejmy nadzieję, że klient się zorientuje i przestanie używać naszego programu. Błąd znajduje się w metodzie ObliczGodzinyNadliczbowe

return iloscPrzepracowanychGodzin - PODSTAWOWY_CZAS_PRACY * STAWKA_GODZINOWA;

161 - 160 * 50 = -7839, hehe Kierownik jeszcze musi dopłacić jeśli będzie miał nadgodziny ;) Dlatego test nie przechodzi. Poprawmy metodę ObliczGodzinyNadliczbowe

 return (iloscPrzepracowanychGodzin - PODSTAWOWY_CZAS_PRACY) * STAWKA_GODZINOWA * 2;

Teraz jest wszystko dobrze. Udało nam się wyłapać i wyeliminować błąd podczas pisania dla niego testu. Jeśli będziemy tworzyć test PRZED pisaniem unikniemy takich głupot.

Zadzwońmy do klienta i powiedzmy, żeby nie używał tej wersji programu, może będzie wyrozymiały... okazało się, że nie było prądu u niego w firmie więc nie zdarzyli wdrożyć nowej wersji :))) (nie liczmy w prawdziwym życiu na takie farty ;))

Podobne testy piszemy dla Piekarza. Pozostaje pytanie, co w takim razie z metodami prywatnymi, czy je też poddajemy testom. Oczywiście jak zawsze "ZALEŻY". Wg mnie, jeśli mamy jakieś drobne metody prywatne, które nie są krytyczne (w miarę proste), to nie testujemy ich, bowiem tak czy tak jak coś będzie źle, to test wykaże błąd w publicznej metodzie używającej tej prywatnej. Lecz jeśli chcemy to przetestować a poza tym MUSIMY, to zawsze można zmienić z private na public. Lepiej mieć przetestowany i sprawdzony kod. Jeszcze jedną metodą na przetestowanie metod prywatnych jest zmienienie modyfikatora dostępu na protected, wtedy test dziedziczy nasza klasę i dzięki temu mamy dostęp do metody "prywatnej". Krótki przykładzik:

public class A
{
    protected void Dodaj(int a, int b)
    {
        return a + b;
    }
}

 [TestFixture]
    public class TestA : A
    {
        [Test]
        public void TestDodaj()
        {
            Assert.AreEqual(3, this.Dodaj(1, 2));
        }
    }

W następnym poście zajmiemy się refaktoryzacją w kierunku wzorca strategia.

Kod źródłowy

[refaktoryzacja] Parametryzowanie metod. - Przykładowa aplikacja cz. 3

Ostatnio przyszedł do nas klient i nagle zmienił nam wymagania. Chciał żeby wynagrodzenie piekarza było obliczane na podstawie 2óch parametrów, które by reprezentowały stawki godzinowe nocne ale również i dzienne. Do stawek nocnych doliczany jest bonus w wysokości 5 zł. Pierwsze co nam przyszło do głowy to dodanie nowej zmiennej do metody ObliczWynagrodzenie(UInt16 iloscPrzepracowanychGodzinDziennych, UInt16 iloscPrzepracowanychGodzinNocnych);w interface IPracownik. Jeślibyśmy to zastosowali, to zmiany w kodzie byłyby baaardzo duże. Trzeba by było zmienić wszystkie klasy implementujące IPracownik, lecz nie tylko, każde dodanie nowej zmiennej kończyłoby się ze zmianami w całej strukturze klas a na dodatek inne klasy np Nauczyciel nie wykorzystują tych zmiennych.

Czyżby nie było dobrego rozwiązania tej metody? Przecież musi istnieć. Może po prostu powinniśmy zrobić po prostu "paczkę" zmiennych. Utworzylibyśmy klasę PrzepracowaneGodziny, która by się składała z 2óch zmiennych, dzienne i nocne. Zróbmy to:

class PrzepracowaneGodziny
{
    private UInt16 dzienne;
    public UInt16 Dzienne
    {
        get { return dzienne; }
    }

    private UInt16 nocne;
    public UInt16 Nocne
    {
        get { return nocne; }
    }

    public PrzepracowaneGodziny(UInt16 dzienne, UInt16 nocne)
    {
        this.dzienne = dzienne;
        this.nocne = nocne;
    }
}

Dobrze skoro mamy już klasę "paczkę", to podepnijmy ją do naszego systemu.

Musimy zmienić IPracownik.ObliczWynagrodzenie, teraz nie będzie przyjmować zmiennej iloscPrzepracowanychGodzin, lecz naszą klasę PrzepracowaneGodziny

interface IPracownik
{
    float ObliczWynagrodzenie(PrzepracowaneGodziny przepracowaneGodziny);
}

no to skoro zdecydowaliśmy się na taki krok, to musimy pozmieniać we wszystkich klasach implementujących interface IPracownik.

class Nauczyciel : IPracownik
{
    private const UInt16 STAWKA_GODZINOWA = 21;

    public float ObliczWynagrodzenie(PrzepracowaneGodziny przepracowaneGodziny)
    {
        return STAWKA_GODZINOWA * przepracowaneGodziny.Dzienne;
    }
}

wygląda nieźle, ale zobaczymy dalej czy to się sprawdzi.

class Kierownik : IPracownik
{
    private const UInt16 STAWKA_GODZINOWA = 50;

    private const UInt16 PODSTAWOWY_CZAS_PRACY = 160;

    private static bool CzyPodstawowyCzasPracyZostalPrzekroczony(UInt16 iloscPrzepracowanychGodzin)
    {
        return iloscPrzepracowanychGodzin > PODSTAWOWY_CZAS_PRACY;
    }

    private static int ObliczGodzinyNadliczbowe(UInt16 iloscPrzepracowanychGodzin)
    {
        return iloscPrzepracowanychGodzin - PODSTAWOWY_CZAS_PRACY * STAWKA_GODZINOWA;
    }

    private static int ObliczPodstawoweWynagrodzenie()
    {
        return STAWKA_GODZINOWA * PODSTAWOWY_CZAS_PRACY;
    }

    public float ObliczWynagrodzenie(PrzepracowaneGodziny przepracowaneGodziny)
    {
        if (CzyPodstawowyCzasPracyZostalPrzekroczony(przepracowaneGodziny.Dzienne))
        {
            return ObliczPodstawoweWynagrodzenie() + 
 ObliczGodzinyNadliczbowe(przepracowaneGodziny.Dzienne);
        }

        return STAWKA_GODZINOWA * przepracowaneGodziny.Dzienne;
    }
}

tutaj też niczego sobie. Okey, to brnijmy w to dalej. Teraz na warsztat bierzemy klasę Piekarz:

public float ObliczWynagrodzenie(PrzepracowaneGodziny przepracowaneGodziny)
{
    return ((STAWKA_GODZINOWA + BONUS_ZA_PRACE_W_NOCY) * przepracowaneGodziny.Nocne) + 
 (STAWKA_GODZINOWA * przepracowaneGodziny.Dzienne);
}

i znów miłe zaskoczenie, gładko, rzekłbym zbyt łatwo ;). Uporządkujmy ciało tej metody, znów tak jakoś skomplikowanie jest. Zastosujmy wydzielenie metod poznane w 2 części

class Piekarz : IPracownik
{
    private const UInt16 STAWKA_GODZINOWA = 10;

    private const UInt16 BONUS_ZA_PRACE_W_NOCY = 5;

    private static int ObliczWynagrodzenieNocne(UInt16 przepracowaneGodzinyNocne)
    {
        return (STAWKA_GODZINOWA + BONUS_ZA_PRACE_W_NOCY) * przepracowaneGodzinyNocne;
    }

    private static int ObliczWynagrodzenieDzienne(UInt16 przepracowaneGodzinyDzienne)
    {
        return STAWKA_GODZINOWA * przepracowaneGodzinyDzienne;
    }

    public float ObliczWynagrodzenie(PrzepracowaneGodziny przepracowaneGodziny)
    {
        return ObliczWynagrodzenieNocne(przepracowaneGodziny.Nocne) + 
 ObliczWynagrodzenieDzienne(przepracowaneGodziny.Dzienne);
    }
}

wow i w sadzie zakończyliśmy zlecenie.

Wciągnięci pracą nie słyszeliśmy jak dzwonił do nas klient. Nagrał się na automatyczną sekretarkę:
Właśnie jadę do was by obejrzeć gotowy produkt. Mam nadzieję że pamiętaliście o tym, że piekarz w święta za przepracowane godziny otrzymuje podwójną stawkę.

Cholera! Zapomnieliśmy o tym. Ale zaraz! Przecież nasza aplikacja jest wspaniale napisania. Drobna zmiana powinna załatwić sprawę. Pomyślmy, potrzebujemy jeszcze zmienną: swiateczne, w klasie PrzepracowaneGodziny:

class PrzepracowaneGodziny
{
    private UInt16 dzienne;
    public UInt16 Dzienne
    {
        get { return dzienne; }
    }

    private UInt16 nocne;
    public UInt16 Nocne
    {
        get { return nocne; }
    }

    private UInt16 swiateczne;
    public UInt16 Swiateczne
    {
        get { return swiateczne; }
    }

    public PrzepracowaneGodziny(UInt16 dzienne, UInt16 nocne, UInt16 swiateczne)
    {
        this.dzienne = dzienne;
        this.nocne = nocne;
        this.swiateczne = swiateczne;
    }
}

teraz musimy zmienić Piekarz.ObliczWynagrodzenie

class Piekarz : IPracownik
{
    private const UInt16 STAWKA_GODZINOWA = 10;

    private const UInt16 BONUS_ZA_PRACE_W_NOCY = 5;

    private const UInt16 MNOZNIK_STAWKI_SWIATECZNEJ = 2;

    private static int ObliczWynagrodzenieNocne(UInt16 przepracowaneGodzinyNocne)
    {
        return (STAWKA_GODZINOWA + BONUS_ZA_PRACE_W_NOCY) * przepracowaneGodzinyNocne;
    }

    private static int ObliczWynagrodzenieDzienne(UInt16 przepracowaneGodzinyDzienne)
    {
        return STAWKA_GODZINOWA * przepracowaneGodzinyDzienne;
    }

    private static int ObliczWynagrodzenieZaSwieta(UInt16 przepracowaneGodzinySwiateczne)
    {
        return STAWKA_GODZINOWA * MNOZNIK_STAWKI_SWIATECZNEJ * przepracowaneGodzinySwiateczne;
    }

    public float ObliczWynagrodzenie(PrzepracowaneGodziny przepracowaneGodziny)
    {
        return ObliczWynagrodzenieNocne(przepracowaneGodziny.Nocne) + 
ObliczWynagrodzenieDzienne(przepracowaneGodziny.Dzienne) + 
ObliczWynagrodzenieZaSwieta(przepracowaneGodziny.Swiateczne);
    }
}

TYLE! W sam raz, bo właśnie przyszedł klient. Ufff udało nam się, tym razem, lecz coś nam chodzi po głowie, że ten kod, który stworzyliśmy wcale nie jest taki dobry... cdn.

Kod źródłowy

[refaktoryzacja] Tworzenie małych pomocniczych metod. Przykładowa aplikacja cz. 2

W poprzedniej części naszej aplikacji poradziliśmy sobie z problemem narażenia kodu na awarie związaną z dodaniem nowych pracowników, teraz zajmiemy się jego dalsza rozbudową.

Przychodzi do nas klient:
Potrzebujemy utworzyć małą aplikację, która na podstawie wybranego zawodu wyliczy nam miesięczne wynagrodzenie. Do tego wynagrodzenie zależy od ilości przepracowanych godzin oraz stawki. Jeśli kierownicy przepracują więcej niż 160, to ich stawka od tej chwili rośnie 2x.

Pierwsze co musimy zrobić, to otworzyć sobie kod źródłowy z naszego poprzedniego przykładu.
Weźmy się może za dodanie nowej funkcjonalności dla kierownika, czyli jeśli jego ilość godzin będzie większa od 160, to jego stawka sie podwaja:

public float ObliczWynagrodzenie(UInt16 iloscPrzepracowanychGodzin)
{
    if (iloscPrzepracowanychGodzin > 160)
    {
       return (STAWKA_GODZINOWA * 160) + (iloscPrzepracowanychGodzin - 160 * STAWKA_GODZINOWA);
    }

    return STAWKA_GODZINOWA * iloscPrzepracowanychGodzin;
}

Dość szybko nam poszło, lecz hola amigo!

Nagle przychodzi klient:
Witam, rozmyśliliśmy się co do podstawowego wymiaru pracy kierownika, nie chcemy by wynosił 160, lecz 170.

Extra, teraz musimy dokonać zmian w 3 miejscach, a na dodatek jeśli zrobi to obca osoba, to może nawet się nie zorientować co oznacza cyfra 160. Nie możemy tak tego zostawić. Musimy poddać kod drobnej refaktoryzacji. Tworzymy stałą PODSTAWOWY_WYMIAR_PRACY i przypisujemy jej wartość 160, następnie wszędzie w klasie 160 zamieniamy na tą stała. Proszę!

class Kierownik : IPracownik
{
    private const UInt16 STAWKA_GODZINOWA = 50;

    private const UInt16 PODSTAWOWY_CZAS_PRACY = 160;

    public float ObliczWynagrodzenie(UInt16 iloscPrzepracowanychGodzin)
    {
        if (iloscPrzepracowanychGodzin > PODSTAWOWY_CZAS_PRACY)
        {
            return (STAWKA_GODZINOWA * PODSTAWOWY_CZAS_PRACY) + (iloscPrzepracowanychGodzin - PODSTAWOWY_CZAS_PRACY * STAWKA_GODZINOWA);
        }

        return STAWKA_GODZINOWA * iloscPrzepracowanychGodzin;
    }
}

Teraz kod jest czytelniejszy i bardziej elastyczny. Jak przyjdzie klient i powie, że jednak podstawowy czas pracy dla kierownika ma wynosić 180, to nikt nie będzie miał problemów z tą zmianą. Lecz metoda ObliczWynagrodzenie wydaje sie bardzo skomplikowana/rozciągnięta, musimy ją jakoś uprościć.

warunek:

if (iloscPrzepracowanychGodzin > PODSTAWOWY_CZAS_PRACY)

zamienimy na statyczna metodę:

private static bool CzyPodstawowyCzasPracyZostalPrzekroczony(UInt16 iloscPrzepracowanychGodzin)
{
   return iloscPrzepracowanychGodzin > PODSTAWOWY_CZAS_PRACY;
}

dzięki temu po przeczytaniu warunku od razu się zorientujemy co ulega sprawdzeniu.

ta linijka jest jeszcze trochę zagmatwana:

return (STAWKA_GODZINOWA * PODSTAWOWY_CZAS_PRACY) + (iloscPrzepracowanychGodzin - PODSTAWOWY_CZAS_PRACY * STAWKA_GODZINOWA);

stwórzmy z niej 2 metody:

wyrażenie:

(STAWKA_GODZINOWA * PODSTAWOWY_CZAS_PRACY)

zamienimy na:

private static int ObliczPodstawoweWynagrodzenie()
{
   return STAWKA_GODZINOWA * PODSTAWOWY_CZAS_PRACY;
}

natomiast:

(iloscPrzepracowanychGodzin - PODSTAWOWY_CZAS_PRACY * STAWKA_GODZINOWA)

na:

private static int ObliczGodzinyNadliczbowe(UInt16 iloscPrzepracowanychGodzin)
{
   return iloscPrzepracowanychGodzin - PODSTAWOWY_CZAS_PRACY * STAWKA_GODZINOWA;
}

Proszę, z linijki niemal tak długiej jak "spaghetti" udało nam się utworzyć całkiem zgrabny kod:

public float ObliczWynagrodzenie(UInt16 iloscPrzepracowanychGodzin)
{
   if (CzyPodstawowyCzasPracyZostalPrzekroczony(iloscPrzepracowanychGodzin))
   {
       return ObliczPodstawoweWynagrodzenie() + 
ObliczGodzinyNadliczbowe(iloscPrzepracowanychGodzin);
   }

   return STAWKA_GODZINOWA * iloscPrzepracowanychGodzin;
}

Jak widać refaktoryzacja działa cuda.

Uff uporaliśmy sie z wymogiem naszego klienta, jesteśmy bardzo z siebie zadowoleni i już wiemy, że nic nam dobrego humoru nie popsuje.
Nagle dzwoni klient:

Witam! Zapomniałem powiedzieć, ze piekarze muszą mieć inaczej liczoną stawkę w nocy.

Hmm co by tu wykombinować? Żeby to zrobić to dla piekarzy gdzieś trzeba byłoby utworzyć zmienne, które by reprezentowały ilość przepracowanych godzin w dzień oraz w nocy. Jeśli tak zrobimy, musielibyśmy naruszyć nasza metodę ObliczWynagrodzenie przyjmującą wyłączenie 1 zmienną a nie 2! Jeśli to zrobimy będziemy musieli dokonać zmian we wszystkich klasach implementujących IPracownik. Nie możemy na to sobie pozwolić!

Telefon od klienta:
Witam, jeszcze o jednej zmianie zapomniałem wam powiedzieć, jeśli piekarz wyrobi w święta jakieś godziny to musimy mu je pomnożyć przez 2.

istny horror... cdn.

a tym czasem tutaj znajdują się źródła

[refaktoryzacja] Zasada. Dbajmy o nazwy klas i zmiennych

Niezwykle ważną sprawą przy wytwarzaniu DOBREJ jakości kodu są poprawnie dobrane nazwy: metod, klas, zmiennych etc. Może się oczywiście tak zdarzyć, iż w pewnym momencie podczas pracy nad projektem, dojdziemy do wniosku, że taka a taka zmienna powinna sie tak i tak nazywać. Więc przystępujemy do refaktoryzacji. Nawet stosowanie tak prostej zasady przyczynia się do tworzenia na prawdę dobrego i przejrzystego kodu. Może się to na pierwszy rzut oka wydawać śmieszne, lecz czy nie zdarzyło się Wam popatrzeć do kodu i rozmyślać o tym co dana zmienna robi? A gdyby dobrze była nazwana, to już po tym byśmy wiedzieli do czego służy.

Korzyści:
-czytelny kod
-zaoszczędzony CZAS