1
\$\begingroup\$

I got this basic script which I'll be using to check if the answer is right. The only problem is that I'll be using with many forms. That's why I need this code to be more like DRY code.

            function checkAnswer()
        {
            var answer = document.getElementById("answer").value;
            if (answer.toLowerCase() == "correct answer")
            {
                alert("Well done!");
            }
            else
            {
                alert("Too bad!");
            }

            var answer2 = document.getElementById("answer2").value;
                     if (answer2.toLowerCase() == "correct answer")
            {
                alert("Well done!");
            }
            else
            {
                alert("Too bad!");
            }

            var answer3 = document.getElementById("answer3").value;
            if (answer3.toLowerCase() == "correct answer")
            {
                alert("Well done!");
            }
            else
            {
                alert("Too bad!");
            }
        }
\$\endgroup\$
1
  • \$\begingroup\$ This looks hypothetical code, as if you had copy-pasted the first answer handling and added numbering 2 and 3. \$\endgroup\$
    – janos
    Commented Feb 12, 2016 at 20:03

1 Answer 1

3
\$\begingroup\$
  1. Checking against a string like this is error prone. What happens if you accidently misspell "correct" one of the times? You're better off using a number constant as an enum to signify success/failure (After rereading your post, I realized I may have misinterpreted your intent).

  2. I'd just extract that repeated bit into a function:

    function answerIsCorrect(answer, correctAnswer) {
        return answer.toLowerCase() === correctAnswer;
    }
    

    Then, since you're also repeating the use of this code, you might as well use a loop to prevent duplication:

    function checkAnswers(correctAnswers) {
    
        //You could also just use a standard for-loop for this
        correctAnswers.forEach(function(correctAnswer, i) {
            //Note unless you check prevent i from being appended when i == 1,
            // your first answer ID will be "answer1" instead of "answer"
            var answer = document.getElementById("answer" + (i + 1)).value;
    
            if (answerIsCorrect(answer, correctAnswer)) {
                alert("Well done!");
    
            } else {
                alert("Too bad!");
            }
        });
    }
    

    correctAnswers could also be a list of numbers, representing each answer's position in the list of answers. This would mean you would only need to check the index of the answer, instead of the value of the answer, which would make manipulations like toLowerCase unneccessary.

  3. Your indentation is a little off at the top, but that could be a pasting error.

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