W tym poście zajmę się refactoringiem jednej złożonej metody. Nie będę dogłębnie wnikał w to co metoda dokładnie robi i w jaki sposób, spojrzę na sam kod od strony refactoringu. Metodę wziąłem od kolegi piszącego na co dzień w innych językach programowania. Metoda ta ma pobierać listę zamówień od jednego z dostawców poprzez API (niestety pierwszego poziomu rest więc kolega ma z tym masę roboty).

Oto metoda przed refactoringiem:

@Override
public List getOrderList() {
    String body = String.format(XmlBuilder.orderListBody(), apiKey);

    Map headers = new HashMap<>();
    headers.put(HttpHeaders.CONTENT_TYPE, MediaType.TEXT_XML_VALUE);

    HttpResponse response = requestBuilder.postRequest(url, headers, body);
    TransactionHeader responseHeaders = extractHeadersFromResponse(response);
    List orderList = extractOrdersFromResponse(response);

    DateTimeFormatter formatter = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss");
    while (responseHeaders.getTooManyItems()) {
        LocalDateTime dateTime = LocalDateTime.parse(orderList.get(orderList.size() - 1).getCreationDateTime(), formatter);
        body = String.format(XmlBuilder.orderListBodyFromDate(dateTime.toLocalDate()), apiKey);
        response = requestBuilder.postRequest(url, headers, body);

        if(httpTooManyRequests(response)) response = requestBuilder.postRequest(url, headers, body);
        responseHeaders = extractHeadersFromResponse(response);
        List secondOrderList = extractOrdersFromResponse(response);
        for (Order x : secondOrderList) {
            if (!orderList.contains(x))
                orderList.add(x);
        }
    }
    return orderList;
}

W tej metodzie można znaleźć kilka poziomów abstrakcji funkcji ale od początku.

1. Od Javy 10 pojawił się syntax sugar w postaci słowa var (nie jest to słowo kluczowe var var = “abc”; jest nadal poprawne). Nie jest to dynamiczne typowanie a skrócenie długich nazw typów. Użyjemy go więc kilka razy.
2. Tworzenie mapy z headerami nie jest tutaj bardzo niezbędne więc możemy je wyekstrahować do metody prywatnej zachowując dobrą nazwę metody (żeby dobrze było wiadomo o co chodzi), np. getHeadersForOrderListRequest().
3. DateTimeFormatter także wyrzucimy do metody prywatnej jednak pozbędziemy się części czasu (który i tak nie jest tutaj wykorzystywany), zostanie więc sama data a co za tym idzie można zmienić typ na LocalDate i pozbyć sie późniejszego wywołania .toLocalDate().
4. Możemy użyć importów statycznych na wywołaniach metod z klasy XMLBuilder, co zwiększy czytelność
5. Nie potrzebne jest sprawdzanie warunku w ifie skoro przed nim, wyżej, mamy to samo przypisanie wartości do zmiennej. EDIT: kolega całkowicie pozbył się później tego sprawdzania w tym miejscu na rzecz oddelegowania tego do innej klasy
6. Programowanie imperatywne czyli np. stare (nie) dobre pętle for można zastąpić pięknym programowaniem deklaratywnym czyli w tym przypadku streamami
7. Można też użyć .parallelStream() w celu przyspieszenia procesowania

Finalnie wersja po refactoringu wygląda tak:

@Override
public List getOrderList() {

    String body = format(orderListBody(), apiKey);
    var headers = getHeadersForOrderListRequest();
    var response = requestBuilder.postRequest(url, headers, body);
    var responseHeaders = extractHeadersFromResponse(response);
    var orderList = extractOrdersFromResponse(response);

    while (responseHeaders.getTooManyItems()) {
        body = format(orderListBodyFromDate(getLastOrderDate(orderList)), apiKey);
        response = requestBuilder.postRequest(url, headers, body);
        responseHeaders = extractHeadersFromResponse(response);

        extractOrdersFromResponse(response).stream()
                .filter(order -> !orderList.contains(order))
                .forEach(orderList::add);
    }
    return orderList;
}

private LocalDate getLastOrderDate(List orderList) {
    return LocalDate.parse(orderList.get(orderList.size() - 1).getCreationDateTime(), getDateFormatter());
}

private Map getHeadersForOrderListRequest() {
    return Map.ofEntries(Map.entry(HttpHeaders.CONTENT_TYPE, MediaType.TEXT_XML_VALUE));
}

private DateTimeFormatter getDateFormatter() {
    return DateTimeFormatter.ofPattern("yyyy-MM-dd");
}

W metodzie mamy teraz podział na przygotowanie zmiennych i na procesowanie ich w pętli while. Zapewne można by to jeszcze jakoś dodatkowo wyczyścić ale nie chciałbym też za bardzo zaciemnić metody chowając np. deklaracje zmiennych przed pętlą while do innej metody. Zapewne dałoby się też wyekstrahować steam do osobngo bloku kodu.

Jeśli ktoś ma jeszcze jakieś propozycje usprawnienia i wyczyszczenia kodu to zapraszam do kontaktu!