Prosta gierka korzystająca z JS i PHP - prośba o feedback

pytanie zadane 16 stycznia 2018 w Nasze projekty przez Gambr Dyskutant (7,530 p.)
otwarte ponownie 16 stycznia 2018 przez Gambr

Cześć. Stworzyłem prostą grę dostępną tutaj. Dziwna nazwa domeny, ale została jeszcze po konkursie historycznym, więc trzymam na niej takie luźne rzeczy :D. Na stronie znajdziecie opis zasad oraz link do GitHuba z aktualną wersją kodu. Jak oceniacie mój projekt? Nie chodzi mi o pomysł, bo wiem, że jest bardzo prosty i nie dostarcza żadnej wartości biznesowej, ale bardziej o sam kod i jego jakość.

Dzięki za odpowiedzi

2 odpowiedzi

odpowiedź 16 stycznia 2018 przez Arkadiusz Waluk Ekspert (287,950 p.)

```
<http-equiv="X-UA-Compatible" content="IE=edge,chrome=1" />
```
Coś takiego nie istnieje, to powinna być meta. I chrome=1 tu niepotrzebne.
HTML pomieszany z PHP (logika, mechanizmy z widokiem), robi się nieczytelnie. Lepiej byłoby oddzielić kod HTML od całej mechaniki, a najlepiej użyć systemu szablonów np. Twig.
W wielu miejscach HTMLa style inline, raczej wypadałoby ich unikać.
```
onclick="showInfo();"
```
To nie jest dobry sposób podpięcia zdarzenia.
Inputom gdzieś się labele pogubiły.
Nie stylizujemy po id i raczej nie po znacznikach, najlepiej po klasach.
Wszędzie pomieszane języki - nazewnictwo zmiennych/funkcji/plików/identyfikatorów raz po polsku, raz po angielsku. Lepiej oczywiście nazywać tylko po angielsku.
Nieprzestrzegane zasady wyglądu kodu PHP PSR-2 - nie są to zasady oficjalne, ale na tyle popularne, że znaczna większość ich używa, ze swojej strony też polecam się zainteresować.
```
echo "<span class='secret' id=secret".$i.">".strtoupper($element['slowo'])."</span>";
```
To żaden błąd, ale osobiście nie widzę sensu używania " gdy nie podstawiamy nic do stringa (tu i tak robisz konkatenacje), można spokojnie użyć '.
```
$result = $connection->query("SELECT * FROM USERS WHERE nick='$_POST[nick]'");
```
Tu będzie idealny atak sql injection, radzę szybko to zabezpieczyć bo jeszcze znajdzie się średnio rozgarnięta osoba, która go wykona.
```
$requestedAction = $_POST['typeAcces'];
```
Przed odczytaniem tego elementu z $_POST wypadałoby sprawdzić czy istnieje (isset() / ??) lub skorzystać np. z filter_input(), w przeciwnym wypadku otrzymasz ostrzeżenie (gdy ktoś nie prześle tego pola).
```
 $scores =  array();
```
Wystarczy $scores = []; - jak dla mnie krócej i czytelniej.
https://github.com/LeonCzerwinski/typingpractice/blob/master/scores.php#L21 tego fragmentu kodu nie rozumiem, jakieś dziwne rzeczy tu są z tymi pętlami.

komentarz 16 stycznia 2018 przez Gambr Dyskutant (7,530 p.)

Dzięki za odpowiedź :) Spodziewałem się ściany błędów bo w webie jestem totalną zielonką. Co do wspomnianego fragmentu, po wyjęciu 3 największych wyników z bazy. iIeruję przez niech foreachem i jeżeli są puste (a może tak być bo moja "gra" ma niewielu użytkowników) to do tablicy scores i nicks wkładam puste napisy a w przeciwnym wypadku dane wyjęte z bazy a następnie w spanach wyświetlam zawartość tablic. Jak teraz to opisuje to czuję wstyd bo to rozwiązanie bardziej barbarzyńskie niż programistyczne

komentarz 16 stycznia 2018 przez Gambr Dyskutant (7,530 p.)

Poprawiłem już sporo rzeczy

komentarz 16 stycznia 2018 przez Arkadiusz Waluk Ekspert (287,950 p.)

Rozumiem, ten fragment raczej wygląda na przekombinowany. Co do poprawek tak na szybko tu zauważyłem sprintf() i mysqli_real_escape_string() jest już jakimś zabezpieczeniem, ale jeszcze lepiej byłoby użyć bindowania. Wtedy osobno do bazy idzie zapytanie i osobno podstawiane wartości, nie ma żadnej możliwości wykonania ataku sql injection.

odpowiedź 16 stycznia 2018 przez marcin99b Szeryf (82,180 p.)

Od części wizualnej

Pierwsze co się rzuca to średnio zaprojektowane UI... albo UX, nie znam sie na tym

Chodzi o to, że czasami wyjeżdża w bok za monitor
Czasami napisy są za duże
Ogólnie to napisy dałbym troche mniejsze i popracował troche nad kolorami, aby bardziej rzucało się w oczy

Co do kodu, po zobaczeniu mieszania html z php od razu z tego wyszedłem, skoro znasz styl pisania w php z przed 10 lat, to nie będę liczył na jakiś sensowny kod

W JS kod jakościowo też wygląda na dość średni, np duże ciała if

A po zobaczeniu letter.js to eh... nie prościej było np skorzystać z tablicy ASCII?

Wnioski:

Jeśli jesteś początkującym, fajnie że coś tworzysz, jakoś kombinujesz mieszając php z js, robisz własne projekty a nie jedynie przepisywanie z poradników (sądząc po kodzie to tak jest). Staraj się jednak podszkolić w pisaniu "lepszego kodu", w JS zainteresuj się np ES6 (nowe standardy), możesz też zajrzeć do bibliotek, takich jak jQuery lub React, nawet jeśli ci się nie przydadzą przy codziennej pracy, ucząc się czegoś na wyższym poziomie, uczymy się też sposobu programowania, na wyższym poziomie. Fajnie, gdybyś zajrzał do Angulara, ale to jak już będziesz coś ogarniał.

Jeśli już programujesz jakiś czas, postaraj się postawić większy nacisk na naukę nowych rzeczy. Wiele osób popełnia ten błąd, że po ogarnięciu podstaw stoją w miejscu. Programowanie ma tą wadę, że stosunkowo wcześnie mamy ogromne możliwości - więc wiele osób stwierdzając "poco mam się uczyć więcej, skoro teraz moge zrobić wszystko?" zatrzymuje swoją naukę... i później mamy "programistów" z 4 letnim "doświadczeniem" i wiedzą/umiejętnościami, jak po pierwszym miesiącu nauki

komentarz 16 stycznia 2018 przez Gambr Dyskutant (7,530 p.)
edycja 16 stycznia 2018 przez Gambr

Dzięki za poświęcony czas. Generalnie jestem hobbystą a z webem nie miałem wcześniej styczności a raczej zajmowałem się androidem i javą. Aktualnie czytam książkę "Czysty Kod" Roberta C Martina i może coś z niej wyniosę. Pracując w swoim tempie stworzę jeszczę tonę śmieciowego kodu, aż w końcu coś się przełamie i będzie dobrze - tak myślę