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.

\$\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.

\$\endgroup\$

Your Answer

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

Not the answer you're looking for? Browse other questions tagged or ask your own question.