matematyk60 / java_company Goto Github PK

1. Build gradlowy:

• proponowałbym wydzielenie osobnego taska na fatjara, nazwanego inaczej np fatJar, a nie dokładanie dependencisów do obecnego taska jar.

2. Ładnie dodałeś git ignora, ale trochę to bierze w łeb jeśli potem w głównym folderze tworzysz kolejny exc1 w którym jest właściwy projekt i te wszystkie katalogi .idea itd. nazwa głównego pakietu to odwrócona domena, czyli przeważnie jakie com.tegess.. pl.dgodek. itd. Wgl o konwencjach nazewnictwa pakietów sobie przeczytaj, fajny temat xD
3. Employee:

• metody w interfejsie są defaultowo public nie trzeba tego pisać
• nie wiem po co Ci metoda getAmountOfWork, skoro masz metodę reportWork, która zwraca tą samą wartość tylko ładnie opakowaną w klasę.

4. Abstract Employee:

• Jest literówka w nazwie klasy, nie wiem po co nadpisujesz tu equalsa?, getter i setter do amoutOfWork moim zdaniem niepotrzebne :)
• toString - znów kwestia konwencji, ja tam preferuje albo jsona, albo scalowe case Classy. Ale najgorzej nie jest xd jedynie poskleja Ci to całe w stringa nieoddzielonego między wyrazami.

5. Developer:

• Jak już extendujesz AbstractEmployee, to nie musisz pisać implements Employee.
• lista tasków może być final (jak wgl większość pól we wszystkich klasach), a do tego nigdzie jej nie wykorzystujesz...

6. TeamManager:

• szanuje, że Ci się chciało bawić z tablicą, ale zupełnie niepotrzebnie. Możesz równie dobrze skorzystać z arraylisty podac jej na początku rozmiar i zamiast iterowania się po tablicy, pętli, ifów masz fajne api listy do dyspozycji, a wydajnościowo to samo. Do tego trzymać zmienną na max elementów i done. To powinno uprościć ten kod bardzo.

7. Report:

• fajny pomysł z interfejsem i różnymi implementacjami, ale metoda do wyświetlania raczej nie powinna być w modelu. Raczej jakiś serwisik przyjmujący Report i printujący toStringa z reportu, albo coś w tym typie.
• Używaj list zamiast tablic, potem masz do dyspozycji stream api, unikasz petli, kod czytelniejszy, mniej ifowania, lambdy itd.

Ogólnie masz literówek kilka (IDE powinno Ci to podkreślić), nieużywanych metod, warto też czasem kliknąć autoformatowanie w pliku który napisałeś, ładniej to wygląda.
Zamiast tablic listy + stream API javy8. Warto zobaczyć co to guava i jak działają immutable collections, bo tu się przydają (szczególnie jak mówimy o randomowym generowaniu). Generowanie może też warto by jakoś zamodelować, a nie wrzucić wszystko do jednej klasy? :)

To chyba tyle -> może warto pomyśleć nad tym jak można takie coś potestować? A reszte będziemy dalej rozwijać to wyjdzie w praniu xd

Dzięki i miłego! :)

1. Builderki:
   Hah, wiedziałem, że coś mi już nie grało jak robiłem review Natalii. Te buildery powinny wygladać moim zdaniem tak:
   https://gist.github.com/Passarinho4/0191adc05e5b9fa57766677a22c84545
   https://gist.github.com/Passarinho4/951645165789bf134a8ad9cd755d6962
   z dokładnością do nazw metod, bo wygenerowałem settery i nie chciało mi się zmieniać.
   Jak już builder do managera to nie getBlalala... tylko buildBlablalal xd

2. Nazewnictwo:
   Tutaj myślę, że warto zwrócić większą uwagę na nazywanie metod, pól nazwami domenowymi bardziej niż co robią technicznie. Np setPredicate -> hireStrategy (raz że strategy sugeruje użycie wzorca strategy, co bardzo ładnie robisz wykorzystując to co oferuje Java 8, a dwa hire mówi o tym, że jest to związane z zatrudnianiem, a nie z pracownikami tylko).

3. Raport:
   Czy raport serio musi zawierać z jakiego kraju pochodzi gość? xd Wiem, nie doprecyzowałem - ale mogłeś zapytać xD

4. ++ za comparisonChain good job!

5. EmployeeFactory -> ziom, przy każdym wywołaniu metody getName tworzysz tablicę z 10 imionami. Wyrzuć to do pola i stworzysz ją raz xd
   Posprzątać sprawę z builderami i mi się podoba :)