Programa Contador Simples

Question

É meu primeiro programa em Javascript. Existe alguma falta ou algo a ser melhorado? Ele simplesmente aumenta, diminui ou zera o contador por botões ou teclas de seta.

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

Meu primeiro conselho seria reformatar seu código com um linter. Não vou entrar em mais detalhes porque suponho que você esteja principalmente atrás de conselhos de implementação, mas, como é, é bastante chocante de ler, semelhante ao texto escrito em seu próprio idioma, mas com letras maiúsculas e pontuação estrangeiras.

Algumas de suas funções podem usar a desestruturação para tornar seus corpos mais concisos.

Por exemplo, isso:

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

Poderia ser reescrito assim, já que você está usando apenas a codepropriedade do argumento:

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

Um conselho mais controverso pode ser escrever isso no topo do seu script:

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

Isso permite que você substitua isso:

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

Com algo muito menos detalhado:

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

Answer 2

Você deve usar um ponto e vírgula no final das linhas.

Não há necessidade de ter o evento keyup nos botões, o documento irá capturar o evento. Você também só precisa de um único ouvinte de evento no documento.

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();
  }
})

Suas funções para aumentar, diminuir e redefinir são semelhantes o suficiente para que possam ser combinadas, para que você não precise repetir o mesmo código. Dessa forma, você também pode definir o número para qualquer valor ou alterá-lo por qualquer valor, se precisar disso mais tarde.

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);
}

Você pode definir os ouvintes de eventos com argumentos predefinidos com bind . O primeiro argumento é definir a palavra- thischave, que não usamos, para que possamos apenas defini-la como null.

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

Embora salvar um elemento em uma variável quando ele for referenciado várias vezes seja uma boa prática, não há necessidade de fazê-lo quando for usado apenas uma vez.

Por último, getNumbernão é um bom nome descritivo.

O código definitivo.

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

Talvez você possa mapear cada código de chave com uma função e evitar verificar o keyCode várias vezes

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

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

Answer 4

Não use strings de modelo ( `number`) se você não estiver realmente usando modelos. Use aspas simples ou duplas normais: 'number'ou "number".