Critique this solution please?

Question

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 ) ;

Answer 1 · 2015-02-25T09:36:54Z

February 25, 2015 9:36am

Here's my 2 cents:

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.
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.
I would also rename the list function and its input argument to something more appropriate according to their intended purpose.
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.

Answer 2 · 2015-02-25T11:46:30Z

February 25, 2015 11:46am

All great points, thanks Andrei!!

Welcome to the Treehouse Community

Looking to learn something new?

Nijad Kifayeh

Nijad Kifayeh

Critique this solution please?

2 Answers

Andrei Fecioru

Andrei Fecioru

Nijad Kifayeh

Nijad Kifayeh