Ocena projektów

Od kilku miesięcy uczę się javy i napisałem 3 aplikacje. Mógłby ktoś zobaczyć czy te aplikacje nadają się do wpisania do cv i może zrobić mały code review.

github

bookstore to była moja pierwsza większa apka. użytkownik może przeglądać książki po kategoriach, autorze i wydawnictwie i składać zamówienia, niezalogowany użytkownik ma koszyk w sesji, a zalogowany w zapisuje sie w bazie. Pracownik może dodawać kategorie i książki, przeglądać zamówienia i zmienić status zamówienia.

Później zrobiłem czytnik rss, to raczej mała i prosta aplikacja ale wrzuciłem ją na heroku i używam (wiem że są lepsze, ale ta jest moja :p)

I trzecia apka, jak widać po nazwie, wzorowałem się na wykopie, można dodawać znaleziska, głosować, są tagi, obserwowanie tagów, profil użytkownika i mikroblog. W tym projekcie pisałem pierwszy raz testy, do reszty napisze jak sie dowiem czy te są dobrze napisane(pewnie przydałyby się jeszcze integracyjne).

I na koniec jeszcze jedno pytanie. Patrząc po tych projektach, jak myślicie jak daleko mi jeszcze do tego momentu w którym mógłbym zacząć szukać pierwszej pracy/stażu i czego jeszcze powinienem się nauczyć.
Myślę żeby zacząć kolejny projekt, ale nie mam pomysłu jaki, albo mógłbym rozbudowywać dalej moj wypok, może jakiś panel administratora, ale w sumie nie wiem co ma być w takim panelu

Wypok:

Ogólnie nie jest źle. Możesz wysyłać CV na juniora. Typowa apka na podstawie tutoriala. Brakuje testów i przemyślenia niektórych corner caseów aplikacji. Wiele pętli i null checków można by przepisać funkcyjnie na Stream/Optional lub Vavr. Multum adnotacji zabija czytelność, ala to specyfika Springa, HIbernate i Jackson.

Fajnie jakbyś napisał jakiś projekt bez Springa. Np. Jakąś bibliotekę, która implementuje grę planszową itp. Na razie nie widzę, żebyś umiał programowanie obiektowe. Poczytaj też o wzorcach projektowych. TDD, zasadach SOLID.

Prujesz do bazy jak dziki:

public List<Tag> getTagsFromDBOrSave(List<Tag> tags) {
        List<Tag> result = new ArrayList<>();
        tags.forEach(t -> {
            Tag tag = tagRepository.findByName(t.getName());
            if (tag == null) {
                tag = tagRepository.save(t);
            }
            result.add(tag);
        });
        return result;
}

Co gdy w tekście ktoś wpisze dwie spacje lub tabulator?

