Ocena projektów

Question

Witam,

tak jak w temacie daje do oceny swoje projekty. Liczę na konstruktywną opinię, która pomoże mi poprawić projekty.

Projekty nie są dokończone ale mają już pewną funkcjonalność.

1. Rest Api w Symfony 3.4 - https://github.com/pnkp/orders-php-rest-api
2. SPA w Angularze 6 - https://github.com/pnkp/order-register-spa

Czekam na komentarze :)

Answer 1

1. Nie musisz importować tych wszystkich styli w globalnym styles.css, dodaj je sobie to tablicy styli w pliku konfiguracyjnym angulara angular.json i webpack za Ciebie zrobi odpowiednie buildy css i wrzuci j w templatkę główną.
2. client_delivery_code?: ClientDeliveryCode, w JS raczej jest nazewnictwo clientDeliveryCode

3. date?: string;
   date_stamp?: Date;

   cholernie myli... czym jest date? Wg mnie raczej powinno być pole date?: Date trzymające obiekt daty, ewentualnie trzymająe timestamp. Do tego to pole date_stamp kompletnie nie wiadomo do czego jest i po co...

4. gdzie jest rzecz najważniejsza - unit tests?

5. https://github.com/pnkp/order-register-spa/blob/master/src/app/clients/client-address/client-address.component.ts#L49  metoda createAddress jest zbyt rozbudowana, zbyt wiele warunków itp. nawet jej nie analizuję bo wiem, że jej otestowanie to będzie katorga... rozbij to na mniejsze metody, jakoś racjonalnie popracuj modyfikatorami dostępu i spróbuj napisać do tego unit testy, przy takich funkcjach od razu widać, że nie masz doświadczenia w testowaniu kodu :)

6. Ponad to masz subskrypcje do Observabli ale nie robisz nigdzie unsubscribe... błąd! Albo rób jawnie usubscribe np. w ngOnDeatroy albo tak zmień logikę, by móc skorzystać z async pipe.

7. Nie stosuj zagnieżdżanych subskrypcji... trudno się to testuje... lepiej poczytaj o różnych operatorach rxjs, flatMap, scan, switchMap itp. itd. i generalnie o dobrych praktykach unikania nested subscribe.

8. <button [disabled]="orderBodies.length < 1 || isResponse" class="ml-1"

   są różne szkoły, ja raczej jestem zwolennikiem aby taką logikę przenieść do kontrolera do jakieś metody w stylu sholudDisableSubmitButton(): boolean, ale to tylko przyjęte zasady, żaden błąd.

9. co robi komponent home :) ? (wiem, pozostałość po buildzie bazowym, ale warto oczyścić projekt z takich śmieci).

10. _source: any; - nie po to mamy TS żeby używać "any" :) albo wiesz z czym pracujesz albo nie :)

11. const token: string = TokenStorage.getToken();
    const bearerToken: string = 'Bearer ' + token;

    ten "token" widzę, że używasz tylko tu, więc nie ma sensu tworzyć go w osobnej stałej.

12. this.mobileQuery = media.matchMedia('(max-width: 600px)');

    nie wnikam w to całe mobileQuery bo tym się nie bawiłem ale nie ważne, kilka razy widziałem to max-width jako string - wyciągnij sobie to może do jakiegoś globalnego pliku typu media-constants.ts, będzie Ci łatwiej tym zarządzać.

Generalnie spoko, masz gdzieś może wersję na domenie żeby sobie poklikać? Moim zdaniem jest to dobry poziom na stanowisko juniorskie i z powodzeniem możesz myśleć o takim stanowisku, chyba, że już pracujesz jako programista :)

CSS nie oceniam bo layout to nie moja mocna strona... od tego są UX designerzy :)

Comments

A dzięki za konkretną odpowiedź, jeszcze nie pracuje jako programista :D

CSS nie oceniam bo layout to nie moja mocna strona... od tego są UX designerzy :)

Layout to też nie moja mocna strona, dlatego użyłem Angular Materials i Bootstrapa :)

W sumie jestem totalnym świeżakiem w Angularze i TypeScripcie, stworzyłem ten projekt do mojego API w PHP, żeby sobie poklikać. Programowanie reaktywne to dla mnie też nowość, stąd te błędy.

Wiem, że brakuje testów jednostkowych jaki front-end i back-end, ale ciężko mi się zabrać za temat testów jednostkowych.

Generalnie spoko, masz gdzieś może wersję na domenie żeby sobie poklikać?

Jeszcze nie mam, chociaż jakiś hosting i domenę mam już wykupioną.

Dzięki Twojej opinii na pewno następny projekt, będzie lepszy a w tym projekcie zrobię refaktoryzację. Jeszcze raz dziękuje za poświęcony czas

 

---

Wiem, że brakuje testów jednostkowych jaki front-end i back-end, ale ciężko mi się zabrać za temat testów jednostkowych.

Spoko, to normalne, wiele osób początkujących omija temat testów bo prawda jest taka, że czasami nad testami siedzisz tyle samo czasu co nad stworzeniem jakiegoś ficzera jak nie dłużej :)

Ale przy większych apkach naprawdę docenia się często dobre testy, które sa czasem lepsze niż dokumentacje.

Answer 2

Sf:

1. Formatowanie kodu na bardzo niskim poziomie. Wygląda na to że nie dysponujesz klawiszem enter.
2. Annotations programming. Bardzo nieczytelne wszystko. 
3. Cała logika w kontrolerach. I jak to testować/rozwijać.
4. Brak testów jednostkowych. 
5. W polach niektórych encji notblank, notnull, a setter przyjmuje nullable. 
6. Nie wiem dlaczego klasy event zawierają EntityManager.

Comments

Nie wiem dlaczego klasy event zawierają EntityManager.

Mam pytanie. Czy to jest błąd? 

Dziękuję za poświęcony czas i feedback :) 

---

Event sam w sobie powinien zawierać informacje o tym co się stało.