Refaktoryzacja – Extract and Move Something i SRP

Na wczorajszym spotkaniu Warszawskiej Grupy Wzorców Projektowych Krzysiek Jelski opowiadał o zasadach S.O.L.I.D. Nie będę teraz poruszał tego tematu zbyt dokładnie, bo nie ma na to czasu. Zajmę się czymś innym. Jednak najpierw kilka słów wprowadzenia, które urodziły się dzięki tej prezentacji.
Generalnie Krzysiek przedstawił nam pewne „złote myśli” programowania obiektowego, a następnie zaprezentował krótką refaktoryzację kodu. Szkoda tylko, że nie opowiadał przy okazji co i jak łączy się z prezentowanymi zasadami. Cóż bywa. W każdym bądź razie w trakcie zabawy z kodem dość intensywnie używał Extract Method i Extract Class. Polegają one na wyłączeniu części kodu do osobnej metody lub klasy. Pozwala to na sprawne stosowanie Singiel Responsibility Principle i tym samym tworzenie dobrego kodu. Jakby przy okazji pojawi się jeszcze Move Method.

Contents

Podstawy

Punktem od którego zaczynamy tą refaktoryzację jest identyfikacja powtarzających się fragmentów kodu. Takie fragmenty powinny w pierwszej kolejności zostać zredukowane do jednego bloku kodu. Skala zmian musi być dostosowana do wielkości powtarzającego się kodu. Warto wyciągać na zewnątrz nie tylko duże konstrukcje, ale też często używane warunki z bloków if.
Po takiej wstępnej przymiarce możemy przejść do kolejnego kroku.

Wyciąganie bloków logicznych

Blokiem logicznym jest na przykład pętla, instrukcja warunkowa lub warunek, które stanową autonomiczną całość. Poniżej kod przed refaktoryzacją.

Listing 1. Kod przed refaktoryzacją. Zagnieżdżona pętla to zuo

package pl.koziolekweb.musicarchchecker;

import java.io.File;

public class App {

   private static final String PATH = "f:/";

   public static void main(String[] args) {
      if (args.length <= 0)
         System.out.println("Nie podano ścieżki do biblioteki! Użyto domyślnej.");

      File library = new File(PATH);
      if (library.isFile())
         throw new IllegalArgumentException("Bilioteka nie jest katalogiem!");
      
      Checker checker = new Checker();
      for (File band : library.listFiles()) {
         if (band.isDirectory() && !band.isHidden()) {
            for (File album : band.listFiles()) {
               if (album.isDirectory() && !album.isHidden()) {
                  if (!checker.check(album.getName())) {
                     System.out.println(album.getAbsolutePath());
                  }
               }
            }
         }
      }
   }
}

Jak widać mamy tu dość poważny problem polegający na dość rozbudowanej strukturze zagnieżdżonych pętli i warunków. Dodatkowo klasa App ma pomieszaną odpowiedzialność, bo realizuje zarówno parsowanie dokumentów jak i sprawdzanie katalogów za pomocą checker.check(). Po refaktoryzacji klasa ta wygląda tak:

Listing 2. Kod po refaktoryzacji. Całość wygląda znacznie lepiej.

package pl.koziolekweb.musicarchchecker;

import java.io.File;

public class App {

   private static final String PATH = "f:/";

   public static void main(String[] args) {
      if (args.length <= 0)
         System.out.println("Nie podano ścieżki do biblioteki! Użyto domyślnej.");

      File library = new File(PATH);
      if (library.isFile())
         throw new IllegalArgumentException("Bilioteka nie jest katalogiem!");

      Checker checker = new Checker();
      checker.checkLibrary(library);
   }
}

