- 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.
Kopiuj
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.
Kopiuj
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?