Styczeń 11, 2019

Strategiczne usuwanie if-ów

Strategiczne usuwanie if-ów

Ponownie zabiorę Cię do swojego labolatorium. Tym razem pokażę Ci jak w łatwy sposób można zwiększyć czytelność swojego kodu. Czynnik, który jest bardzo ważny podczas programowania. W końcu jeden projekt może tworzyć wielu programistów jednocześnie i każdy z nich dokłada od siebie kolejne cegiełki – przy okazji będąc zmuszony do rozumienia cudzych cegiełek.

Zazwyczaj sprowadza się to do prób rozumienia, a tak być nie powinno. Kod napisany przez nas powinien być napisany tak, aby mógł być czytany jak dobra powieść. Musi być wciągający, a nie odrzucający. Wtedy na pewno będziemy programistą, który jest mile widziany w każdym zespole – no powiedzmy. 😉

Skoro znajdziesz się w labolatorium to weźmiemy jakiś organizm żywy – popracujemy nad realnym przykładem i zastanowimy się jak można je zoptymalizować do czytelniejszej formy. Naszym królikiem doświadczalnym będzie moduł odpowiedzialny za wczytywanie i parsowanie konfiguracji z pliku – takie małe nawiąze do piątego zadania z NJD.

Na początek…

Zacznijmy od napisania omawianego wcześniej modułu. Na początku potrzebujemy sam plik konfiguracyjny, będzie on w formie Springowej – properties. 

Tak wygląda przykładowy plik konfiguracyjnyo nazwie: application.properties – zacznijmy tylko od trzech kluczy.

1024kb.baseurl=https://1024kb.pl
1024kb.port=80
1024kb.timeout=5000

Naszym zadaniem jest wczytanie owej konfiguracji, zacznijmy od modelu. W sumie ten etap można pominąć i zapisywać całą konfigurację tylko do mapy – jednak dla czytelności chcę trzymać ją w modelu. Przy okazji w modelu można ustawić pewną domyślną konfigurację.

package org.blog;

public class Properties {
    private String baseUrl = "http://1024kb.pl/";
    private int port = "8080";
    private int timeout = 10000;

    public Properties() {

    }

    public String getBaseUrl() {
        return baseUrl;
    }

    public int getPort() {
        return port;
    }

    public int getTimeout() {
        return timeout;
    }

    public void setBaseUrl(String baseUrl) {
        this.baseUrl = baseUrl;
    }

    public void setPort(int port) {
        this.port = port;
    }

    public void setTimeout(int timeout) {
        this.timeout = timeout;
    }

    @Override
    public String toString() {
        return "Properties{" +
                "baseUrl='" + baseUrl + '\'' +
                ", port='" + port + '\'' +
                ", timeout=" + timeout +
                '}';
    }
}

Czas na Loader, będzie on odpowiedzialny za wczytanie konfiguracji z pliku i jej sparsowanie. Na koniec oczywiście zwrócenie modelu.

package org.blog;

import org.blog.parser.PropertiesParser;

import java.io.BufferedReader;
import java.io.FileNotFoundException;
import java.io.FileReader;
import java.io.IOException;
import java.util.LinkedList;
import java.util.List;

public class PropertiesLoader {
    private PropertiesParser propertiesParser = new PropertiesParser();

    public Properties loadProperties(String fileName) {
        List<String> propertiesLines = loadPropertiesLines(fileName);

        return propertiesParser.parse(propertiesLines);
    }

    private List<String> loadPropertiesLines(String fileName) {
        List<String> propertiesLines = new LinkedList<>();

        try {
            BufferedReader bufferedReader = new BufferedReader(new FileReader(fileName));

            String line = bufferedReader.readLine();
            while (line != null) {
                propertiesLines.add(line);
                line = bufferedReader.readLine();
            }

            bufferedReader.close();

        } catch (IOException e) {
            e.printStackTrace();
        }

        return propertiesLines;
    }
}

I nie jest to jeszcze element, który by nas interesował. Interesuje nas dokładnie linia:

return propertiesParser.parse(propertiesLines);

A dokładnie to co się kryje w metodzie parse. 😉

Najprościej oczywiście użyć if-ów i sprawdzać czy wczytany klucz jest równy kluczowi z konfiguracji.

package org.blog.parser;

import org.blog.Properties;
import java.util.List;

public class PropertiesParser {
    public Properties parse(List<String> lines) {
        Properties properties = new Properties();

        for (String line : lines) {
            properties = parse(properties, line);

        }

        return properties;
    }

