Proste API z cytatami motywacyjnymi - code review

Question

Witam!

Od niedawna stawiam pierwsze kroki w szeroko pojętym web development'cie (obecnie eksploruję zarówno frontend jak i backend). Napisałem mój pierwszy backend'owy projekcik w Node.js z użyciem Express'a i PostgreSQL. Jest to małe publiczne API, które daje dostęp do ponad pięciuset różnych inspiracyjnych/motywacyjnych cytatów. Jeśli ktoś ma chwilę i chęć (kod jest naprawdę mały, więc nie powinno to pochłonąć za dużo czasu) to prosiłbym o ocenę projektu i wskazówki odnośnie funkcjonalności, struktury kodu i bezpieczeństwa. Traktuję ten post jako walkę z moją małą fobią przed code review ;)

Link do projektu na githubie: https://github.com/TheMadMike/MQuotesAPI

Z góry dzięki! :)

 

Answer 1

Fajnie, że wystawiasz coś do CR. Ja zawsze miałem obawy przed dużą liczbą komentarzy/dyskusjami, ale prawda jest tak, że im jest ich więcej, tym więcej się nauczysz.

1. Jeśli to gotowy projekt, to warto w README.md zawrzeć informację o tym jak go odpalić, oraz jakiego softu wymagasz, szczególnie że nie użyłeś vagranta/dockera.
2.  za docs. Oczywiście to mały projekt ale warto rzucić okiem na Swaggera i OpenApi. 
3. no test specified 
4. Warto rozdzielić aplikację i jej start, łatwiej potem testować.
5. Osobiście umieściłbym łączenie bazy danych w innym miejscu. Np przed wystartowaniem expressa.
6. Nie bawiłbym się w takie zwracanie cytatu codziennego. Dlaczego? Założenie powinno być takie że jeśli wyłączysz i włączysz to powinna działać tak samo. U Ciebie tak nie będzie. Stąd pomysł ogarnięcia algorytmu który będzie bazował tylko na dacie oraz liczbie cytatów w bazie.
7. Chyba nie ma obsługi błędów. Od siebie gorąco polecam ten artykuł. Dla async request handlerow warto użyć wrappera.
8. Ogólnie czemu nie typescript? dostaniesz typy importy i wiele innych fajnych rzeczy.
9. Warto ogarnąć coś do walidacji na przyszłość, korzystałem z express validatora, chociaż widziałem różne opinie.
10. Dotnev do instalacji
11. Jeśli nie używasz dockera warto pomyśleć o pm2. Ale używaj dockera ​​​​​​
12. Popatrz w jaki sposób przekazywać parametry do query w tym libie. Teoretycznie walidacja jest ale nie wiem czy SQLmap by czego nie zwojował. 
13. Ogólnie spoko ale postgres to duża kobyła jak na 500 cytatów. Imo sqlite starczy  potem mniej walki z zestawianiem środowiska.
14. Trochę kiepski Error handling przy robieniu zapytania. Imo lepiej w takim wypadku rzucać http 500.
15. Nie jestem przekonany czy co query musisz robić connect. Do zweryfikowania.
16. Używałbym spójnie Arrow Fn wszędzie. Oczywiście uważamy na this ​​​​​​

Ogólnie spoko. Ogarnij Typescript bo warto. Co do bazy danych na przyszłość orm w stylu Sequelize czy Typeorm. No i generalnie framework w stylu Nest albo hapi, bo express to jak dla mnie biblioteka.

 

A no i testy 

Comments

Super, dzięki za odpowiedź i tyle cennych wskazówek !  

O testach niby coś tam słyszałem, nawet wcześniej w c++ się o nie otarłem, ale nie wiem kiedy i jak je praktycznie stosować. Znasz może jakiś wartościowy artykuł/tutorial (najlepiej anglojęzyczny) na temat testowania i tego kiedy i jak powinno się pisać testy? 

Jeszcze raz, dzięki za odpowiedź 

---

Testy powinno się pisać zawsze. Są oczywiście pewne założenia projektowe z których może wynikać brak testów np. mało czasu do oddania, albo problemy finansowe. Od strony technicznej bo zapewne bardziej to Cię interesuje. Testuje się większość kodu, ale nie:

• Framework - zewnętrzny kod powinien być testowany nie przez nasz projekt
• Język - 2+2 to raczej będzie 5
• Kod trywialny - nie mówię o prostych algorytmach, ale np. Settery. Z testami setterów się nie spotkałem.

Będzie Ci potrzebny Jest no i zapewne Sinon. Pod expressa korzystałem z powodzeniem z supertesta. Chai do asercji.

https://link.medium.com/Ul65vYJtE6 wygląda spoko.