Wątek przeniesiony 2016-03-06 18:17 z Newbie przez furious programming.

Proszę o ocene

0

Witam

Proszę o ocenę pod względem dobrych praktyk / czytelności kodu. Zawsze mam z tym problem więc chciałem napisać coś "nieco" większego żeby ktoś mógł to ocenić.
Napisałem gre "Snake" używając canvas i javascript. Możemy wybierać sobie poziom szybkości i opcje przechodzenia przez ściany :
Link : http://layoutpawelp.comuf.com/snake/

Czekam na wszelkie słowa krytyki :)

PS: Sterowanie wsadem i strzałkami

0
  1. Dziwne formatowanie kodu.
  2. Brak komentarzy (JSDoc).
  3. Meal.checkMeal? checkCollisionWithSnake od razu mówi, co ta metoda robi, a checkMeal nic nie daje do zrozumienia.
  4. 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?

0

Co do punktu 13 : Jak można inaczej zobaczyć kiedy gra się skończy?

0

W event listenerze chyba zapomniałeś o else if (jeśli już zostawić instrukcje warunkową). Poza tym nie powinieneś pchać logiki do event listenera, powinien on przekazać sterowanie do odrębnej metody, np:

window.addEventListener('keyup', function (e) {
		changeSnakeDirection(e.keyCode);
    });

No i 'keyup' w grze zręcznościowej to chybiony pomysł - zamień na keydown.

0

Jak można inaczej zobaczyć kiedy gra się skończy?
No wystarczy przekleić ten kod, który aktualnie masz w callbacku, do miejsca wywołania callbacka - albo lepiej: od razu wydzielić do osobnej metody, ale nie bawić się w funkcje anonimowe w takim błahym przypadku.

1

Uwagi odnośnie samej gry.

  1. Masz przycisk Nowa gra, ale już poziomy szybkości Slow, Medium i Fast. Zdecyduj się na polski lub angielski.
  2. Poziom szybkości Fast jest dla mnie odrobinę za szybki, a poziom Medium jest ślimaczy i nie daje mi przyjemności z gry.

1 użytkowników online, w tym zalogowanych: 0, gości: 1