- Dziwne formatowanie kodu.
- Brak komentarzy (JSDoc).
-
Meal.checkMeal
? checkCollisionWithSnake
od razu mówi, co ta metoda robi, a checkMeal
nic nie daje do zrozumienia.
- Raczej nie pisz
canvasWidth
/canvasHeight
, tylko pójdź poziom abstrakcji wyżej i zrób sobie klasę Map
z polami width
oraz height
, i tę klasę wstrzykuj.
switch(this.snakeDirection){
case DIRECTION.LEFT:
newX--;
break;
case DIRECTION.RIGHT:
newX++;
break;
case DIRECTION.UP:
newY--;
break;
case DIRECTION.DOWN:
newY++;
break;
}
Tak w ramach zadania: napisz to bez wykorzystania jakiejkolwiek instrukcji warunkowej ;-)
6. Musiałem 20 sekund zastanawiać się, co robi metoda Snake.checkWalls
. Żyjemy w XXI wieku, istnieją terabajtowe dyski, więc nie oszczędzaj na nazwach metod. checkCollisionWithBoundaryWalls
może i wygląda przydługawo, ale za to nie trzeba zerkać do kodu, aby wiedzieć, co ta metoda robi.
7.
if (head.x >= width || head.x < 0 || head.y >= height || head.y < 0){
return true;
}
return false;
Nah, nah. return (head.x >= width || ...);
.
8. Wciąż nie czaję, co robi metoda Snake.checkSnakeBody
.
9. 0 != i
warunki w stylu yody są dobre dla początkujących (prościej zauważyć pomyłkę =
/==
), lecz nie powinieneś ich wykorzystywać - pisz i != 0
.
10. Pozbądź się luźnych funkcji - nie clear
, tylko CanvasHelper.clear
; nie startGame
, tylko Game.start
(...).
11. window.addEventListener('keyup', function (e) {
to w końcu wykorzystujesz to jQuery czy nie?
12. Imho nazwa Meal
nie pasuje - prędzej Food
.
13. A to, że startGame
jako callback przyjmuje funkcję uruchamianą podczas przegrania, to już w ogóle jakieś mistrzostwo. Dlaczego callback? Czy kiedykolwiek będzie potrzeba przekazania tam innej funkcji, niż ta aktualna?