Jest to wynik mojej dotychczasowej pracy. Po pierwsze wykorzystałem Extract Method, Extract Class i Singiel Responsibility Principle i przeniosłem cały kod do klasy Checker tym samym klasa App odpowiada tylko za sprawdzanie argumentów i odpalenie sprawdzania biblioteki. Nadal są to dwie odpowiedzialności, dlatego też kolejnym krokiem będzie przeniesienie sprawdzania parametrów do osobnej metody i w kolejnym kroku osobnej klasy. Klasa App będzie miała w swoim zakresie tylko sterowanie aplikacją. Na tym etapie warto zapoznać się z możliwościami IDE jeśli chodzi o automatyzację niektórych procesów. Zazwyczaj powinniśmy mieć możliwość zastosowania trzech podstawowych refaktoryzacji:

  • Extract Method - w celu przeniesienia zaznaczonego bloku kodu do osobnej metody
  • Extract Class - w celu przeniesienia metody do klasy bazowej.
  • Move Method - w celu przeniesienia metody do nowej klasy.

Jeżeli twoje IDE nie umożliwia tych prostych działań w stosunku do zwykłej klasy wywal je.
Wracając do klasy App... Zapewne jesteś ciekawy czytelniku co z niej pozostało 🙂 Niewiele:

Listing 3. App - po dobrej refaktoryzacji nie ma już co refaktoryzować.

package pl.koziolekweb.musicarchchecker;

public class App {

	private static LibraryFactory libraryFactory = new LibraryFactory();
	private static Checker checker = new Checker();

	public static void main(String[] args) {
		checker.checkLibrary(libraryFactory.createLibrary(args));
	}
}

Moim zdaniem mistrzostwo świata 🙂 W praktyce nie ma tu już co refaktoryzować, bo każda kolejna zmiana zagmatwała by tylko go zamiast go uprościć. Świerzbi mnie jeszcze przenieść klasę Checker do fabryki i rozmawiać o interfejsach, ale... zresztą kill'em 🙂

Listing 4. App - jak widać zawsze się coś znajdzie...

package pl.koziolekweb.musicarchchecker;

public class App {

	private static LibraryFactory libraryFactory = new LibraryFactory();

	public static void main(String[] args) {
		CheckerFactory.getChecker().checkLibrary(
				libraryFactory.createLibrary(args));
	}
}

Ekstremizm jest fajny w tym przypadku. Porównując kod z listingów 1 i 4 w praktyce nie można powiedzieć, że to ten sam program. Chcąc rozbić jedną metodę, main, na kilka przy pomocy Extract Method i pewnych bloków logicznych w praktyce uprościłem cały program główny tak, że można go opisać w jednym zdaniu.

SemantykaW i refaktoryzacja

W trakcie spotkania poruszony został temat niechęci do refaktoryzacji wynikającej z konieczności tworzenia dokumentacji. Oczywiście od razu padło pojęcie kodu samodokumentującego się. W trakcie planowania refaktoryzacji należy odwoływać się do umiejętności językowych. Opiszę tutaj mój sposób na refaktoryzację za pomocą języka. Wypracowałem go pewien czas temu i nazwałem "zasadą trzech zdań".

Jeżeli nie potrafisz opisać tego co kod robi za pomocą maksymalnie trzech zdań prostych to kod prawdopodobnie wymaga refaktoryzacji.

Jest to wariacja na temat zasady, mówiącej, że kod dłuższy niż pięć linii wymaga prawdopodobnie refaktoryzacji.
Programowanie jest zajęciem wciągającym, ale przede wszystkim wymagającym umiejętności pogodzenia myślenia abstrakcyjnego i logicznego. Charakter pracy wymaga też przedstawiania swoich wyrobów w sposób dostępny i łatwo przyswajalny dla ogółu. Jeżeli chcemy to osiągnąć musimy sami rozumieć kod, który produkujemy. Należy też pamiętać, że osoby, które będą czytały kod powinny móc zrozumieć jego działanie na dowolnym poziomie abstrakcji. Dlatego też warto wprowadzić pewne metody językowe przy tworzeniu kodu. Ułatwią one nie tyko poznanie kodu innym osobą, ale przede wszystkim ułatwią nam tworzenie dokumentacji, modyfikowanie i utrzymanie kodu.

Epopeja to zuo

