JavaScript-Projekt-CodeReview

Question

Witam,

Mógłbym prosić o cr, tego skromnego projektu ?

Dodam, że jestem średnio-zaawansowany w js, mimo, że projekt tego nie oddaje (cofam się do podstaw zanim wejdę w React lub Vue)

Code: https://github.com/maciej790/maciej790.github.io

Live: https://maciej790.github.io/

 

Dzięki za wszelkie uwagi :)

Answer 1

Sugestie ode mnie:

• https://github.com/maciej790/maciej790.github.io/blob/main/index.html#L7
  jeśli to nie jest bardzo ważny skrypt (który musi być załadowany na samym początku strony), to umieść go przed </body> lud ustaw atrybut defer, żeby nie blokował parsera DOM → https://addyosmani.com/blog/script-priorities/
• https://github.com/maciej790/maciej790.github.io/blob/main/index.html#L24
  znacznik <h1> na stronie powinien być jeden
• https://github.com/maciej790/maciej790.github.io/blob/main/index.html#L29
  jaką rolę pełni ten formularz, skoro nie ma w nim żadnych kontrolek?
• https://github.com/maciej790/maciej790.github.io/blob/main/js/app.js#L27
  tego typu powtarzalne stringi (lub liczby) warto wrzucić do zmiennej traktowanej jako stałą, żeby w razie zmiany nazwy nie trzeba było robić tego w X miejscach w projekcie a w jednym. Możesz też przy okazji użyć wspomnianych zmiennych w tej tablicy
• https://github.com/maciej790/maciej790.github.io/blob/main/js/app.js#L21
  co tutaj oznacza liczba 3? Dla kogoś, kto spogląda w kod nie znając pełnego kontekstu, taka liczba nic nie mówi. Dlatego warto (analogicznie do poprzedniej rady) umieszczać takie "magiczne" wartości w zmiennych o czytelnej nazwie
• https://github.com/maciej790/maciej790.github.io/blob/main/js/app.js#L27
  tego typu komentarze są zbędne, bo poniższy if wyraźnie pokazuje co się dzieje. Jeśli już dodajesz komentarz niezwiązany z metadanymi np. opisujący nagłówek funkcji, to komentuj kod wtedy, gdy jego implementacja jest na tyle skomplikowana, że mimo jego (subiektywnie) dobrej czytelności może być problem z jego zrozumieniem
• https://github.com/maciej790/maciej790.github.io/blob/main/js/app.js#L31
  połowa zawartości tego stringa jest powtarzalna w całym skrypcie, więc warto stworzyć funkcję pomocniczą, która będzie zwracać tekst w zależności od przekazanych parametrów
• https://github.com/maciej790/maciej790.github.io/blob/main/js/app.js#L77
  jeśli dobrze widzę, to tutaj nie umieszczasz kodu HTML a czysty tekst (liczby również są traktowane jako tekst w DOM-ie), więc lepiej użyć textContent lub innerText zamiast innerHTML
• https://github.com/maciej790/maciej790.github.io/blob/main/js/app.js#L82
  w takim przypadku wystarczy zapisać button.addEventListener("click", startGame); a argument zdarzenia zostanie przekazany automatycznie do funkcji startGame
• co do CSS'ów:
  staraj się nie stylować po tagach (ani tym bardziej pod id - akurat tego nie robisz, ale warto o tym wspomnieć przy okazji), lecz po klasach oraz - sugestia podoba do "magicznych wartości" w JS - jeśli dana wartość, np. 20px, powtarza się, to stwórz zmienną SCSS/CSS o czytelnej nazwie i jej reużywaj

Comments

Dzięki, za feedback.

Czy możesz może polecić jakiś jeszcze projekt do zrobienia tak na moim poziomie :) , czyli taki nie za trudny ?

---

W Google jest mnóstwo stron z pomysłami na projekty. Można też szukać na GitHub, np. w tym repo jest sporo pomysłów (łącznie z przykładowymi ficzerami danej aplikacji) podzielonych na trzy poziomy trudności.

https://github.com/florinpop17/app-ideas