Is there a way to simplify this JavaScript code?

Question

I know this is a broad question but I'm still novice to know what to search/ask for.

I have this JavaScript series for a Jeopardy game. I am sure that there is a better way to do this because there is so much repetition. I'm thinking something with some variables, arrays, and/or passing IDs through when the function is called but don't know where to start. Any ideas would be appreciated. I am not asking anyone to do the work, just give me some ideas and examples of which direction to go.

var score = 0;


//Disable the question button
function disableButton(btnID){
        document.getElementById(btnID.id).disabled = true;
    }

//Show current score
function endQuestion(){
  alert ("Your total score is now " +score);
}

//Toggle Images On
function showImgBtn1(){
  document.getElementById('btn1pic').style.visibility = 'visible';
  setTimeout(askQuestion1,3000);
}
function showImgBtn2(){
  document.getElementById('btn2pic').style.visibility = 'visible';
  setTimeout(askQuestion2,3000);
}
//This keeps going for every question--repeated 9 to 20 times


//Questions
function askQuestion1()
{
  var answer = prompt ("What body system contains the heart, blood, arteries, and veins?") .toLowerCase();
   if (answer==="circulatory") {
    alert ("Correct!");
     score += 100;
  }
  else {
    alert ("Sorry, incorrect.");
     score -= 100;
  }
    endQuestion();
    document.getElementById('btn1pic').style.visibility = 'hidden';
}

function askQuestion2()
{
  var answer = prompt ("What body system contains the brain, spinal cord, and nerves?") .toLowerCase();
  if (answer==="nervous") {
   alert ("Correct!");
    score += 200;
  }
  else {
    alert ("Sorry, incorrect.");
    score -= 200;
  }
  endQuestion();
  document.getElementById('btn2pic').style.visibility = 'hidden';
}
//This keeps going for every question--same code, just replace the question and answer repeated 9 to 20 times

This is how I call it on my HTML page:

<td><button id="btn1" onclick="showImgBtn1();disableButton(btn1)">100</button></td> //each button has a successive ID

and this is how I have the pics set up on my HTML page:

<div>
  <img class="picClue" id="btn1pic" src="btn1.jpg" height="200px">
  <img class="picClue" id="btn2pic" src="btn2.jpg" height="200px">
  <img class="picClue" id="btn3pic" src="btn3.jpg" height="200px">
  <img class="picClue" id="btn4pic" src="btn4.jpg" height="200px">
</div>

Thanks for any advice!

don't know where to start... Put all duplicate code into one function - Start with : showImgBtn(button, askQuestion, duration) - Then do this in a similar way with askQuestion(questionId) and so on.. — B001ᛦ
– B001ᛦ, Commented Dec 17, 2019 at 14:29
start with the data. I think a quiz is is an ordered (?) list of picture+question+correct answer. think of your function(s) to render your page to take that as an input. Also, if you want to make a serious quiz - don't send the answers to the client :) — Len
– Len, Commented Dec 17, 2019 at 14:36

Nicolas · Accepted Answer · 2020-01-08 15:48:43Z

5

I see a few things that you could improve. It is a good things that you notice there is a lot of repetition in your code. First of all, the function askQuestion1 and askQuestion2 does the exact same thing with the exception of a few variable. So you could make one function and store your different value into a object. Something like this :

let values = {
  question1: {
    questionText: "What body system contains the heart, blood, arteries, and veins?",
    correctValue: "circulatory",
    id: 'btn1pic'
  },
  question2: {
    questionText: "What body system contains the brain, spinal cord, and nerves?",
    correctValue: "nervous",
    id: 'btn2pic'
  }
}

let score = 0;

function askQuestion(question)
{
  var answer = prompt(question.questionText);
   // validate null and check if the answer is correct.
   if (answer && answer.toLowerCase() === question.correctValue) {
    alert ("Correct!");
     score += 100;
  }
  else {
    alert ("Sorry, incorrect.");
     score -= 100;
  }
    endQuestion();
    document.getElementById(question.id).style.visibility = 'hidden';
}

function endQuestion() {}

askQuestion(values.question1);
askQuestion(values.question2);

as Edric pointed out in the comment, it is a good idea to validate that your user did not cancel the prompt, resulting in a null value in your answer variable.

You could even save your question in an array and use a for loop to ask each of them:

    let values = [
      {
        questionText: "What body system contains the heart, blood, arteries, and veins?",
        correctValue: "circulatory",
        id: 'btn1pic'
      },
      {
        questionText: "What body system contains the brain, spinal cord, and nerves?",
        correctValue: "nervous",
        id: 'btn2pic'
      }
    ]
/* ... */
    for(let i = 0; i < values.length; i ++) {
        askQuestion(values[i]);
    }

The same things goes for your showImgBtn function, having a question variable would create less repetition.

//Toggle Images On
function showImgBtn(question){
  document.getElementById(question.id).style.visibility = 'visible';
  setTimeout(() => { askQuestion(question); },3000);
}

Other than that, it seams fine.

I highly encourage you to check the comment aswell, many people are suggesting ways to improve my answer.

edited Jan 8, 2020 at 15:48

answered Dec 17, 2019 at 14:38

Nicolas

8,7364 gold badges25 silver badges58 bronze badges

Sign up to request clarification or add additional context in comments.

12 Comments

Edric Over a year ago

You didn't consider that the user might cancel the prompt that would be shown, which would result in an error where you're calling toLowerCase on a null result.

h0r53 Over a year ago

Since this is jeopardy I would add questionScore as an attribute to each question in values.

Bill Pratt Over a year ago

Thanks @Nicolas I see what you're talking about and will work on it. I am having trouble with passing the data from HTML to JavaScript but think I have something typed wrong somewhere. Thanks to the other commenters too. Great ideas all around.

Nicolas Over a year ago

The ;showImgBtn does not take a string as argument but a question object: showImgBtn(values[0])

Nicolas Over a year ago

but if you're calling that from your HTML, you might not have access to the values varaible. So you could use another function that would take a string and call the coresponding question object. function(key) { showImgBtn(values[key]); };

|

Collectives™ on Stack Overflow

Is there a way to simplify this JavaScript code?

1 Answer 1

12 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

12 Comments

Your Answer

Sign up or log in

Post as a guest

Related