Dlaczego w szkole nie lubiliśmy czytać lektur? Generalnie były zbyt nudne lub za długie. "Lalka" Prusa jest ciekawa, ale niemiłosiernie długa i dlatego jej nie przeczytałem za pierwszym podejściem. "Cierpienia młodego Wertera" są stosunkowo krótkie, ale nudne i dlatego przeczytałem skrót ze streszczenia. Generalnie zawsze odnosiłem wrażenie, że lista lektur ustawiana jest na zasadzie "jak ciekawe to musi być długie, a krótkie muszą być nudne". Podobnie ma się sprawa z kodem.
Popatrzmy na listing 1. Kod jest długi i choć przy wczytaniu się okazuje się stosunkowo prosty to wymaga wczytania się i jest zbyt szczegółowy. Kod na listingu 4. jest już krótki i zwarty. Jest on przykładem realizacji "Zasady trzech zdań". Precyzyjnie tłumaczy co robi dany kod na danym poziomie abstrakcji , jednocześnie nie dostarcza dodatkowych, zbędnych informacji oraz pozwala na zadanie pytań dlaczego, jak, po co.
Zwięzły kod to kod, który przy opisie dostarcza pełnej wiedzy na swój temat przy jednoczesnym pomijaniu szczegółów nieistotnych na danym poziomie. Pisanie epopei programistycznej czy to w dokumentacji czy to w kodzie zazwyczaj mija się z celem, bo powoduje zagmatwanie problemu. Należy jednak pamiętać, że kod krótki nie zawsze jest dobry. Przykładem jest kod z listingu 3. Pomimo, że jest on tak samo długi jak kod z listingu 4 to ma pewien mankament. Dostarcza za dużo informacji. Czytelnika nie interesuje to jak konkretnie dostarczany jest obiekt checker ponieważ informacja ta nie mieści się w pytaniu "Jak działa aplikacja?". Jeżeli jednak zainteresowany będzie drążył temat, "Jak uzyskiwany jest obiekt checker?", to będziemy mogli mu swobodnie odpowiedzieć na to pytanie kierując do klasy CheckerFactory. Istotne jest tu to, że po takim skierowaniu dostanie precyzyjną odpowiedź na pytanie, a jednocześnie nie będzie sobie zaprzątał głowy innymi elementami aplikacji.
Wracając do dokumentacji. Skoro nasz kod jest naprawdę dobry, czyli:

  • Jest luźno powiązany - obiekty komunikują się za pomocą interfejsów
  • Jest abstrakcyjny na danym poziomie szczegółowości - operacje nie dostarczają informacji o swoich szczegółach i ogólniejszym kontekście swego działania
  • Jest zwięzły - robi tylko tyle co musi i nie ma ozdobników
Mowa jest srebrem, milczenie złotem

Pytanie jaka jest rola dokumentacji? Dokumentacja przede wszystkim powinna robić dwie rzeczy. Tłumaczyć kod na język zrozumiały dla ogółu. Wiązać poszczególne elementy pozwalając na szybkie przemieszczanie się po między nimi.
Drugi element jest czysto funkcjonalny. W języku Java mechanizmy JavaDoc udostępniają adnotacje @link i @see, które pozwalają na kierowanie ruchem. Mechanizm "all known subclasses/interfaces/implementations" jest automatem, który pozwala na uszczegółowienie odpowiedzi na pytania dlaczego i w jaki sposób.
Pierwszy element jest znacznie bardziej skomplikowany. Mówiłem już, że dobry kod pozwala opisać się językiem naturalnym w trzech zdaniach. Dokumentacja powinna zawierać te trzy zdania, ale powinna też praktycznie demonstrować zasady użycia kodu. Nie można dopuścić do sytuacji, w której zachowanie kodu nie jest wytłumaczone w dokumentacji. Jako przykład podam klasę Checker.

Listing 5. Checker - coś nie tak z dokumentacją

package pl.koziolekweb.musicarchchecker;

import //...

/**
 * Prawidłowa nazwa katalogu z plikami powinna spełniać takie założenia:<br/>
 * <ul>
 * <li>pierwsze cztery znaki to data wydania</li>
 * <li>następnie spacja myślnik i spacja lub sama spacja</li>
 * <li>następnie nazwa płyty</li>
 * </ul>
 * Przykład: 2007 - Liberty Or Death
 */