public List<Tag> getTagsFromContent(String content) {
String[] words = content.split(" ");

W nowym springu adnotacja @Autowired jest nie potrzebna nad ctorem. Możesz też użyć @RequiredArgsConstructor z lomboka

@Autowired
    public VoteService(VoteRepository voteRepository, UserService userService) {
        this.voteRepository = voteRepository;
        this.userService = userService;
}

Takie wstrzykiwanie przez setery to proszenie się o Null Pointer

 @Autowired
    public void setTagService(TagService tagService) {
        this.tagService = tagService;
}

Testy tylko dla happy path :(

Bookstore:

Nawet nie patrzę. Ta sama architektura jak Wypok. Czyli programowanie proceduralne w Javie.
Brak testów. (Być może są tam same Crudy i dlatego)

Rss reader:

Taka obsługa wyjątku to proszenie się o zgubienie StackTrace:

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

Nie zamykasz klasy, która implementuje interfejs Closable. Poczytaj o try-with

new XmlReader(new URL(url))

Wstrzykuj password encoder z kontekstu, aby mieć zawsze pewność, że używasz tego samego algorytmu hashowania.

PasswordEncoder encoder = new BCryptPasswordEncoder();

W resourceach jakieś prekompilowane magiczne pliki JS. Tego tam być nie powinno. Jeżeli faktycznie bardzo chcesz serwować JS ze Springa to wrzuć te pliki w czasie kompilacji lub zaczytuj z dysku, ale nie trzymaj ich w repozytorium.

Testów brak. :(

1.Dobrą rzeczą jest wstrzykiwanie przez konstruktor, czyli dobrze zrobiłes. Ja bym oznaczył pola jako final.
2.Z serwisów nie powinno się zwracać encji JPA tylko obiekty DTO. Najlepiej niemutowlane. Zwłaszcza jeśli korzystasz z JPA. Kojarzysz coś takiego jak dirty checking etc?
3.Brak testów - poważny problem. Najlepiej ogarnij kombo Groovy + Spock. Nie mockuj do przesady, nietórych rzeczy nie da się przetestować jednostkowo łatwo, załatw to integracyjnie
4. ```java
Publisher publisher = book.getPublisher();
Publisher foundPublisher = publisherRepository.findByName(publisher.getName());
if(foundPublisher == null){
publisherRepository.save(publisher);
}else{
book.setPublisher(foundPublisher);
}

Najgorszej. To nie jest dobrze zrobione, zastanów się czemu ;)
5. Każda książka może mieć wiele autorow (i odwrotnie).
Jak zapisałbyś  [taką książke](https://www.amazon.com/Java-Concurrency-Practice-Brian-Goetz/dp/0321349601) ?
6.Poza kolekcjami w encjach możesz korzystac z lepszych kolekcji niż "wbudowanych" w Jave: https://www.youtube.com/watch?v=2JTlFAjhL3U (ale to już wyższy level)
7.Nie wstawiaj HttpSession do warstwy logiki. Zrób jakiś SessionContext dzięki czemu logika bedzie niezależna od HttpSession.
8.I tak piszesz lepszy kod niż ja kiedy przeszedłem 1 rozmowę o pracę :D
EDIT:
Spring Data może zwracać Optionala. Jesli wiesz że encji może nie być i to jest normalne  to zwróc Optional (albo Option z vavra)

@nie100sowny:
Teraz zacząłem czytać Wzorce projektowe. Rusz głową, o TDD czytałem, ale tutaj testy pisałem dopiero na końcu, następnym razem będę próbował, o SOLID poczytam.

Prujesz do bazy jak dziki:

Czyli jak?

Co gdy w tekście ktoś wpisze dwie spacje lub tabulator?

Nawet nie pomyślałem, teraz podmieniam \t i \n na spacje

Takie wstrzykiwanie przez setery to proszenie się o Null Pointer

To miałem zmienić i zapomniałem

Testy tylko dla happy path :(

Jakie jeszcze powinny być?

W resourceach jakieś prekompilowane magiczne pliki JS

To z* ng build* angulara

Co do reszty że brak testów to pisałem w pierwszym poście że napisze jeśli się okaże że te co mam w wypoku są ok.

@scibi92

2.Kojarzysz coś takiego jak dirty checking

Nie, zaraz sprawdze

3.Brak testów

Jak już pisałem, dopisze testy.

4.Najgorszej. To nie jest dobrze zrobione, zastanów się czemu ;)

Podejrzewałem że to nie jest dobrze, ale w sumie to nie wiem jak inaczej to zrobić

5.Każda książka może mieć wiele autorow (i odwrotnie).

Nie pomyślałem :p

Bardziej polecam wzorce projektowe bandy czworga.

kamil159 napisał(a):

Prujesz do bazy jak dziki:

Czyli jak?

Rób zapytanie o wiele elementów. A nie dla każdego osobne.

Testy tylko dla happy path :(

Jakie jeszcze powinny być?

Test sprawdzające puste stringi, te tabulatory, dziwne znaki unicode, itp.

W resourceach jakieś prekompilowane magiczne pliki JS

To z* ng build* angulara

Wiem, ale nie trzymaj tego w git, bo nikt tego edytował nie będzie.

@nie100sowny:
Mam jeszcze kilka pytań odnośnie testow.
Czy tak się testuje walidacje jak ja to zrobiłem w kontrolerach, dla każdej walidacji osobny test?
Czy w ogóle pisać testy jednostkowe dla metod w kontrolerach które tylko wołają serwis?
Chciałbym też napisać testy integracyjne, jeśli dobrze rozumiem to powinny sprawdzać czy komponenty działają między sobą, czyli integracyjne testy pisać dla kontrolerów? Bo przecież one wywołują całą reszte.
I czy integracyjnie też testować różne przypadki?

kamil159 napisał(a):

@nie100sowny:
Czy w ogóle pisać testy jednostkowe dla metod w kontrolerach które tylko wołają serwis?

Testujesz odpowiedzialności/funkcjonalności. Jeśli ten Controller po coś jest, to musisz przetestować to po co jest.

Hej!

Od siebie dorzucę przyłożenie się do sposobu pisania commitów.
https://chris.beams.io/posts/git-commit/
Niezbyt trudna sprawa a bardzo ułatwia życie.

Pozdrawiam!