Ocena projektu Rest Api Java Spring + prośba o kilka wskazówek

0

Witam, jeśli komuś się chciało tu zajrzeć to zachęcam do oceny mojego api https://github.com/Oziaka/home-budget-api-v2

Chciałbym też prosić o klika wskazówek:
- Jak testować api czy dokończyć te testy używając MockMvc czyli same testy integracyjne czy jednak zrobić jednostkowe a może takie i takie,
- Czy ten projekt nadaje się do CV czy to jednak nie ten poziom
- Jeżeli nie ten poziom to co powinienem poprawić
- I co zrobić żeby jakoś zawojować, pokazać na rynku pracy, robić więcej projektów, jakaś biblioteka

4

Zaczynając od góry:

Statyczne importy = ban,

Klasa JsonTool :

  1. Ta nazwa nic nie mówi, co to jest tool? Tool może być do miliona rzeczy.
  2. public static String parseJson(Object obj) nazwa metody sugeruje, że będziesz w niej parsował JSONa, a na wejściu przyjmujesz obiekt. Źle.
} catch (Exception e) {
            throw new RuntimeException(e);
        }

Co to jest za obsługa wyjątków? Rzucenie innego wyjątku i elo?

 public static Object parseObject(String json, Class clazz) {
        try {
            return mapper.readValue(json, clazz);
        } catch (Exception e) {
            throw new RuntimeException(e);
        }
    }

skoro przekazujesz już typ klasy w parametrze to zmień ta metodę tak aby automatycznie miała ten sam zwracany typ, skorzystaj z generyków. Bez sensu to jest to rzutowanie później przy każdym użyciu, jest to niestety źle. Ta trzecia metoda to samo.

Klasa Tool:

  1. public static String randomString() nazwy metod powinna być odczasownikowe getRandomString(). Reszta to samo.
  2. Nazwa klasy Tool nic nie mówi.

Klasa UriTool

  1. Nazwa klasy
  2. Nazwy metod jw.

Klasa UserTool

  1. Nazwa klasy i nazwy metod.
  2. Nie podoba mi się, że metoda registerRandomUser przyjmuje MockMvc, według mnie powinna mieć swój obiekt.

Klasa WalletTool jw.

Klasa UserResourceTest

  1. Potrafisz uzasadnić użycie @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)?
  2. Podwójnie zła nazwa should_register_user. Po pierwsze camelCase, po drugie przyjmij lepszą konwencje nazewnictwa testów tak żeby każdy po rzuceniu okiem od razu wiedział co on sprawa. Testy to wymagania dlatego muszą mieć dobre nazewnictwo. Sugeruję konwencje (są to przykłady): assertThatUserCanBeRegistered albo givenRegistrationRequestThenUserIsSuccesfullyRegisreted
  3. Brakuje komentarzy given when then. Zle się to czyta.
  4. Ten JSON w postaci Stringa w metodzie should_return_principal() to zły pomysł. Powinieneś czytać go z osobnego pliku.
  5. Tutaj jakaś dziwna konwersja DTO na DTO:
  UserDto expectedResponse = UserDto.builder()
                .email(userDto.getEmail())
                .build();

Nie kumam tego.

  1. .items(new HashMap<>()) korzystaj z Collections.emptyMap()

Klasa WalletResourceTest

  1. import java.awt.*; wtf ?
  2. To samo co wyżej plus nie pushuj zakomentowanego kodu.

Na razie sporo tego tego, więc oprócz testów do reszty kodu nie zaglądałem.

Czy ten projekt nadaje się do CV czy to jednak nie ten poziom
Jak parę osób zrobi jeszcze porządne CR i poprawisz uwagi to możesz wrzucić, ale bądźmy szczerzy - nie będą to fajerwerki.

Jeżeli nie ten poziom to co powinienem poprawić I co zrobić żeby jakoś zawojować, pokazać na rynku pracy, robić więcej projektów, jakaś biblioteka

