• Najnowsze pytania
  • Bez odpowiedzi
  • Zadaj pytanie
  • Kategorie
  • Tagi
  • Zdobyte punkty
  • Ekipa ninja
  • IRC
  • FAQ
  • Regulamin
  • Książki warte uwagi

Aplikacja internetowa - code review

VPS Starter Arubacloud
0 głosów
212 wizyt
pytanie zadane 16 marca 2017 w JavaScript przez ŁukaszD. Użytkownik (540 p.)
Witam

Tworzę aplikację internetową która znajduje się na stronie http://frien.cba.pl/

Na chwilę obecną jest tylko strona startowa.

Zwracam się z prośbą o przegląd kodu(głównie zależy mi o przegląd kodu pod kątem js-a).

Z góry dziękuję za pomoc/oceny/uwagi.

2 odpowiedzi

+2 głosów
odpowiedź 16 marca 2017 przez Tomek Sochacki Ekspert (227,510 p.)

Witam,

jest już trochę późno i już mi się powoli obraz zamazuje :) więc tak tylko na szybko co mi się rzuciło w oczy:

  1. deklarując zmienne nie musisz (a nawet nie zaleca się) powtarzać "var". Wystarczy jedno var i deklarujesz od razu komplet zmiennych, po każdej dając przecinek zamiast średnika (po za oczywiście ostatnią, gdzie ma być średnik).
  2. zanim prześlesz dane na serwer rozważ ich wstępną weryfikację po stronie klienta i w razie czego eleganckie obsłużenie błędu. Na przykład rozważ określenie konkretnych reguł dla hasła (min i max długość, dozwolone i obowiązkowe znaki itp.) i kontroluj wprowadzone hasło zgodnie z tymi regułami. Można to łatwo zweryfikować jednym wyrażeniem regularnym. Podobnie kwestia nicku. Oczywiście pamiętaj, że walidacja w JS u klienta to tylko dodatek, właściwa kontrola musi odbyć się na serwerze zanim zapiszesz dane do bazy.
  3. poczytaj w wolnej chwili o metodzie preventDefault zastępującej return false. Na razie masz krótkie kody funkcji, ale gdy nabiorą one większych gabarytów to zaletą preventDefault jest to, że możesz ją wywołać w dowolnym miejscu, nawet na początku funkcji w przeciwieństwie do return false (możesz więc np. założyć sobie zasadę, że umieszczasz tę metodę na początku funkcji, które mają wstrzymywać akcje domyślne).
  4. polecam rozpoczęcie stosowania trybu 'use strict'. Nie będę tutaj omawiał tego zagadnienia gdyż jest ono dość szerokie, a bez problemu znajdziesz wiele informacji w necie (w razie pytań daj znać co ewentualnie wyjaśnić itp.).
  5. Używając zapisów typu: "if(blad_formularza[0] == true)" uważaj na zasady niejawnej konwersji typów w JS. Nie twierdzę, żebyś zawsze stosował zapis "===" zamiast "==" gdyż oba rozwiązania mają swoje wady i zalety, ale uwazaj, gdyż pusty ciąg znakowy "" zostanie przekonwertowany do false, natomiast każdy ciąg zawierający jakieś znaki, np. "xx" będzie konwertowany na true. Czasami może to rodzić problemy, na przykład każda tablica w JS niejawnie jest konwertowana na true, gdyż tak na prawdę jest to obiekt "[object Array]", a w JS każdy obiekt jest true (nawet pusty, czyli również pusta tablica).

Powiem szczerze, że nie analizowałem czy kod w ogóle będzie poprawnie działać (godzina i umysł już nie do końca zdolny do analizy... :) więc tylko tak na szybko parę moich sugestii rzuciłem.

Pozdrawiam,

Tomek

1
komentarz 17 marca 2017 przez Schizohatter Nałogowiec (39,600 p.)

deklarując zmienne nie musisz (a nawet nie zaleca się) powtarzać "var". Wystarczy jedno var i deklarujesz od razu komplet zmiennych, po każdej dając przecinek zamiast średnika (po za oczywiście ostatnią, gdzie ma być średnik).