    private Properties parse(Properties properties, String line) {
        final String [] values = line.split("=");
        final String key = values[0];
        final String value = values[1];


        if (key.equals("1024kb.baseurl")) {
            properties.setBaseUrl(value);
        } else if (key.equals("1024kb.port")) {
            properties.setPort(Integer.valueOf(value));
        } else if (key.equals("1024kb.timeout")) {
            properties.setTimeout(Integer.valueOf(value));
        }

        return properties;
    }
}

Oczywiście trochę czytelniejsze może być zastosowanie switch-case w połączeniu z enum. Zobaczmy jak będzie wyglądać takie rozwiązanie.

Na początku dodajmy typ wyliczeniowy PropertiesKey, który będzie przechowywać wszystkie dostępne klucze konfiguracyjne – w sobie będzie zawierał String identyfikujący z konkretnym kluczem konfiguracyjnym.

package org.blog;

import com.sun.javafx.fxml.PropertyNotFoundException;

import java.util.Arrays;
import java.util.LinkedList;
import java.util.List;

public enum PropertiesKey {
    BASE_URL("1024kb.baseurl"),
    PORT("1024kb.port"),
    TIMEOUT("1024kb.timeout");

    private final String key;

    PropertiesKey(String key) {
        this.key = key;
    }

    public String getKey() {
        return key;
    }

    public static PropertiesKey getEnumByKey(String key) {
        List<PropertiesKey> propertiesKeys = new LinkedList<>(Arrays.asList(PropertiesKey.values()));
        return propertiesKeys.stream()
                .filter(propertyKey -> propertyKey.getKey().equals(key))
                .findFirst()
                .orElseThrow(PropertyNotFoundException::new);
    }
}

Ze względu, że ptrzebujemy na podstawie podanego Stringa otrzymać konkretny enum musimy napisać taką metodę jak getEnumByKey, która znajdzie dla nas konkretny Enum na podstawie wpisanego do enuma Stringa – jeśli nie znajdzie to rzuci wyjątkiem PropertyNotFoundException. Nic skomplikowanego, a przyda się za chwile.

Po refaktoryzacji if ze Stringiem na switch-case z typem wyliczeniowym nasza metoda parse wygląda teraz tak:

private Properties parse(Properties properties, String line) {
    final String [] values = line.split("=");
    final String key = values[0];
    final String value = values[1];

    final PropertiesKey keyEnum = PropertiesKey.getEnumByKey(key);

    switch (keyEnum) {
        case BASE_URL:
            properties.setBaseUrl(value);
            return properties;

        case TIMEOUT:
            properties.setTimeout(Integer.valueOf(value));
            return properties;

        case PORT:
            properties.setPort(Integer.valueOf(value));
            return properties;
    }

    return properties;
}

Muszę przyznać, że całkiem nieźle, cel niby zdobyty – w końcu pozbyliśmy się if-ów… ale na rzecz switch-case. Nie do końca mnie to zadowala. Możesz się zastanawiać dlaczego tak uważam – w sumie rozwiązanie jest czytelne, więc jak można sprawić, aby było „lepsze”?

Przypuśćmy, że nasz plik konfiguracyjny będzie się ciągle rozrastał wraz z rozwojem projektu – mamy np. aktualnie 20 kluczy – co w takim przypadku?

private Properties parse(Properties properties, String line) {
    final String [] values = line.split("=");
    final String key = values[0];
    final String value = values[1];

    final PropertiesKey keyEnum = PropertiesKey.getEnumByKey(key);

    switch (keyEnum) {
        case BASE_URL:
            properties.setBaseUrl(value);
            return properties;

        case TIMEOUT:
            properties.setTimeout(Integer.valueOf(value));
            return properties;

        case PORT:
            properties.setPort(Integer.valueOf(value));
            return properties;
            
        case ANY:
            properties.setAny(value);
            return properties;
            
        case ANY:
            properties.setAny(value);
            return properties;
            
        case ANY:
            properties.setAny(value);
            return properties;
            
        case ANY:
            properties.setAny(value);
            return properties;
            
        case ANY:
            properties.setAny(value);
            return properties;
            
        case ANY:
            properties.setAny(value);
            return properties;
            
        case ANY:
            properties.setAny(value);
            return properties;
            
        case ANY:
            properties.setAny(value);
            return properties;
            
        case ANY:
            properties.setAny(value);
            return properties;
    }

    return properties;
}

To niestety nasza metoda się rozrasta i rozrasta… końca nie widać. I to w przypadku, gdy tylko ustawiamy od razu wartość,  a co w przypadku, gdy będziemy musieli zrobić coś dodatkowego z wczytaną wartością? Metoda rozrasta się dalej. Musimy przyciąć jak najszybciej korzenie, nie o to nam chodzi.

