Zastany kod w praktyce

Ostatnio opisałem jak można zrozumieć legacy code, wymieniając kilka pomocnych technik . Natomiast dziś pokażę jak w praktyce zastosować testy oraz mały refactor aby zrozumieć zastany kod.

Zastany kod- przykładowy serwis

Najpierw spróbuj w starym kodzie znaleźć miejsce gdzie należy wprowadzić zmiany. W moim przykładzie ten fragment wygląda następująco.

@RequiredArgsConstructor
public class LegacyService {

    private ExternalService externalService;
    private AnotherService anotherService;
    private NextService nextService;

    public String veryImportantLegacyLogic(String a, Integer b, boolean c) {
        if (!c) {
            a = externalService.callRemote();
        }

        List<LegacyEntity> l = new ArrayList<>();
        for (int i = 0; i < b; i++) {
            LegacyEntity le = anotherService.getFromDb(a, i);
            l.add(le);
        }

        List<LegacyEntity> l2 = nextService.removeDuplicates(l);

        if (l.size() - l2.size() > 0) {
            return "Duplicates exist for id " + a;
        } else {
            return "No dupliactes found for id" + a;
        }
    }
}

Na pierwszy rzut oka trudno powiedzieć co tutaj się dzieje. Ponieważ nazwy zmiennych nic nie mówią, istnieje skomplikowana logika, występują wywołania zewnętrznych serwisów. Wszystko to powoduje, że ten kod jest dość skomplikowany.

Testy

Aby zrozumieć ten serwis zaczynam od napisania testów, które pozwolą mi przejść całą metodę. Początkowo skupiam się na pojedynczej ścieżce aby jak najszybciej wyjść z metody. Dodatkowo używam nullowych i defaultowych wartości parametrów, bo nie wiem jakie one powinny być. Najprostszy przykład pierwszego testu, może wyglądać tak.

    @Test
    void should(){
        //Given
        
        //When
        String result = legacyService.veryImportantLegacyLogic(null, null, false);
        
        //Then
        assertThat(result).isEqualTo("b");
    }

Zgodnie z oczekiwaniami, test nie przejdzie ponieważ jeden z serwisów jest nullem. W tym momencie następuje ważna rzecz – zmianiam nazwę tego testu na coś co mówi co tu się dzieje np should_throw_npe_when_external_service_is_null oraz poprawiem go aby przechodził.

Kolejnym krokiem, jest stworzenie następnego testu, podobnego do powyższego. Jedyną różnicą jest to, że w miejscu gdzie poprzednio test nie przeszedł należy wprowadzić nie nullową zależność. W tym celu zamockuję externalService. Test wywali się w kolejnym miejscu.

Poprawiam ten test, zmieniam nazwę i powtarzam powyższe kroki z kolejnymi testami do momentu aż uda się wyjść z metody bez błędu. W razie potrzeby ustawiam odpowiednie parametry i mockuję kolejne zależności.

