why does this javascript array for loop not work like the literal version?

Question

I'm new to javascript and was trying to refactor some code, clearly there's something I'm missing in javascript that I'd like to learn. This code produces a value once all 5 list boxes have something selected:

function GetTotal() {
        //_listSeverity = document.getElementById("_listSeverity");
        function ParseListBoxvalue(listBox) {
        return parseInt(GetListBoxValue(listBox),10);
        }
        _listSeverity = document.getElementById("<%= _listSeverity.ID %>");
        _listAssociate = document.getElementById("<%= _listAssociateImpact.ID %>");
        _listCustomerImpact = document.getElementById("<%= _listCustomerImpact.ID %>");
        _listRegulatoryImpact = document.getElementById("<%= _listRegulatoryImpact.ID %>");
        _listShareholderImpact = document.getElementById("<%= _listShareholderImpact.ID %>");
        _calculatedTotal = (ParseListBoxvalue(_listAssociate) +
            ParseListBoxvalue(_listSeverity) + ParseListBoxvalue(_listCustomerImpact) 
           +ParseListBoxvalue(_listRegulatoryImpact) + ParseListBoxvalue(_listShareholderImpact)
          )/ 5;
        if (isNaN(_calculatedTotal))
            document.getElementById("_total").innerHTML = "Not enough information";
        else
            document.getElementById("_total").innerHTML = _calculatedTotal;
    }

Then I tried to refactor into a for loop to eliminate some of the code duplication. I tried many methods of if(typeof _calculatedValue !='undefined') I found out on google to see if that could solve it. As I understand it I'm not running into a scope issue here as the only actual scopes are bounded by function(){} declarations. This never produces a value. I realize the / 5 isn't in it yet, but that doesn't seem a reason to me for this to always produce a NaN.

 function GetTotal() {
        //_listSeverity = document.getElementById("_listSeverity");
        function ParseListBoxvalue(listBox) {
        return parseInt(GetListBoxValue(listBox),10);
    }
        var _ListIds=new Array("<%= _listSeverity.ID %>","<%= _listAssociateImpact.ID %>",
            "<%= _listCustomerImpact.ID %>", "<%= _listRegulatoryImpact.ID %>",
            "<%= _listShareholderImpact.ID %>");
//            _calculatedTotal = (ParseListBoxvalue(_listAssociate) +
//                ParseListBoxvalue(_listSeverity) + ParseListBoxvalue(_listCustomerImpact) 
//               +ParseListBoxvalue(_listRegulatoryImpact) + ParseListBoxvalue(_listShareholderImpact)
        //              )/ 5;
        for (i = 0; i < _ListIds.length; i++) {
            if (i==0)
                _calculatedTotal = ParseListBoxvalue(_ListIds[i]);
            else
                _calculatedTotal += ParseListBoxvalue(_ListIds[i]);
        }

        if (isNaN(_calculatedTotal))
            document.getElementById("_total").innerHTML = "Not enough information";
        else
            document.getElementById("_total").innerHTML = _calculatedTotal;
    }

The other function should be of no relevance but here it is :

function GetListBoxValue(listBox) {
        index = listBox.selectedIndex
        try {
            opt = listBox.options[index]
            return opt.value;
        } catch (e) { return null; }

    }

What is wrong with that for loop? or is it something besides the for loop that causes the refactoring to not produce a value?

Answer 1

You didn't call document.getElementById():

_calculatedTotal = ParseListBoxvalue(_ListIds[i]);

should be

_calculatedTotal = ParseListBoxvalue(document.getElementById(_ListIds[i]));

and similarly for the other branch.

Answer 2

Some issues:

• As Greg said, you didn't call document.getElementById.
• You're not dividing the total by 5, which the original code did.
• Be sure to declare your variables (such as i); otherwise, they're globals (see The Horror of Implicit Globals). (The old code was really neglectful about that as well.)
• Strongly recommend using braces even when there's only one statement in a branch.

Answer 3

Some improvements:

var _calculatedTotal  = ParseListBoxvalue(document.getElementById(_ListIds[0]));
for (var i = 1; i < _ListIds.length; i++) {
 _calculatedTotal += ParseListBoxvalue(document.getElementById(_ListIds[i]));
}

document.getElementById("_total").innerHTML = isNaN(_calculatedTotal) ? "Not enough information" : _calculatedTotal


function GetListBoxValue(listBox) {
 if(listBox.selectedIndex){
 var opt = listBox.options[listBox.selectedIndex]
 }
 return opt ? opt.value : null;
}