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.
  5.     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, botów: 0