Jak robić lepszy code review w Javie

Code review wpływa na jakość oprogramowania i zmniejsza ryzyko powstania bugów. Dobrze wykonany przegląd kodu prowadzi do mniejszej liczby bugów, lepszego zrozumienia kodu i poprawia transfer wiedzy. Osoby, które często recenzują kod, piszą go lepiej, niż przeciętny programista. To dlatego, że z jednej strony widzą więcej, a z drugiej widzą więcej kodu.

Twój zespół jest tak, dobry, jak najsłabszy recenzent w teamie.

- Joel Kemp

Oto kilka rzeczy, na które warto zwrócić uwagę w czasie review.

Sprawdzanie bezpieczeństwa

Zabezpiecz swoje usługi. Twórz je solidnie, spodziewaj się niespodziewanego zachowania i twórz scenariusze awaryjne dla każdego błędu.

Nie loguj poufnych informacji. Chodzi np. O dane kart kredytowych, uprawnienia, hasła itp.

Nie twórz wyjątków, które przekazują wrażliwe dane. To może je ujawnić, a osoba wywołująca kod może wykorzystać te dane niezgodnie z przeznaczeniem.

Parametryzuj zapytania SQL przy użyciu javowego PreparedStatement. To pozwoli Ci uniknąć SQL injection.

Sanityzuj dane wejściowe. Szczególnie mocno skup się na danych z niezaufanych źródeł i pamiętaj, że dane z formularzy internetowych mogą zawierać złośliwe dane.

Niepoprawna sanityzacja danych może prowadzić do wielu problemów z bezpieczeństwem, takich jak Cross-Site Scripting (XSS) i XML injection.

Sprawdzanie testów

Zwróć uwagę na testy dla granicznych wartości. Testujesz zakres wartości? Wybierz najmniejszą, największą wartość dla zakresu i trochę większą niż maksimum.

Stwórz “klasy” wartości i wybierz po jednej z każdej klasy. Przykład:

Dla zakresu 1..100 będziemy mieć 3 klasy. Jedną z tego właśnie zakresu, drugą dla liczb mniejszych niż 1, trzecią dla liczb większych niz 100.

Nie testuj tylko udanych ścieżek. Testuj też te, które nie kończą się powodzeniem, warto też przetestować ścieżki z wyjątkami, gdzie trafisz na niezdefiniowane zachowanie.

Sprawdzanie dostępu do klas Javy

Ograniczaj dostęp. Eksponuj tak mało, jak to tylko możliwe, sprawdź, czy da się przekonwertować to, co jest public na private i protected.

Zależności komponentów powinny być zorientowane w kierunku odpowiadającym stabilności.

Szukaj tego, co się zmienia - to właśnie to powinno zostać wyciągnięte do osobnych obiektów, które potem możesz wstrzyknąć.

Przedkładaj kompozycję nad dziedziczenie.

Final i static

Stałe powinny być umieszczone w klasach final. Dzięki temu trzymamy je w jednym miejscu. Mogą być potem użyte w klasach i w testach.

Dodaj prywatny konstruktor dla klas statycznych. Dzięki temu zapewniasz, że klasa statyczna nie będzie mieć instancji.

Klasy bez podklas powinny być oznaczone jako final. W ten sposób ograniczamy dziedziczenie tam, gdzie go nie trzeba

Fabryki i Buildery

Preferuj fabryki zamiast konstruktorów, bo wyłączają one logikę tworzenia obiektów. Tworzenie obiektów, w większości przypadków, nie jest powiązane z logiką biznesową i może być użyte wielokrotnie. Bez fabryki konieczne jest kopiowanie kodu.

Masz wiele argumentów w konstruktorze? Użyj wzorca builder. Musisz dodać wszystkie atrybuty w łańcuchu wywołań i zakończyć wszystko zbudowaniem instancji.

[kod]

Sprawdzanie użycia null

Unikaj null jako wyjścia metody, zamiast tego użyj Optional.

Optional sprawia, że kod jest czystszy. Praca z interfejsem Optional jest o wiele przyjemniejsza niż z null.

Czasami null jest znaczące, a czasami oznacza “nic”. Kiedy null coś znaczy, użyj Optional, a jeżeli null oznacza faktycznie “nic”, użyj null.

Co to znaczy, że null jest czymś? Pusty koszyk to już coś, brakujące konto to też już coś. To przykłady, gdzie null to “coś” i ma znaczenie.

Nie spodziewasz się, że wyjście będzie puste? Masz do czynienia z niespodziewanym lub niezdefiniowanym zachowaniem? Wtedy użyj null.

Kiedy pracujesz z listami, nie zwracaj null. Biorąc pod uwagę to, co napisałem wyżej, pusta lista to już “coś”. Zwróć pustą kolekcję, a nie null.

Programiści martwią się, że zwracanie pustej listy pogorszy wydajność, co nie jest prawdą. Można użyć niemutowalnej listy, by temu przeciwdziałać. Użyj Collections.emptyList(), Collections.emptySet().

Sprawdzanie kodu z wyjątkami

Dla niespodziewanych scenariuszy użyj wyjątków. To pozwoli na łatwiejsze sprawdzenie, co się stało po fakcie.

Obsłuż wszystkie uzasadnione wyjątki, a gdy zobaczysz jakiś zignorowany, popraw to. Możesz nawet zmusić Sonara, by powiedział o takim kodzie. Wywołujący kod powinien wiedzieć, dlaczego wyjątek się wydarzył, ale pamiętaj, by nie ujawniać wrażliwych informacji.

Zaloguj, co spowodowało wyjątek. Pola, wejścia, parametry - zaloguj je po wystąpieniu wyjątku.

Użyj Optional, gdy null jest “czymś”. Zwróć null albo rzuć wyjątek w sytuacjach niespodziewanych.

Użyj checked exceptions gdy da się z nich wybrnąć i runtime exceptions dla błędów koderskich.

Niespodziewane zachowanie powinno skutkować unchecked exception, bo nie można stworzyć obsługi, która mogłaby mu zapobiec. W takiej sytuacji najlepiej zalogować co się da i rzucić wyjątkiem.

Unikaj null pointer exception i zrób to, co do Ciebie należy, żeby ich uniknąć. NPE są dość kosztowne. Jak bardzo? Zależy od stosu, ale większość kosztu to wywołanie metody fillInStackTrace. Eksperymentalne dane ze StackOverflow sugerują, że spowalniają wykonanie nawet o 10 razy.

Podsumowanie

W skrócie:

• Ogranicz dostęp do wnętrza klas
• Używaj klas final i zapobiegaj tworzeniu instacji klas statycznych
• Używaj fabryk do tworzenia instancji
• Preferuj Optional (zamiast null)
• Nie ignoruj wyjątków
• Pamiętaj o różnicach między wyjątkami checked i unchecked
• Unikaj wyjątków, jeżeli to możliwe

Oryginał tekstu w języku angielskim przeczytasz tutaj,