Einfaches Zählerprogramm
Es ist mein erstes Programm in Javascript. Gibt es einen Mangel oder etwas zu verbessern? Der Zähler wird einfach durch Tasten oder Pfeiltasten vergrößert, verkleinert oder zurückgesetzt.
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)
Antworten
Mein erster Rat wäre, Ihren Code mit einem Linter neu zu formatieren. Ich werde nicht näher darauf eingehen, da ich davon ausgehe, dass Sie hauptsächlich nach Implementierungsratschlägen suchen, aber das Lesen ist ziemlich irritierend, ähnlich wie in Ihrer eigenen Sprache geschriebener Text, jedoch mit ausländischer Groß- und Kleinschreibung und Zeichensetzung.
Einige Ihrer Funktionen könnten eine Destrukturierung verwenden, um ihren Körper enger zu machen.
Zum Beispiel:
document.addEventListener(`keyup`, function (e) {
if (e.code === `ArrowRight` || e.code === `ArrowLeft`) {
resetFunc()
}
})
Könnte wie folgt umgeschrieben werden, da Sie nur die codeEigenschaft des Arguments verwenden:
document.addEventListener(`keyup`, function ({ code }) {
if (code === `ArrowRight` || code === `ArrowLeft`) {
resetFunc()
}
})
Ein kontroverser Ratschlag könnte sein, dies oben in Ihr Skript zu schreiben:
const $ = document.querySelector.bind(document);
Auf diese Weise können Sie Folgendes ersetzen:
const getNumber = document.getElementById(`number`)
const incrButton = document.getElementById(`incr`)
const decrButton = document.getElementById(`decr`)
const resetButton = document.getElementById(`reset`)
Mit etwas viel weniger Ausführlichem:
const getNumber = $(`#number`)
const incrButton = $(`#incr`) const decrButton = $(`#decr`)
const resetButton = $(`#reset`)
Sie sollten ein Semikolon am Zeilenende verwenden.
Auf den Schaltflächen muss kein Keyup-Ereignis vorhanden sein. Das Dokument fängt das Ereignis ab. Sie benötigen außerdem nur einen einzigen Ereignis-Listener für das Dokument.
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();
}
})
Ihre Funktionen zum Erhöhen, Verringern und Zurücksetzen sind so ähnlich, dass sie kombiniert werden können, sodass Sie nicht denselben Code wiederholen müssen. Auf diese Weise können Sie die Zahl auch auf einen beliebigen Wert einstellen oder um einen beliebigen Betrag ändern, wenn Sie dies später benötigen.
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);
}
Sie können die Ereignis-Listener mit voreingestellten Argumenten mit bind festlegen . Das erste Argument besteht darin, das thisSchlüsselwort festzulegen, das wir nicht verwenden, sodass wir es einfach auf festlegen können null.
incr.addEventListener(`click`, changeNumber.bind(null, 1));
reset.addEventListener(`click`, setNumber.bind(null, 0));
decr.addEventListener(`click`, changeNumber.bind(null, -1));
Das Speichern eines Elements in einer Variablen, wenn es mehrmals referenziert wird, ist zwar eine gute Vorgehensweise, es ist jedoch nicht erforderlich, wenn es nur einmal verwendet wird.
Schließlich getNumberist kein guter beschreibender Name.
Der endgültige Code.
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>
Vielleicht könnten Sie jeden Schlüsselcode einer Funktion zuordnen und vermeiden, den Schlüsselcode mehrmals zu überprüfen
const map = {
'ArrowUp': incrFunc,
'ArrowRight': resetFunc,
'ArrowLeft': resetFunc,
'ArrowDown': decrFunc,
}
document.addEventListener(`keyup`, function (e) {
if(map[e.code]) {
map[e.code]();
}
})
Verwenden Sie keine Vorlagenzeichenfolgen ( `number`), wenn Sie keine Vorlagen verwenden. Verwenden Sie normale einfache oder doppelte Anführungszeichen: 'number'oder "number".