public class Checker implements CheckerInterface {

	private CheckerPrinter checkerPrinter = new CheckerPrinter();

	/**
	 * Wzorzec sprawdzający poprawność nazwy płyty
* "^\\d{4}(( - )|( )).*$" */ private Pattern pattern = Pattern.compile("^\\d{4}(( - )|( )).*$"); /** * Metoda sprawdza czy nazwa płyty pasuje do wzorca. * * @param name * nazwa płyty * @return prawda jeżeli pasuje do wzorca. Fałsz w przeciwnym wypadku */ public boolean check(String name) { //... } /* (non-Javadoc) * @see pl.koziolekweb.musicarchchecker.CheckerInterface#checkLibrary(java.io.File) */ public void checkLibrary(File library) { //... } private void checkBand(File band) { //... } private void checkAlbum(File album) { //... } private boolean isNotHiddenAndIsDirectory(File entry) { //... } }

Pytanie co jest nie tak z dokumentacją tej klasy? Niech będzie to praca domowa, a odpowiedzi proszę umieszczać w komentarzach.

Samodzielne opisywanie się kodu

Ostatnim zagadnieniem, które poruszę jest problem nazewnictwa. Mając refaktoryzować kod zastanawiamy się w pierwszej kolejności po co. Odpowiedź na to pytanie jest oczywista. Następnie stajemy przed problemem jak najlepiej nazwać nowy element. Niestety trzeba czasami wysilić szare komórki, ale zyski są ogromne. Popatrzcie na ostatnią metodę na listingu 5. Co robi? Nazwa jednoznacznie wskazuje co metoda robi i nawet bez kodu można odtworzyć jej działanie. Nie można tego powiedzieć o metodzie check, która sprawdza coś, ale bez dokumentacji nie wiadomo co.
Nadając nazwę jakiemuś elementowi należy spróbować z trzech zdań opisujących ten element wyciągnąć słowo lub słowa klucze. Odpowiednie ich zestawienie tworzy nazwę nowego elementu.

Podsumowanie

Zaczęliśmy od trzech prostych refaktoryzacji, które produkują spójny i zwięzły kod. Ich "wadą mniemaną" jest konieczność tworzenia dokumentacji, ale jak przedstawiłem w drugiej części odpowiednio przeprowadzona refaktoryzacja nazw pozwala na uniknięcie konieczności pisania rozbudowanej dokumentacji. Dobra znajomość języka potocznego pozwala na sprawne zarządzanie kodem poprzez tworzenie łatwej w użyciu dokumentacji. Dobra dokumentacja gwarantuje, że kod opisywany jest zwięzly, elastyczny i łatwiejszy w utrzymaniu. Ta wzajemna zależność pomiędzy elementami kodu i dokumentacją pozwala nam na ciągłe ulepszanie naszych produktów.

3 myśli na temat “Refaktoryzacja – Extract and Move Something i SRP

  1. Odnośnie listingu nr 5 – za duzo kropek wg mnie.

    Checker checker = CheckerFactory.getChecker();
    Library library = LibraryFactory.createLibrary(args);
    checker.checkLibrary(library);

    Wg. mnie wyglada nieco bardziej czytelnie.

  2. Maćku, rzeczywiście jest tam dużo kropek, ale wynika to z tego, że Eclipse w trakcie refaktoryzacji czarował za bardzo.

Napisz odpowiedź

Twój adres email nie zostanie opublikowany. Pola, których wypełnienie jest wymagane, są oznaczone symbolem *

To create code blocks or other preformatted text, indent by four spaces:

    This will be displayed in a monospaced font. The first four 
    spaces will be stripped off, but all other whitespace
    will be preserved.
    
    Markdown is turned off in code blocks:
     [This is not a link](http://example.com)

To create not a block, but an inline code span, use backticks:

Here is some inline `code`.

For more help see http://daringfireball.net/projects/markdown/syntax