Ja bym poszedł w technologie, których nie ma każdy bootcampoicz czy wannabe developer w CV. Będzie cię to kosztowało trochę wysiłku, ale postawienie czegoś w WebFluxie albo czegoś prostego na kilku mikroserwisach dałoby ci przewagę nad resztą. I obowiązkowe dobre README.md podejdź do tego pliku na poważnie, to jest pierwszy plik po wejściu w repo, powinien być ładny, estetyczny i mówić wszystko o projekcie. Do tego jakiś prosty ale ładny front w Vue.js i wrzucenie całości na Heroku/AWS. Nie oszukujmy się - ludzie wolą testować taki projekt tzw live, a nie przekopywać się przez kod w repo.

0

Dzięki za feedback, od razu biorę się za poprawki

Klasa JsonTool:

  1. public static String parseJson(Object obj) nazwa metody sugeruje, że będziesz w niej parsował JSONa, a na wejściu przyjmujesz obiekt. Źle

Nie rozumiem, jak masz metodę Integer.parseInt() to parsujesz np String na int

}catch (Exception e) {
            throw new RuntimeException(e);
        }

Co to jest za obsługa wyjątków? Rzucenie innego wyjątku i elo?

No wyrzuciłem RuntimeExcpetion() ponieważ nie chcę aby test dalej trwał, w takim razie jak ty byś to obsłużył?

Klasa UserResourceTest

  1. Potrafisz uzasadnić użycie @DirtiesContext(classMode = DirtiesContext.ClassMode.AFTER_EACH_TEST_METHOD)?

No na razie ale kiedyś się przyda

  1. Podwójnie zła nazwa should_register_user. Po pierwsze camelCase, po drugie przyjmij lepszą konwencje nazewnictwa testów tak żeby każdy po rzuceniu okiem od razu wiedział co on sprawa. Testy to wymagania dlatego muszą mieć dobre nazewnictwo. Sugeruję konwencje (są to przykłady): assertThatUserCanBeRegistered albo givenRegistrationRequestThenUserIsSuccesfullyRegisreted

Z konwencją nazewniczą się zgodzę ale kto jakiego case używa to już jego sprawa

  1. Tutaj jakaś dziwna konwersja DTO na DTO:
  UserDto expectedResponse = UserDto.builder()
                .email(userDto.getEmail())
                .build();

Nie kumam tego.

No po ten userDto posiada hasło a rest api jego nie zwraca, i w ten sposób mam ładnie cały obiekt który zwraca rest api

1

ja bym te get pozamieniał na find

    Optional<Wallet> getById(Long id);

    @Query("SELECT w FROM Wallet w INNER JOIN w.users u WHERE w.id = :walletId AND u.email = :email")
    Optional<Wallet> getByIdAndUserEmail(@Param("walletId") Long walletId, @Param("email") String email);

    Optional<Wallet> getByIdAndOwner(Long walletId, User owner);

skoro grzebiesz tymi metodami w bazie i zwracasz optionale.

2
꧁Oziaka꧂ napisał(a):

Dzięki za feedback, od razu biorę się za poprawki

Klasa JsonTool:

  1. public static String parseJson(Object obj) nazwa metody sugeruje, że będziesz w niej parsował JSONa, a na wejściu przyjmujesz obiekt. Źle

Nie rozumiem, jak masz metodę Integer.parseInt() to parsujesz np String na int

No właśnie, jak masz metodę parseInt to parsujesz Stringa na Int. W metodzie parseJson spodziewałbym się, że będzie parsował Stringa na Jsona.

2
꧁Oziaka꧂ napisał(a):
}catch (Exception e) {
            throw new RuntimeException(e);
        }

Co to jest za obsługa wyjątków? Rzucenie innego wyjątku i elo?

No wyrzuciłem RuntimeExcpetion() ponieważ nie chcę aby test dalej trwał, w takim razie jak ty byś to obsłużył?

Jeśli nie chcesz na każdym poziomie przerzucać wyjątków to @SneakyThrows Twoim przyjacielem.

1 użytkowników online, w tym zalogowanych: 0, gości: 1