Kiedy test w końcu przejdzie, warto napisać testy dla alternatywnych ścieżek oraz zweryfikować wywołania zamockowanych serwisów. Ważne aby udało Ci się zrozumieć zastany kod a każdy test może Ci w tym pomóc. Dla serwisu stworzyłem takie testy:

    private LegacyService legacyService;

    @Mock
    private ExternalService externalService;
    @Mock
    private AnotherService anotherService;
    @Mock
    private NextService nextService;

    @Test
    void should_throw_npe_when_external_service_is_null (){
        //Given
        legacyService = new LegacyService(null, null, null);

        //When
        Throwable thrown = catchThrowable(() -> legacyService.veryImportantLegacyLogic(null, null, false));

        //Then
        assertThat(thrown).isInstanceOf(NullPointerException.class);
    }


    @Test
    void should_throw_npe_when_integer_param_is_null(){
        //Given
        legacyService = new LegacyService(externalService, null, null);

        //When
        Throwable thrown = catchThrowable(() -> legacyService.veryImportantLegacyLogic(null, null, false));

        //Then
        assertThat(thrown).isInstanceOf(NullPointerException.class);
    }

    @Test
    void should_throw_npe_when_another_service_is_null(){
        //Given
        legacyService = new LegacyService(externalService, null, null);

        //When
        Throwable thrown = catchThrowable(() -> legacyService.veryImportantLegacyLogic(null, 1, false));

        //Then
        assertThat(thrown).isInstanceOf(NullPointerException.class);
    }

    @Test
    void should_throw_npe_when_next_service_is_null(){
        //Given
        legacyService = new LegacyService(externalService, anotherService, null);

        //When
        Throwable thrown = catchThrowable(() -> legacyService.veryImportantLegacyLogic(null, 1, false));

        //Then
        assertThat(thrown).isInstanceOf(NullPointerException.class);
    }

    @Test
    void should_pass_method_with_duplicates(){
        //Given
        legacyService = new LegacyService(externalService, anotherService, nextService);

        //When
        String result = legacyService.veryImportantLegacyLogic(null, 1, false);

        //Then
        assertThat(result).isEqualTo("Duplicates exist for id null");
    }

    @Test
    void should_pass_method_with_no_duplicates(){
        //Given
        legacyService = new LegacyService(externalService, anotherService, nextService);
        when(nextService.removeDuplicates(anyList())).then(AdditionalAnswers.returnsFirstArg());

        //When
        String result = legacyService.veryImportantLegacyLogic(null, 1, false);

        //Then
        assertThat(result).isEqualTo("No dupliactes found for id null");
    }

    @Test
    void should_call_external_service_once_when_flag_is_false(){
        //Given
        legacyService = new LegacyService(externalService, anotherService, nextService);

        //When
        String result = legacyService.veryImportantLegacyLogic(null, 1, false);

        //Then
        verify(externalService).callRemote();
    }

    @Test
    void should_not_call_external_service_when_flag_is_true(){
        //Given
        legacyService = new LegacyService(externalService, anotherService, nextService);

        //When
        String result = legacyService.veryImportantLegacyLogic(null, 1, true);

        //Then
        verify(externalService,never()).callRemote();
    }

    @ParameterizedTest
    @ValueSource(ints={0,1,3})
    void should_call_another_service_exactly_times_like_int_param(Integer param){
        //Given
        legacyService = new LegacyService(externalService, anotherService, nextService);

        //When
        String result = legacyService.veryImportantLegacyLogic("a", param, true);

        //Then
        verify(anotherService,times(param)).getFromDb(anyString(),anyInt());
    }

    @Test
    void should_call_next_service_once(){
        //Given
        legacyService = new LegacyService(externalService, anotherService, nextService);

        //When
        String result = legacyService.veryImportantLegacyLogic("a", 1, true);

        //Then
        verify(nextService).removeDuplicates(anyList());
    }

Mały refactor

Po napisaniu testów, można wykonać mały refactor. Przez mały refactor rozumiem tutaj nadanie lepszych nazw zmiennym, wyekstrahowanie metod. Możesz tu użyć wszystkiego co sprawi, że zastany kod będzie łatwiejszy w zrozumieniu. Po moich zmianach LegacyService wygląda następująco:

public String veryImportantLegacyLogic(String id, Integer version, boolean couldCallRemote) {
        id = callRemote(id, couldCallRemote);

        List<LegacyEntity> legacyEntities = findByLegacyEntityAndVersionLessOrEqual(id, version);

        return checkDuplicates(id, legacyEntities);
    }

    private String checkDuplicates(String id, List<LegacyEntity> legacyEntities) {
        List<LegacyEntity> legacyEntitiesWithoutDuplicates = nextService.removeDuplicates(legacyEntities);

        if (doListsHaveDifferentSize(legacyEntities, legacyEntitiesWithoutDuplicates)) {
            return "Duplicates exist for id " + id;
        } else {
            return "No dupliactes found for id " + id;
        }
    }

    private boolean doListsHaveDifferentSize(List<LegacyEntity> legacyEntities, List<LegacyEntity> legacyEntitiesWithoutDuplicates) {
        return legacyEntities.size() - legacyEntitiesWithoutDuplicates.size() > 0;
    }

    private List<LegacyEntity> findByLegacyEntityAndVersionLessOrEqual(String id, Integer version) {
        List<LegacyEntity> legacyEntities = new ArrayList<>();
        for (int i = 0; i < version; i++) {
            LegacyEntity le = anotherService.getFromDb(id, i);
            legacyEntities.add(le);
        }
        return legacyEntities;
    }

    private String callRemote(String id, boolean couldCallRemote) {
        if (!couldCallRemote) {
            return externalService.callRemote();
        }
        return id;
    }

Zakończenie

Zazwyczaj legacy code jest w dużo gorszym stanie niż mój przykład. Jednak dzięki temu ćwiczeniu łatwiej będzie Ci zrozumieć zastany kod. A jak już będziesz musieć z nim pracować, to może się okaże, że nie jest aż tak źle jak myślałeś.

Kod dostępny jest na githubie.