Problem
So I’m very new if JavaScript and my question is this a right way to implement this functionality? How could I improve this code.
<!DOCTYPE html>
<html lang="en">
<head>
<title>TEST</title>
<script>
document.addEventListener('DOMContentLoaded', function() {
let correct = document.querySelector('.correct');
correct.addEventListener('click', function() {
correct.style.backgroundColor = 'green';
document.querySelector('#A1').innerHTML = 'Правильно';
});
let incorrects = document.querySelectorAll('.incorrect');
for (let i = 0; i < incorrects.length; i++) {
incorrects[i].addEventListener('click', function() {
incorrects[i].style.backgroundColor = 'red';
document.querySelector('#A1').innerHTML = 'Неправильно';
});
}
document.querySelector('.check').addEventListener('click', function() {
let input = document.querySelector('input');
if (input.value === '17') {
input.style.backgroundColor = 'green';
document.querySelector('#A2').innerHTML = 'Правильно';
} else {
input.style.backgroundColor = 'red';
document.querySelector('#A2').innerHTML = 'Неправильно';
}
});
});
</script>
</head>
<body>
<h1>TEST</h1>
<section>
<article>
<hr>
<h3>How many times can you subtract 6 from 30?</h3>
<button class="incorrect">ДВА РАЗA</button>
<button class="incorrect">ЧЕТЫРЕ РАЗA</button>
<button class="incorrect">ТРИ РАЗA</button>
<button class="incorrect">ШЕСТЬ РАЗ</button>
<button class="correct">ПЯТЬ РАЗ</button>
<p id="A1">Сколько раз вы можете вычесть 6 из 30?</p>
</article>
<article>
<hr>
<h3>What number should be subtracted from each of 50, 61, 92, 117 so that the numbers, so obtained in this order, are in proportion ?</h3>
<input type="text"></input>
<button class="check">Check</button>
<p id="A2">Какое число нужно вычесть из 50, 61, 92, 117, чтобы числа, полученные в таком порядке, были пропорциональны?</p>
</article>
</section>
</body>
</html>
Solution
Congrats on getting something like this working! I obviously don’t know anything about the background of the project, so keep in mind that while my feedback may be valuable, if it’s just a small side project you might not want to overengineer it.
Regardless, here are some points for you to regard:
-
External scripts: While embedding scripts into the document directly is a slight performance gain, using an external script gives you the luxury of
-
Mixed languages: While mostly an oddity while reading, it’s generally a bad user experience to have two languages mixed together. For concistency in terms of UX, I’d advise that you stick to either one.
-
Reducing duplicated code: When writing code, it’s always nice that if you have to change something, you only have to change it in one place, instead of multiple. This makes your code less error prone to spontaneous changes made by you. Considering this, we could refactor the following:
let input = document.querySelector('input');
if (input.value === '17') {
input.style.backgroundColor = 'green';
document.querySelector('#A2').innerHTML = 'Правильно';
} else {
input.style.backgroundColor = 'red';
document.querySelector('#A2').innerHTML = 'Неправильно';
}
… into …
const input = document.querySelector('input');
const answer = document.querySelector('#A2');
if (input.value === '17') {
input.style.backgroundColor = 'green';
answer.innerHTML = 'Правильно';
} else {
input.style.backgroundColor = 'red';
answer.innerHTML = 'Неправильно';
}
- HTML cleanup: If you need to query the document to fetch elements, it’s always good to be as explicit as possible. On one line you are querying for
input, but what if the page contains multiple input elements later? You should try to get a consistent and explicit HTML structure going for the questions/answers (which also ties in with the point made later on below). An example could be this:
<article id="q1" class="question">
<hr>
<h3>...</h3>
<input type="text"></input>
<button class="check">Check</button>
<p class="answer">...</p>
</article>
Each of the elements in this node can be uniquely identified simply by the fact that they are a child of the unique(!) question element with the ID “q1”. Queries that looked for input before would now look for #q1 > input instead.
You could also try to remove redundant classes, unless they are used for styling. In the first question, there is one button with the “correct” class, with all others being “incorrect”. But the fact that every button that is not explicitely “correct” should be incorrect should be natural, hence you could consider removing these excessive classes.
The points mentioned above will probably increase the overall readability and maintainability of the project, but the biggest concern currently is the extendibility.
Look at it this way: Currently, every time you want to add a new question to the quiz, you need to modify your source code, mostly adding duplicated statements to check correct/incorrect answers. If you give your questions a more concrete structure, you could write reusable code, that is valid for any question that you add!
The exact implementation for this is up to you, but it could help to generate an object from an HTML node, grabbing the question title, description, answering possibilities (and the correct one obviously!) and setting up the question/answer handling.
Always remember: It’s almost always better to have something that is extendible and/or scalable without having to modify the source code, just by modifying the markup or configuration.