Programa de contador simple

Question

Es mi primer programa en Javascript. ¿Hay alguna carencia o algo a mejorar? Simplemente aumenta, disminuye o restablece el contador mediante botones o teclas de flecha.

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

Mi primer consejo sería reformatear su código con un linter. No entraré en esto con más detalle porque asumo que lo que más busca son consejos de implementación, pero tal como está, es bastante discordante de leer, similar al texto escrito en su propio idioma pero con mayúsculas y puntuación extranjeras.

Algunas de sus funciones podrían usar la desestructuración para hacer que sus cuerpos sean más concisos.

Por ejemplo, esto:

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

Podría reescribirse así, ya que solo está usando la codepropiedad del argumento:

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

Un consejo más controvertido podría ser escribir esto en la parte superior de su guión:

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

Esto le permite reemplazar esto:

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

Con algo mucho menos detallado:

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

Answer 2

Deberías usar un punto y coma al final de las líneas.

No es necesario tener un evento keyup en los botones, el documento captará el evento. También solo necesita un único detector de eventos en el 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();
  }
})

Sus funciones para aumentar, disminuir y restablecer son lo suficientemente similares como para que puedan combinarse, por lo que no tiene que repetir el mismo código. De esta manera, también tiene una forma de establecer el número en cualquier valor o cambiarlo por cualquier cantidad si lo necesita más adelante.

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

Puede configurar los detectores de eventos con argumentos preestablecidos con bind . El primer argumento es establecer la thispalabra clave, que no usamos, por lo que podemos establecerla en null.

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

Si bien guardar un elemento en una variable cuando se hace referencia varias veces es una buena práctica, no es necesario hacerlo cuando solo se usa una vez.

Por último, getNumberno es un buen nombre descriptivo.

El código final.

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

Tal vez podría asignar cada código clave con una función y evitar verificar el código clave varias veces

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

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

Answer 4

No use cadenas de plantillas ( `number`) si no está usando plantillas. Use comillas simples o dobles normales: 'number'o "number".