Prosty program licznika

Question

To mój pierwszy program w Javascript. Czy jest jakiś brak lub coś do poprawienia? Po prostu zwiększa, zmniejsza lub resetuje licznik za pomocą przycisków lub klawiszy strzałek.

HTML,

<h1 id="number"></h1>
<button class="btn" id="incr">Increase</button>
<button class="btn" id="reset">Reset</button>
<button class="btn" id="decr">Decrease</button>

JS,

let number = 0

const getNumber = document.getElementById(`number`)
const incrButton = document.getElementById(`incr`)
const decrButton = document.getElementById(`decr`)
const resetButton = document.getElementById(`reset`)

getNumber.textContent = number

const incrFunc = function () {
  ++number
  getNumber.textContent = number
  if (number > 0) {
    getNumber.style.color = `green`
  }
}

const resetFunc = function () {
  number = 0
  getNumber.textContent = number
  getNumber.style.color = `gray`
}

const decrFunc = function () {
  --number
  getNumber.textContent = number
  if (number < 0) {
    getNumber.style.color = `red`
  }
}

incrButton.addEventListener(`keyup`, function (e) {
  e.stopPropagation()
  //console.log(e.target === document.body)
  if (e.code === `ArrowUp`) {
    incrFunc()
  }
})

document.addEventListener(`keyup`, function (e) {
  //console.log(e.target === document.body)
  if (e.code === `ArrowUp`) {
    incrFunc()
  }
})

document.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowRight` || e.code === `ArrowLeft`) {
    resetFunc()
  }
})

resetButton.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowRight` || e.code === `ArrowLeft`) {
    resetFunc()
  }
})

document.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowDown`) {
    decrFunc()
  }
})

decr.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowDown`) {
    decrFunc()
  }
})

incrButton.addEventListener(`click`, incrFunc)

resetButton.addEventListener(`click`, resetFunc)

decrButton.addEventListener(`click`, decrFunc)

Answer 1

Moja pierwsza rada to przeformatowanie kodu za pomocą lintera. Nie będę wchodził w to bardziej szczegółowo, ponieważ zakładam, że głównie szukasz porad dotyczących implementacji, ale ponieważ jest to dość irytujące, podobnie jak tekst napisany w twoim własnym języku, ale z obcą wielką literą i interpunkcją.

Niektóre z twoich funkcji mogą korzystać z destrukturyzacji, aby ich ciała były bardziej zwięzłe.

Na przykład to:

document.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowRight` || e.code === `ArrowLeft`) {
    resetFunc()
  }
})

Można to przepisać w ten sposób, ponieważ używasz tylko codewłaściwości argumentu:

document.addEventListener(`keyup`, function ({ code }) {
  if (code === `ArrowRight` || code === `ArrowLeft`) {
    resetFunc()
  }
})

Bardziej kontrowersyjną radą może być napisanie tego na początku scenariusza:

const $ = document.querySelector.bind(document);

Pozwala to zastąpić to:

const getNumber = document.getElementById(`number`)
const incrButton = document.getElementById(`incr`)
const decrButton = document.getElementById(`decr`)
const resetButton = document.getElementById(`reset`)

Z czymś znacznie mniej szczegółowym:

const getNumber = $(`#number`)
const incrButton = $(`#incr`) const decrButton = $(`#decr`)
const resetButton = $(`#reset`)

Answer 2

Powinieneś używać średnika na końcu linii.

Nie ma potrzeby umieszczania zdarzenia keyup na przyciskach, dokument przechwyci zdarzenie. Potrzebujesz także tylko jednego detektora zdarzeń w dokumencie.

document.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowUp`) {
    incrFunc();
  }
  else if (e.code === `ArrowDown`) {
    decrFunc();
  }
  else if (e.code === `ArrowRight` || e.code === `ArrowLeft`) {
    resetFunc();
  }
})

Twoje funkcje zwiększania, zmniejszania i resetowania są na tyle podobne, że można je łączyć, więc nie musisz powtarzać tego samego kodu. W ten sposób możesz również ustawić liczbę na dowolną wartość lub zmienić ją o dowolną kwotę, jeśli będziesz tego potrzebować później.

function setNumber(value) {
  number = value;
  getNumber.textContent = number
  if (number < 0) {
    getNumber.style.color = `red`
  }
  else if (number > 0) {
    getNumber.style.color = `green`
  }
  else {
    getNumber.style.color = `gray`
  }
}

function changeNumber(change) {
    setNumber(number + change);
}

Możesz ustawić detektory zdarzeń za pomocą wstępnie ustawionych argumentów za pomocą bind . Pierwszym argumentem jest ustawienie thissłowa kluczowego, którego nie używamy, więc możemy je po prostu ustawić null.

incr.addEventListener(`click`, changeNumber.bind(null, 1));
reset.addEventListener(`click`, setNumber.bind(null, 0));
decr.addEventListener(`click`, changeNumber.bind(null, -1));

Chociaż zapisywanie elementu w zmiennej, gdy odwołuje się wiele razy, jest dobrą praktyką, nie ma potrzeby, gdy jest używany tylko raz.

Wreszcie, getNumbernie jest to dobra nazwa opisowa.

Ostateczny kod.

const numberDisplay = document.getElementById(`number`);
let number = 0;
setNumber(number);

function setNumber(value) {
    number = value;
    numberDisplay.textContent = number;
    if (number < 0) {
      numberDisplay.style.color = `red`;
    }
    else if (number > 0) {
      numberDisplay.style.color = `green`;
    }
    else {
      numberDisplay.style.color = `gray`;
    }
}

function changeNumber(change) {
    setNumber(number + change);
}

document.addEventListener(`keyup`, function (e) {
  if (e.code === `ArrowUp`) {
    changeNumber(1);
  }
  else if (e.code === `ArrowDown`) {
    changeNumber(-1);
  }
  else if (e.code === `ArrowRight` || e.code === `ArrowLeft`) {
    setNumber(0);
  }
})

document.getElementById(`incr`).addEventListener(`click`, changeNumber.bind(null, 1));
document.getElementById(`reset`).addEventListener(`click`, setNumber.bind(null, 0));
document.getElementById(`decr`).addEventListener(`click`, changeNumber.bind(null, -1));

<h1 id="number"></h1>
<button class="btn" id="incr">Increase</button>
<button class="btn" id="reset">Reset</button>
<button class="btn" id="decr">Decrease</button>

Answer 3

Może mógłbyś zmapować każdy kod klucza za pomocą funkcji i uniknąć wielokrotnego sprawdzania kodu klucza

const map = {
  'ArrowUp': incrFunc,
  'ArrowRight': resetFunc,
  'ArrowLeft': resetFunc,
  'ArrowDown': decrFunc,
}

document.addEventListener(`keyup`, function (e) {
  if(map[e.code]) {
    map[e.code]();
  }
})

Answer 4

Nie używaj ciągów szablonów ( `number`), jeśli w rzeczywistości nie używasz szablonów. Użyj zwykłych pojedynczych lub podwójnych cudzysłowów: 'number'lub "number".