Wielka prośba o codereview

0

Właśnie skończyłem technikum i zamierzam iść na studia. Od pewnego czasu programuje w Javie, a w najbliższym czasie chciałbym się dostać do pierwszej pracy, najprawdopodobniej jako Junior Java Dev / Staż. Napisałem sobie prosty projekt uproszczonego blackjack'a starając się w przy tym trzymać wszelkich zasad clean code itd.

Chciałbym jednak wiedzieć czy robię jakieś straszne błędy bądź stosuje jakieś złe praktyki no i ogólnie fajnie byłoby wiedzieć jak wy to widzicie

Link do projektu: https://github.com/Videso/blackjack

0

spaghetti code

0
Młody Byk napisał(a):

spaghetti code

  • nie kompiluje sie
0

Mógłbyś powiedzieć, co się dzieje, jakie błędy są? Sam sprawdzałem i musze powiedzieć, że 'u mnie działa'.

W projekcie używałem po raz pierwsze Gradle'a. Ja uruchamiam projekt za pomocą terminala (gradle run), bardzo możliwe, że gdy ty zainstalowanego Gradle'a nie masz to będziesz musiał wpisać "./gradlew run".

0

Odkrywam rekurencję :D

Jeśli chodzi o code review na szybko, to:

  1. https://github.com/Videso/blackjack/blob/master/src/main/java/org/bitbucket/videso/model/StatisticsTemplate.java - metoda prepareCards ma w sobie jakąś logikę, mimo że jest w modelu. W modelu nie powinno być zbytniej logiki
  2. https://github.com/Videso/blackjack/blob/master/src/main/java/org/bitbucket/videso/service/impl/BlackjackPointsCalculatorImpl.java - można wydzielić parę prywatnych metod, żeby nie było tylu zagnieżdżeń
0
Pinek napisał(a):
  1. https://github.com/Videso/blackjack/blob/master/src/main/java/org/bitbucket/videso/model/StatisticsTemplate.java - metoda prepareCards ma w sobie jakąś logikę, mimo że jest w modelu. W modelu nie powinno być zbytniej logiki

NIe zgadzam się.

  1. https://github.com/Videso/blackjack/blob/master/src/main/java/org/bitbucket/videso/service/impl/BlackjackPointsCalculatorImpl.java - można wydzielić parę prywatnych metod, żeby nie było tylu zagnieżdżeń

Nie zgadzam się. To jest akurat miejsce na folda (czyli stream.reduce) (być może warto wydzielić logikę dodawania).

Ogólnie to wygląda na kod seniora przygotowującego szkolenie z Javy/ Testowania/ Guice, a nie kod Juniora: Od pewnego czasu programuje w Javie. Jest bardzo dobrze. Można by się czepiać, tak jak do każdego nietrywialnego kodu da się czepić, ale bez przesady - jest git. Testy też wygladają na sensowne.

Na koniec najważniejsze:

10 lines of code = 10 issues.

500 lines of code = "looks fine."

Code reviews.

źródło: https://twitter.com/iamdevloper/status/397664295875805184

EDIT:
Jeszcze drobne uwagi

@Getter
@Setter
public class Player {

Na kiego grzyba te gettery i settery w Playerze? Można playerowi zmienić "punkty" bez dawania kart do ręki? Chcemy tego?

0

Bardzo dziękuje za wszelkie uwagi.

Co do nich:

  • StatisticsTemplate.class - Miałem tutaj rozterki co do umieszczania logiki. Wiem, że projektując typową 'warstwową' aplikację webową model powinien odzwierciedlać encje w bazie, do tego jakieś DTO, aby nie przesyłać zbędnych pól itd. Ale doszedłem do wniosku, że umieszczenie prostej metody update'ującej punkty przy dodaniu nowej karty jest jak najbardziej w porządku. Za pierwszym razem nie było w tej klasie żadnej logiki, ale zauważyłem, że poprzez pobieranie kart i wstrzykiwanie w każdym miejscu strategii liczenia kart gdzie akurat jest to potrzebne, a następnie set'towanie otrzymanego wyniku.. robi się okropny syf.

  • BlackjackPointsCalculatorImpl.class - rzeczywiście dałoby się bardziej to rozbić, aby było bardziej czytelne. Zostawiłem to w ten sposób, ponieważ SonarLint przestał się czepiać po zmianie 'pierwszej' wersji. (na początku było (OKROPNE) jedno zagnieżdzenie więcej i krzyczał, że tak nie wolno :) ). Do poprawy.

  • Player.java - racja, nie powinienem był wrzucać Setterów na wszystkie pola, mój błąd.

Ogólnie to wygląda na kod seniora przygotowującego szkolenie z Javy/ Testowania/ Guice, a nie kod Juniora: Od pewnego czasu programuje w Javie. Jest bardzo dobrze. Można by się czepiać, tak jak do każdego nietrywialnego kodu da się czepić, ale bez przesady - jest git. Testy też wygladają na sensowne.

Bardzo dobrze słyszeć, że kod się dobrze prezentuje. Co do "od pewnego czasu", to będą to jakieś 2 lata z przerwami z czego ostatni okres bardzo nie wiele pisałem, z powodu przygotowań do matury. Z tego też powodu ten projekt miał za zadanie przypomnieć sobie podstawowe rzeczy.

