Co sądzicie o moim skrypcie Opcje

[Laghnarr]

Witam.

Wczoraj zacząłem bawić się z JavaScriptem i trochę mnie wciągnęło

Zrobiłem coś takiego:

Kod

window.addEventListener('keydown', function(event) {
  switch (event.keyCode) {
    case 37: // Left
      guzik_lewo.style.backgroundColor='red';
          guzik_gora.style.backgroundColor='#DDDDDD';
    guzik_dol.style.backgroundColor='#DDDDDD';
    guzik_prawo.style.backgroundColor='#DDDDDD';
    x.value=x-=10
    break;

    case 38: // Up
      guzik_lewo.style.backgroundColor='#DDDDDD';
    guzik_gora.style.backgroundColor='red';
    guzik_dol.style.backgroundColor='#DDDDDD';
    guzik_prawo.style.backgroundColor='#DDDDDD';
    y.value=y-=10
    break;

    case 39: // Right
    guzik_lewo.style.backgroundColor='#DDDDDD';
    guzik_gora.style.backgroundColor='#DDDDDD';
    guzik_dol.style.backgroundColor='#DDDDDD';
    guzik_prawo.style.backgroundColor='red';
    x.value=x+=10
    break;

    case 40: // Down
    guzik_lewo.style.backgroundColor='#DDDDDD';
    guzik_gora.style.backgroundColor='#DDDDDD';
    guzik_dol.style.backgroundColor='red';
    guzik_prawo.style.backgroundColor='#DDDDDD';
    y.value=y+=10
    break;
  }
}, false);
var guzik_lewo = document.getElementById('Lewo')
var guzik_prawo = document.getElementById('Prawo')
var guzik_gora = document.getElementById('Gora')
var guzik_dol = document.getElementById('Dol')
var x =0;
var y = 0;
function rysuj () {

var canvas = document.getElementById('canvas');
                var canvas = canvas.getContext('2d');
                 canvas.clearRect(0,0,500,500);
var pionek = new Image();
pionek.src = "player.gif"
pionek.onload = function () { canvas.drawImage(pionek, x, y); };
}

var play = setInterval('rysuj()',30);

Skrypt odczytuje czy została wciśnięta jakaś strzałka, jeśli tak to przemieszcza obrazek w odpowiednim kierunku.


Obsługę klawiszy zerżnąłem z innej stronki (nie mogłem sobie z tym poradzić sam) Ale reszta jest moja.
Co o tym sądzicie, co mam źle, czego nie powinienem robić?



Pozdrawiam.

Ten post edytował Laghnarr 15.02.2015, 22:37:02

[PrinceOfPersia]

źle jest, że piszesz 12 razy to samo:

[JAVASCRIPT] pobierz, plaintext 
element.style.backgroundColor='#DDDDDD';
[JAVASCRIPT] pobierz, plaintext 

4 razy to samo:

[JAVASCRIPT] pobierz, plaintext 
element.style.backgroundColor='red';
[JAVASCRIPT] pobierz, plaintext 

A można tak:
http://jsfiddle.net/h5kbe0xn/

(zauważ, że masz do dyspozycji również HTML i CSS, nie musisz wszystkiego w JavaScript pisać).



4 razy też używasz tej samej liczby

[JAVASCRIPT] pobierz, plaintext 
10
[JAVASCRIPT] pobierz, plaintext 

na oznaczenie szybkości poruszania się (a wystarczyło gdzieś wcześniej zrobić coś takiego:

[JAVASCRIPT] pobierz, plaintext 
var speed = 10
[JAVASCRIPT] pobierz, plaintext 

i byś mógł użyć speed, zamiast pisać 10 w 4 miejscach.

Pisząc kod myśl o tym, że będziesz musiał kiedyś go zmodyfikować. A łatwiej jest zmodyfikować kod w jednym miejscu niż w kilkunastu różnych - poczytaj o zasadzie DRY (don't repeat yourself).

[JAVASCRIPT] pobierz, plaintext 
var play = setInterval('rysuj()',30);
[JAVASCRIPT] pobierz, plaintext 

1. nie rysuj() w stringu, bo to nie ma tu najmniejszego sensu, możesz podać funkcję jako argument:

[JAVASCRIPT] pobierz, plaintext 
setInterval(rysuj, 30);
[JAVASCRIPT] pobierz, plaintext 

2. nie 30, tylko prędzej 16 (bo 1000 milisekund / 60 FPS ~ 16)

3. ale najlepiej i tak skorzystać z requestAnimationFrame
https://developer.mozilla.org/en-US/docs/We...tAnimationFrame
żeby przeglądarka mogła poprawnie zoptymalizować wyświetlanie.

Ten post edytował PrinceOfPersia 16.02.2015, 06:24:45