3
\$\begingroup\$

I've made a quiz where you get a random question, and you have to guess which StackExchange site it's from.

const optionsCount = 6;
const sitesURL = 'https://api.stackexchange.com/2.2/sites?pagesize=999&filter=!SmNU3t0u7kg0gEzR5B';
const questionsURL = 'https://api.stackexchange.com/2.2/questions?page=1&pagesize=100&order=desc&sort=votes&filter=!bA1d_Kvv2YPQWk';
let game = {
 questionReady: false,
 question: {},
 options: [],
 answer: 0,
 questionsAnswered: 0,
 questionsCorrect: 0,
 sites: []
}
let elements = {
 stats: null,
 question: null,
 options: [],
 next: null
}
document.addEventListener('DOMContentLoaded', init);
function init() {
 elements.stats = document.getElementById('stats');
 elements.question = document.getElementById('question');
 elements.next = document.getElementById('next');
 
 let answers = document.getElementById('answers');
 for(let i = 0; i < optionsCount; i++) {
 let button = document.createElement('button');
 button.addEventListener('click', makeGuess.bind(null, i));
 answers.appendChild(button);
 elements.options.push(button);
 }
 
 loadSites().then(chooseQuestion);
}
function loadSites() {
 return fetch(sitesURL)
 .then(response => response.json())
 .then(data => {
 game.sites = data.items.filter(e => e.site_type === "main_site");
 })
}
function loadQuestions(index) {
 return fetch(questionsURL + '&site=' + game.sites[index].api_site_parameter)
 .then(response => response.json())
 .then(data => {
 game.sites[index].questions = data.items;
 })
}
async function chooseQuestion() {
 game.options = getOptions(optionsCount, game.sites.length);
 game.answer = randInt(optionsCount);
 game.question = await getQuestion(game.options[game.answer]);
 updateView();
 game.questionReady = true;
}
function getOptions(amount, max) {
 let options = [];
 while(options.length < amount) {
 let value = randInt(max);
 if(!options.includes(value)) {
 options.push(value);
 }
 }
 return options;
}
function randInt(limit) {
 return Math.floor(Math.random() * limit);
}
async function getQuestion(index) {
 if(!game.sites[index].questions) {
 await loadQuestions(index);
 }
 let size = game.sites[index].questions.length;
 return game.sites[index].questions[randInt(size)];
}
function updateView() {
 elements.question.innerHTML = game.question.title;
 for(let i = 0; i < optionsCount; i++) {
 elements.options[i].innerHTML = game.sites[game.options[i]].name;
 }
}
function makeGuess(guess) {
 if(game.questionReady) {
 game.questionsAnswered++;
 if(guess === game.answer) {
 game.questionsCorrect++;
 }
 else {
 elements.options[guess].classList.add('wrong');
 }
 elements.options[game.answer].classList.add('correct');
 elements.next.disabled = false;
 elements.question.setAttribute('href', game.question.link);
 let percent = Math.round(100 * game.questionsCorrect / game.questionsAnswered);
 elements.stats.innerText = `${game.questionsCorrect}/${game.questionsAnswered} - ${percent}%`;
 game.questionReady = false;
 }
}
function onNextClick() {
 elements.question.removeAttribute('href');
 for(option of elements.options) {
 option.classList.remove('correct', 'wrong');
 }
 elements.next.disabled = true;
 chooseQuestion();
}
.correct {
 background-color: #54da54;
}
.wrong {
 background-color: #ff6565;
}
#answers button {
 margin: 2px;
}
<p id="stats">0/0 - 0%</p>
<h2><a id="question" target="_blank"></a></h2>
<div id="answers"></div>
<br><br>
<button id="next" onclick="onNextClick()" disabled>Next question</button>

Here's a JSFiddle if you like fiddling.

It should work in all modern browsers. I've tried to take advantage of some of the newer javaScript features where I could. I'm wondering if I can make the code better, like use of best practices, readability, efficiency, and that kind of stuff.

t3chb0t
44.6k9 gold badges84 silver badges190 bronze badges
asked Jun 9, 2018 at 16:42
\$\endgroup\$

1 Answer 1

4
\$\begingroup\$

These are some very quick comments about a few things I noticed.

Code style

Consider wrapping your code in an Immediate Invoked Function Expression. This will prevent other code on a page from (accidentally) interacting with your code.

In the following loop you are dropping option in the global namespace. Use const option of elements.options instead:

 for(option of elements.options) {
 option.classList.remove('correct', 'wrong');
 }

Consider using const more. If you do not intend to re-assign a variable, declaring it const will remind you that you are doing something that you did not intend to do in the past. It could also potentially improve performance.

Error handling

Your code lacks error handling. In particular, if an api call fails, your code will fail on the line with let size = game.sites[index].questions.length;. Catch errors and do something accordingly.

onclick

Don't use onclick or any of its siblings as attribute on a html element. (In fact, don't use it in javascript either) Instead, use an addEventListener like you already did for your other buttons.

answered Jun 9, 2018 at 20:25
\$\endgroup\$

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.