Prośba o sugestie i sprawdzenie kodu w javie.

Ave!
Chciałbym się poradzić, co można zmienić, ulepszyć w programie, który mam napisać jako swego rodzaju praca domowa.

Wymagania:
Program ma naliczać wynagrodzenie dla pracowników szkoły.
Założenia:
· Program nie łączy się z bazą danych, wszystkie niezbędne dane są dostarczane w metodzie main
· Przechowujemy o każdym pracowniku: imię, nazwisko, pesel, typ zatrudnienia (etat, fakturowiec, outsourcer)
· Każdy pracownik ma inną stawkę wynagrodzenia
o Etatowcy mają wynagrodzenie zależne od stanowiska (sprzątaczka, dyrektor, nauczyciel, itd…) zmodyfikowaną o wypracowane lata (tabela mnożników, lub dla uproszczenia x za każdy rok)
o Fakturowcy i outsourcer-rzy mają stawkę godzinową indywidualną dla każdego pracownika

Kod:
https://github.com/piszu/School

Znane błędy:

• złe nazewnictwo interfejsu getSalary

Z góry dziękuję za konstruktywne uwagi i wskazówki :)

Jak dla mnie to książkowy przykład na dziedziczenie. Tworzysz sobie hierarchię pracowników, gdzie każda klasa nadpisuje konkretną metodę (wynagrodzenie np.). Do tego wprowadzasz te wszystkie dodatkowe wymagania, które podane są w treści.

A co do samego kodu:

• staraj się używać modyfikatorów przy polach w klasie
• jeżeli z góry znasz wartość jakiegoś pola (np. wynagrodzenie dla nauczyciela), które się nie zmienia, to deklaruj takie pole jako final
• tą drabinkę if'ów w jednej klasie zastąp czymś ładniejszym - switch będzie czytelniejszy, jakiś drobny polimorfizm będzie poprawny programistycznie.

w Worker.java - po co Ci settery i gettery jak pola do których przez nie się dostajesz są publiczne, bez sensu... to ma być prywatne. To samo w enumach(Position), więcej prywatnosci.

Ale widzę biznesowy potencjał :)

https://github.com/piszu/School/blob/master/School/src/pl/piszu/tests/TestWorker.java > WTF? po co testujesz mechanizmy języka (g/s testuje się za pomocą automatów by Cobertura/Sonar się na zielono świecił)

https://github.com/piszu/School/blob/master/School/src/pl/piszu/worker/Person.java id - PESEL, a pole typu String. Lepiej utworzyć klasę PESEL (podpieprzyć skądś + info copyleft) z walidatorem itp. duperelami.

https://github.com/piszu/School/blob/master/School/src/pl/piszu/main/EmploymentType.java to jest ładne.
https://github.com/piszu/School/blob/master/School/src/pl/piszu/main/Position.java a to już nie do końca (błędy zaokrągleń jak tego użyjesz do liczenia pensji!!!)
https://github.com/piszu/School/blob/master/School/src/pl/piszu/main/SalaryBonus.java metoda fromInteger WTF?!? drabinka ifów w tym miejscu to zło w czystej postaci. Do zmiany - przenieść to do enumów jako metodę isInRange
https://github.com/piszu/School/blob/master/School/src/pl/piszu/main/SalaryCounter.java skoomplikowany ten kod... podziel na metody.
https://github.com/piszu/School/blob/master/School/src/pl/piszu/main/SalaryInterface.java znowu double do pieniędzy... Deutsche Bank kiedyś na tym poległ ;)

Swoją droga GitHub powinien mieć mechanizm codereview... szkoda, że nie ma.

Github ma code review tylko dla pull requestów/wspólnych repo. Co boli.

BTW. robienie cr przez forum to jakiś piekielny pomysł.

Nazywaj commity jak człowiek, a nie Commit 1, Commit 2. Po co Ci to? I tak jak będziesz miał dużo gałęzi to wyjdą dziwne rzeczy potem. Dawaj tam opis tego co zrobiłeś oraz co jest jeszcze do zrobienia.