W tym momencie możemy trochę nawiązać do drugiej zasady SOLID:

Klasy, moduły, funkcje powinny być otwarte na rozszerzenie, ale zamknięte na modyfikacje[

Postarajmy się, żeby nasza metoda nie musiała się już dalej rozrastać, ale nadal mogła być rozszerzana o parsowanie nowych kluczy konfiguracyjnych.

Strategia

Musimy przyjąć pewną strategię – zobaczyć czy jakiś wzorzec projektowy nie mógłby nam w tym pomóc.

I na wstępie już powiem, że oczywiście chodzi o wzorzec projektowy Strategia. Pochodzi on z grupy wzorców behawioralnych, czyli służy do opisywania konkretnych zachowań. Stworzenie takich zachowań pozwala nam na zwiększenie czytelności kodu o czym się zaraz przekonasz. Na razie brzmi tajemniczo, ale już przechodzę do konkretów.

Wróćmy do naszej publicznej metody parse, która analizuje linia po linii wczytaną konfigurację.

public Properties parse(List<String> lines) {
    Properties properties = new Properties();

    for (String line : lines) {
        String [] values = line.split("=");
        String key = values[0];
        String value = values[1];

        
        //how to parse bro???
    }

    return properties;
}

Aby odpowiedzieć na pytanie jak to sparsować potrzebujemy ustalić sobię jakąś strategię. Wydzielić pewien poziom abstrakcji – wspólny interfejs dla różnych strategii.

package org.blog.parser.strategy;

import org.blog.Properties;

public interface PropertyParserStrategy {
    Properties parse(String value, Properties properties);
}

I o to stworzyłem interfejs PropertyParserStrategy, z jedną metodą, która przyjmuje tylko wartość konfiguracji oraz obiekt Properties, do którego zapiszę wczytaną wartość. I na czym teraz polega magiczne wykorzystanie strategii?

Wróćmy do naszego parsera PropertiesParser i utwórzmy w nim EnumMapę, gdzie kluczem są klucze – masło maślane – konfiguracji czyli PropertiesKey. Zaś wartościami w parach są strategie – czyli wszystkie klasy implementujące interfejs PropertyParserStrategy.

public class PropertiesParser {
    private final EnumMap<PropertiesKey, PropertyParserStrategy> parserStrategies = new EnumMap<>(PropertiesKey.class);

    public PropertiesParser() {
    }

    public Properties parse(List<String> lines) {
        Properties properties = new Properties();

        for (String line : lines) {
            String [] values = line.split("=");
            String key = values[0];
            String value = values[1];


            //how to parse bro???
        }

        return properties;
    }
}

Już się trochę zdradziłem – klasy implementujące interfejs. Choć mogło to być dla Ciebie wiadome, że skoro mamy interfejs to i będa klasy.

Stwórzmy sobie trzy klasy – czyli tyle ile mamy kluczy konfiguracyjnych.

BaseUrl strategy

package org.blog.parser.strategy;

import org.blog.Properties;

public class PropertyBaseUrlParserStrategy implements PropertyParserStrategy {
    @Override
    public Properties parse(String value, Properties properties) {
        properties.setBaseUrl(value);
        return properties;
    }
}

Port strategy

package org.blog.parser.strategy;

import org.blog.Properties;

public class PropertyPortParserStrategy implements PropertyParserStrategy {
    @Override
    public Properties parse(String value, Properties properties) {
        properties.setPort(Integer.valueOf(value));
        return properties;
    }
}

Timeout strategy

package org.blog.parser.strategy;

import org.blog.Properties;

public class PropertyTimeoutParserStrategy implements PropertyParserStrategy {
    @Override
    public Properties parse(String value, Properties properties) {
        properties.setTimeout(Integer.valueOf(value));
        return properties;
    }
}

 

Skoro mamy już odpowiednie strategie – strategie czyli w naszym przypadku sposób zapisywania danego klucza konfiguracyjnego.

Mamy już strategie oraz mapę, która czeka na nią to wrzućmy je tam. 😉

public class PropertiesParser {
    private final EnumMap<PropertiesKey, PropertyParserStrategy> parserStrategies = new EnumMap<>(PropertiesKey.class);

    public PropertiesParser() {
        parserStrategies.put(PropertiesKey.BASE_URL, new PropertyBaseUrlParserStrategy());
        parserStrategies.put(PropertiesKey.TIMEOUT, new PropertyTimeoutParserStrategy());
        parserStrategies.put(PropertiesKey.PORT, new PropertyPortParserStrategy());
    }

    public Properties parse(List<String> lines) {
        Properties properties = new Properties();

        for (String line : lines) {
            String [] values = line.split("=");
            String key = values[0];
            String value = values[1];


            //how to parse bro???
        }

        return properties;
    }
}

Nasza mapa pod odpowiednim kluczem ma już odpowiednią strategię zapisywania konfiguracji. Czas użyć tego mechanizmu – czy będzie to trudne?

Na pewno dużo prostsze, a co najważniejsze – zapiszemy to tylko raz!

public Properties parse(List<String> lines) {
    Properties properties = new Properties();

    for (String line : lines) {
        String [] values = line.split("=");
        String key = values[0];
        String value = values[1];


        properties = parserStrategies.get(PropertiesKey.getEnumByKey(key)).parse(value, properties);
    }

    return properties;
}

I mamy najważniejszą linię w tej metodzie:

properties = parserStrategies.get(PropertiesKey.getEnumByKey(key)).parse(value, properties);

Nie dzieje się tutaj nic magicznego – na podstawie wczytanego klucza w mapie wyszukujemy odpowiedniej strategii i to ona jest już odpowiedzialna za zapisanie wartości w odpowiednie miejsce w konfiguracji.

Co nam to daje? Nie będziemy musieli już dłużej rozwijać metody parse, a o to nam chodziło.

Jest jeszcze oczywiście jeden mały błąd – strategia może nie zostać znaleziona i otrzymamy NullPointerException, aby bez problemu możemy się przed tym uchronić korzystając z Optional.

public Properties parse(List<String> lines) {
    final Properties properties = new Properties();

    for (String line : lines) {
        String [] values = line.split("=");
        final String key = values[0];
        final String value = values[1];

        Optional.of(parserStrategies.get(PropertiesKey.getEnumByKey(key)))
                .ifPresent(strategy -> strategy.parse(value, properties));
    }

    return properties;
}

Druga strona medalu

Niby super piękne rozwiązanie – nowy klucz wystarczy utworzyć nową strategię i dodać ją do mapy – co może być tutaj problematyczne?

Pierwszą rzeczą jaka rzuca mi się w oczy to to, że obowiązkowo musimy dodać naszą strategię do mapy. I jest to problem dopóki nie znamy bibliotek/frameworków udostępniającym możliwość użycia Dependency Injection.

Wtedy wystarczy stworzyć klasę, odpowiednio nazwać, a DI kontener (np. Spring) już za nas wstrzyknie wszystkie strategie w odpowiednie miejsce. Krótki przykład wstrzykiwania mapy w Springu możesz zobaczyć tutaj.

Kolejnego problemu nie znalazłem, możliwe, że istnieje, a ja go po prostu nie znalazłem. Jednak to rozwiązanie dużo bardziej mi się podoba i według mnie jest czytelniejsze – to jednak chyba jest jeszcze jedna wada…

Stworzenie bardziej sexy rozwiązania kosztuje więcej czasu niż najprostszego podejścia – np. if/switch, lecz dalsze utrzymywanie/rozwijanie kodu może szybko zwrócić nam ten czas.

Podsumowanie

Pokazałem Ci w swoim labolatorium wzorzec projektowy strategia, która pozwoliła nam w strategiczny sposób pozbyć się ifów oraz switchów. Brzmi pięknie, cel osiągniety, a dalsze rozwijanie projektu jest prostsze. Nie musimy już ruszać modyfikować metody parse, aby dodać kolejną konfigurację.

Podsumujmy owe doświadczenie, strategia zapewnia nam:

  • zmniejszenie ilości wyrażeń warunkowych,
  • uproszczenie struktury kodu, algorytmy mogą być przeniesione do strategii unikając zaśmiecenia np. metody,
  • prostsze testowanie jednostkowego – każda strategia ma wydzielone swoje testy jednostkowe co pozwala szybciej znaleźć błąd jaki może generować metoda korzystająca ze strategii,
  • łatwiejsza analiza kodu, nie musimy już się przedzierać przez duże ilości ifów i switchów.

Na koniec też warto wspomnieć, że w konwencji strategii klasą zbierającą strategie (w naszym przypadku PropertiesParser) nazywa się tzw. Contextem.

A Twoim zdaniem warto poświęcić czas na tworzenie wielu klas na rzecz zmniejszenia ilości wyrażeń warunkowych? Daj co myślisz o wzorcu projektowym strategia w komentarzu. 😉

Kod z tego doświadczenia możesz znaleźć w tym repozytorium.