Dużo się uczyłem na temat wszelkiego rodzaju czystego kodu, dobrych praktyk itp., nie mam jednak prawie żadnego doświadczenia komercyjnego, nie jestem studentem (jeszcze) i osobiście uważam, że może być u mnie średnio z czystą wiedzą teoretyczną (a'ka akademicką) gdyż jestem głównie praktykiem i samoukiem.

Chciałbym się jeszcze zapytać jak to jest z tą znajomością technologii. Póki co miałem okazje pisać w: HTML, CSS, JS, Angularjs, Java , Spring, QML, w tym różne biblioteki/tool'e np. Gulp, npm, git, junit, mockito, assertj, guice, lombok, hibernate ......

W czym rzecz... osobiście uważam, że ciężko jest powiedzieć coś w stylu "Umiem Hibernate/Umiem Springa", bo zawsze się znajdzie osoba/rekruter/ktoś która udowodni nam, że tak nie jest. Czy to jakieś dzikie configi w tej technologii o których najstarsi górale nie śnili, czy adnotacje, klasy itd.
Jak dużo powinienem umieć na poziom takiego Juniora z np. Springa?

Jak się za niego brałem, nie miałem większych problemów tzn. umiem zrobić endpointy, DI/IoC też nie jest zbytnio skomplikowane, ale jak zauważyłem jak wygląda np. konfiguracja systemu logowania (spring security), gotowe springowe obiekty itp. to już sam osobiście wolałem od zera napisać najprostszy system zarządzania logowaniem (tokeny).

Wydaje mi się, że nie mogę napisać w CV, że umiem Springa jeżeli nie znam typowo springowych klas. Podobna sytuacja dotyczy takiego Hibernate'a w którym też można kopać bez końca.
No chyba, że miałbym zaznaczyć w cv/portfolio, co dokładnie umiem z danej technologii.

Co uważacie?
No i jestem ciekaw, czego od nowego pracownika Junior/Intern/Mid byście oczekiwali, na jakie umiejętności(technologie i co dokładnie) stawialibyście nacisk, wiadomo.. zależy od projektu, ale bardziej chodzi tak ogólnie.

Edit: literówka x2

0

W czym rzecz... osobiście uważam, że ciężko jest powiedzieć coś w stylu "Umiem Hibernate/Umiem Springa", bo zawsze się znajdzie osoba/rekruter/ktoś która udowodni nam, że tak nie jest. Czy to jakieś dzikie configi w tej technologii o których najstarsi górale nie śnili, czy adnotacje, klasy itd.
Jak dużo powinienem umieć na poziom takiego Juniora z np. Springa?

DI, Spring MVC, Data JPA, trochę Security - w skrócie - zrób na początek bibliotekę z rejestracją i logowaniem, no i logiką typową dla biblioteki ;) Twój kodzik jest fajny. Moim zdaniem już nadajesz się na Juniora ;)
Mądrze prawisz. Hibernate i Spring to dość magiczne frameworki więc absolutnie nie przejmuj się, że nie poznasz ich w stopniu bardzo dobrym. Ba, dobra znajomość to już wyczyn. Ale wiadomo jak to w życiu - dla niektórych HRów warto wpisać Spring 4/5 gwiazdek lub coś w tym stylu.

Jak zaznaczyć w CV znajomość Springa ? Ja np. po prostu wymieniłem moduły Springa z jakimi coś robiłem. W ostateczności możesz wpisać - podstawy Springa, i wymienić moduły konkretne.

Ogólnie na co kładą nacisk ? Java i logiczne myślenie. Reszta to kwestia douczenia.

Ogólnie to powodzenia ;) Sensownie piszesz i wydaje się, że poradzisz sobie ;)

0

Unikałbym stosowania interfejsów, gdzie masz jedną implementację z taką samą nazwą jak interfejs i sufiksem impl. IMHO jeżeli coś ma robić konkretną rzecz (domena, logika, biznes, jak zwał tak zwał) i nie dotyka to I/O to nie ma sensu stosowania interfejsu, chyba że jest kilka implementacji, ale wtedy warto byłoby nazwać ten jedyną implementację, tak żeby miało to jakiś sens dla osoby czytającej.

https://github.com/Videso/blackjack/blob/master/src/main/java/org/bitbucket/videso/service/impl/BlackjackPointsCalculatorImpl.java - prywatne metody ułatwiłyby czytanie kodu, np.
card.getRank().equalsIgnoreCase("ace") -> isAce(card) lub card.isAce()

Co do ogólnej jakości kodu to zgodze się z jarkiem

à propos co do pytania czego oczekiwać - jeżeli ma jakieś pojęcie o algorytmach/strukturach danych to na pewno duży nie plus i nie mam na myśli potrafienia zaimplementować na kartce czy w ogóle z pamięci, a ogółem też myśle, ze jest bardzo dobrze i długo pracy szukać nie bedziesz i celuj najlepiej w dobre i bardzo dobre firmy

0

Bardzo dobry kod. Gdybym miał się do czegoś przyczepić to bym się przyczepił, ale na Twoim etapie kariery nie ma sensu. Patrząc na to jak programujesz sam zaczniesz wyłapywać to, co mogłoby być lepiej napisane.

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