Z tym "nie zaleca się" to bym uważał. Dla czytelności niektórzy wolą dać var wszędzie (http://javascript.crockford.com/code.html#variable%20declarations)

 

poczytaj w wolnej chwili o metodzie preventDefault zastępującej return false. Na razie masz krótkie kody funkcji, ale gdy nabiorą one większych gabarytów to zaletą preventDefault jest to, że możesz ją wywołać w dowolnym miejscu, nawet na początku funkcji w przeciwieństwie do return false (możesz więc np. założyć sobie zasadę, że umieszczasz tę metodę na początku funkcji, które mają wstrzymywać akcje domyślne).

.preventDefault() a return to zupełnie różne rzeczy, więc nie rozumiem, jak jedno może zastępować drugie.

 

+1 głos
odpowiedź 17 marca 2017 przez Kamil Naja Nałogowiec (27,330 p.)
W kilku miejscach możesz wykorzystać .toggleClass(), co skróci kod.

Zamień polskie nazwy funkcji/metod na ang.

Niezbyt podoba mi się formatowanie if else - można nad tym dywagować, ale zalecany jest raczej nawias otwierający w tej samej linii kodu, a nie w osobnej. Długie wyrażenie sprawdzające wygląda niepokojąco.
komentarz 17 marca 2017 przez Dash Nałogowiec (29,650 p.)

Jeżeli dobrze myślę o czym mówisz, to wojna pomiędzy : 

if (var) {


}

a 

if (var) 
{

}

Jest podobna do wojny pomiędzy Coca Colą a Pepsi, czy xBoxem a Playstation. Ciężko powiedzieć która strona ma racje, oba zapisy są dobre, nie licząc tego że otwieranie klamry w tej samej linii co napisany if wygląda nieczysto, nieprofesjonalnie i ogólnie autorowi trzeba licencję na programowanie cofnąć ;). 

komentarz 17 marca 2017 przez Kamil Naja Nałogowiec (27,330 p.)
Pozostaje się zgodzić. Chodziło mi jeszcze o to, że w kodzie jest kilka instrukcji if else pod sobą i nie wygląda to zbyt czytelnie - może więcej komentarzy, albo (jeśli się da) może wprowadzić jakieś zagnieżdżenie?
komentarz 17 marca 2017 przez Arkadiusz Fus Obywatel (1,100 p.)
edycja 18 marca 2017 przez Arkadiusz Fus

Totalnie się nie zgodzę, 

W js używa się te zapisu nie bez powodu:

if (var) {
 
 
}

Jest to związane z używaniem return :

return
{
 
}

W tym przypadku nie zadziała i trzeba użyć tego pierwszego. 

Dla mnie używanie tego drugiego sposobu wygląda "nieczysto". 

Kolega niżej wyjaśnił dlaczego to nie zadziała :) 

komentarz 17 marca 2017 przez Kamil Naja Nałogowiec (27,330 p.)
Jeszcze z wytycznych Google

Because of implicit semicolon insertion, always start your curly braces on the same line as whatever they're opening.

Dzięki temu JS nie wstawi "inteligentnie" średnika w pustej linijce.

https://google.github.io/styleguide/javascriptguide.xml

Podobne pytania

+1 głos
2 odpowiedzi 190 wizyt
pytanie zadane 7 lutego 2017 w JavaScript przez szmq Pasjonat (22,770 p.)
+1 głos
1 odpowiedź 231 wizyt
pytanie zadane 7 kwietnia 2021 w JavaScript przez Dani3l Bywalec (2,160 p.)
0 głosów
3 odpowiedzi 570 wizyt
pytanie zadane 13 lutego 2017 w HTML i CSS przez dewe Gaduła (4,300 p.)

92,451 zapytań

141,261 odpowiedzi

319,073 komentarzy

61,853 pasjonatów

Motyw:

Akcja Pajacyk

Pajacyk od wielu lat dożywia dzieci. Pomóż klikając w zielony brzuszek na stronie. Dziękujemy! ♡

Oto polecana książka warta uwagi.
Pełną listę książek znajdziesz tutaj.

Akademia Sekuraka

Akademia Sekuraka 2024 zapewnia dostęp do minimum 15 szkoleń online z bezpieczeństwa IT oraz dostęp także do materiałów z edycji Sekurak Academy z roku 2023!

Przy zakupie możecie skorzystać z kodu: pasja-akademia - użyjcie go w koszyku, a uzyskacie rabat -30% na bilety w wersji "Standard"! Więcej informacji na temat akademii 2024 znajdziecie tutaj. Dziękujemy ekipie Sekuraka za taką fajną zniżkę dla wszystkich Pasjonatów!

Akademia Sekuraka

Niedawno wystartował dodruk tej świetnej, rozchwytywanej książki (około 940 stron). Mamy dla Was kod: pasja (wpiszcie go w koszyku), dzięki któremu otrzymujemy 10% zniżki - dziękujemy zaprzyjaźnionej ekipie Sekuraka za taki bonus dla Pasjonatów! Książka to pierwszy tom z serii o ITsec, który łagodnie wprowadzi w świat bezpieczeństwa IT każdą osobę - warto, polecamy!

...