Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript JavaScript Loops, Arrays and Objects Tracking Multiple Items with Arrays Build a Quiz Challenge, Part 2 Solution

Critique this solution please?

I went about the solution differently than Dave, can you please comment on this code? Anything inherently wrong/inefficient about it (I know using document.write() is a no-no these days, so ignore that!) ? Length wise it's the same # of lines as Dave's solution....

var quiz = [  
    ["What year is it", 2015],  
    ["How many hours are in a day", 24],  
    ["How many days are in a week", 7]  
] ;  

var response ;   
var correct = [ ] ;  
var incorrect = [ ] ;  
var numberCorrect = 0 ;  
var numberIncorrect = 0 ;  

for( i = 0; i < quiz.length; i++ ) {  
    response = parseInt( prompt( quiz[i][0] ) );  
    if( response === quiz[i][1] ) {  
        numberCorrect++ ;  
        correct.push(quiz[i][0]) ;  }  
    else {  
        numberIncorrect++ ;  
        incorrect.push(quiz[i][0]) ;  }   

function score( message ) {  
    if( message === numberCorrect ) {  
    document.write( "You answered " + message + " questions correctly.<br>" ) ; }  
    else if( message === numberIncorrect ) {  
    document.write( "You answered " + message + " questions incorrectly.<br>" ) ; }  

function list( message ) {  
    if( message === correct ) {  
        var list = "You answered the following questions correctly: <ol>" ;  
        for( i = 0; i < correct.length; i++) {  
            list += "<li>" + correct[i] + "</li>" ; }  
    else if( message === incorrect ) {  
        var list = "You answered the following questions incorrectly: <ol> " ;  
        for( i = 0; i < incorrect.length; i++) {  
            list += "<li>" + incorrect[i] + "</li>" ; }  
    list += "</ol>" ;  
    document.write( list ) ;  

score( numberCorrect ) ;  
score( numberIncorrect ) ;  
document.write( "<br><br>" ) ;  
list( correct ) ;  
document.write( "<br>" ) ;  
list( incorrect ) ;

2 Answers

Here's my 2 cents:

  1. I would rename the score function to something like printResults. The name of the input argument should have been called score because that is what is actually passed to this function: the function takes the player's score and prints out a congratulatory message based on it.
  2. You do not need the numberCorrect and numberIncorrect variables. You can simply use the length of the correct and the incorrect lists that you populate during the execution of the program for the same purpose.
  3. I would also rename the list function and its input argument to something more appropriate according to their intended purpose.
  4. What I would consider a little bit on the "dangerous" side is the name of the local list variable inside the function with the same name. The list variable hides the list function object in the lexical scope of the list function. In other words, inside the list function you are no longer able to reference the function itself because it is hidden by the local variable. In your particular example, this is not critical; but had the list function been recursive, you would have had a problem. As a general rule you want to stay away (as much as possible) from situations where symbols are hidden like that. Simply rename the local list variable to something else.

All great points, thanks Andrei!!