For loop

[Gowri pandiyan]

HI,

I am facing some issue with below code, Kindly let me know what is wrong in this code? Its always adding up values

<script>

function SumRow()
{
var val = 0;

var arr1, arr2;

var val2, val3;




arr1 = ['Q1_1','Q1_2','Q1_3','Q1_4','Q1_5','Q2_1','Q2_2','Q2_3','Q3_1','Q3_2','Q4_1','Q4_2','Q5_1','Q5_2','Q5_3','Q6_1','Q6_2','Q6_3','Q7_1','Q7_2','Q7_3','Q7_4','Q8_1','Q8_2','Q8_3']


arr2 = ['5','10','15','20','25','10','20','30','10','20','10','20','5','10','15','5','10','15','5','10','15','20','10','20','30']





if(document.getElementById(arr1[0])!=null && document.getElementById(arr1[0]).checked)

{


val2 = parseInt( arr2[0] );

val3 = parseInt( document.getElementById('result_1').value);


val = val2 + val3;




document.getElementById('result_1').value = val;



}


if(document.getElementById(arr1[1])!=null && document.getElementById(arr1[1]).checked)

{


val2 = parseInt( arr2[1] );

val3 = parseInt( document.getElementById('result_1').value);


val = val2 + val3;




document.getElementById('result_1').value = val;



}


if(document.getElementById(arr1[2])!=null && document.getElementById(arr1[2]).checked)

{


val2 = arr2[2];

val3 = parseInt( document.getElementById('result_1').value);


val = val2 + val3;




document.getElementById('result_1').value = val;



}










}


setInterval("SumRow()",100);




</script>

[Gowri pandiyan]

Earlier I used For loop to get it work, that was also not working

[Fotiman]

It looks like you want to loop over each of the values in arr1. Here is how I'd start rewriting what you have currently.

  var i,
    el,
    result_1 = document.getElementById('result_1'),
    val = 0,
    arr1 = ['Q1_1','Q1_2','Q1_3','Q1_4','Q1_5','Q2_1','Q2_2','Q2_3','Q3_1','Q3_2','Q4_1','Q4_2','Q5_1','Q5_2','Q5_3','Q6_1','Q6_2','Q6_3','Q7_1','Q7_2','Q7_3','Q7_4','Q8_1','Q8_2','Q8_3'],
    arr2 = ['5','10','15','20','25','10','20','30','10','20','10','20','5','10','15','5','10','15','5','10','15','20','10','20','30'],
    val2,
    val3;

I've gotten rid of the extra "var" statements (try to use 1 "var" with the variables separated by commas). I've added a new variable, i, which will be used in the loop. I've added a new variable, el, which will hold the reference to the element (if you ever find that you're calling getElementById more than once for the same element, then modify that to store it in a variable instead... searching the DOM is a relatively expensive operation). And I've added a new variable to hold the reference to result_1 for the same reason.
Next, create the loop:

  for (i = 0; i < arr1.length; i++)
  {
    el = document.getElementById(arr1[i]); 
    if (el != null && el.checked)
    {
      val2 = parseInt(arr2[i]);
      val3 = parseInt(result_1.value);
      val = val2 + val3;
      result_1.value = val;
    }
  }

Each time through the loop, i will be used as an index into the arr1 and arr2 arrays. We'll search the DOM for the element, if it's not null and it's checked then we'll get the values, add them, and stuff them back into result_1. The end result is this:

function SumRow() {
  var i,
    el,
    result_1 = document.getElementById('result_1'),
    val = 0,
    arr1 = ['Q1_1','Q1_2','Q1_3','Q1_4','Q1_5','Q2_1','Q2_2','Q2_3','Q3_1','Q3_2','Q4_1','Q4_2','Q5_1','Q5_2','Q5_3','Q6_1','Q6_2','Q6_3','Q7_1','Q7_2','Q7_3','Q7_4','Q8_1','Q8_2','Q8_3'],
    arr2 = ['5','10','15','20','25','10','20','30','10','20','10','20','5','10','15','5','10','15','5','10','15','20','10','20','30'],
    val2,
    val3;

  for (i = 0; i < arr1.length; i++)
  {
    el = document.getElementById(arr1[i]); 
    if (el != null && el.checked)
    {
      val2 = parseInt(arr2[i]);
      val3 = parseInt(result_1.value);
      val = val2 + val3;
      result_1.value = val;
    }
  }
}

setInterval("SumRow()", 100);

Here's some things to consider, though:
1. arr1 and arr2 must be the same length. If you add another item to arr1 and forget to add an item to arr2, then you'll get an error because the index will be out of the range of arr2 when it gets to the end. In addition, there seems to be a strong relation between the items in arr1 and arr2. Suppose you want to update the value in arr2 for the item in arr1 that has the value 'Q4_2'... you need to count which index 'Q4_1' is at in the array, then count to the same item in arr2... it's prone to error, and can be difficult to maintain. A better alternative may be to use a single array, with each item in the array being either an object or an array of 2 values. For example:

  arr1 = [
    {id: 'Q1_1', value: '5'},
    {id: 'Q1_2', value: '10'},
    {id: 'Q1_3', value: '15'},
    {id: 'Q1_4', value: '20'},
    {id: 'Q1_5', value: '25'},
    {id: 'Q2_1', value: '10'},
    {id: 'Q2_2', value: '20'},
    {id: 'Q2_3', value: '30'},
    {id: 'Q3_1', value: '10'},
    {id: 'Q3_2', value: '20'},
    {id: 'Q4_1', value: '10'},
    {id: 'Q4_2', value: '20'},
    {id: 'Q5_1', value: '5'},
    {id: 'Q5_2', value: '10'},
    {id: 'Q5_3', value: '15'},
    {id: 'Q6_1', value: '5'},
    {id: 'Q6_2', value: '10'},
    {id: 'Q6_3', value: '15'},
    {id: 'Q7_1', value: '5'},
    {id: 'Q7_2', value: '10'},
    {id: 'Q7_3', value: '15'},
    {id: 'Q7_4', value: '20'},
    {id: 'Q8_1', value: '10'},
    {id: 'Q8_2', value: '20'},
    {id: 'Q8_3', value: '30'}
  ],

With this, the relation becomes much more obvious, and much easier to update and maintain. Less prone to human error. Your loop code would then modify slightly to access the id and value properties of the objects within your array:

  for (i = 0; i < arr1.length; i++)
  {
    el = document.getElementById(arr1[i].id); 
    if (el != null && el.checked)
    {
      val2 = parseInt(arr1[i].value);
      val3 = parseInt(result_1.value);
      val = val2 + val3;
      result_1.value = val;
    }
  }

2. If you're going to treat the values for each of these as an integer, then store them as integers instead of as strings. That will eliminate the need to call parseInt on them. For example, instead of this:
{id: 'Q1_1', value: '5'},
do this:
{id: 'Q1_1', value: 5},
And then instead of this:
val2 = parseInt(arr1[i].value);
do this:
val2 = arr1[i].value;

You would still want to call parseInt on the result_1.value, though, as that will still be a string (assuming result_1 is an input element).

[Fotiman]

End result:

function SumRow() {
  var i,
    el,
    result_1 = document.getElementById('result_1'),
    val = 0,
    arr1 = [
      {id: 'Q1_1', value: 5},
      {id: 'Q1_2', value: 10},
      {id: 'Q1_3', value: 15},
      {id: 'Q1_4', value: 20},
      {id: 'Q1_5', value: 25},
      {id: 'Q2_1', value: 10},
      {id: 'Q2_2', value: 20},
      {id: 'Q2_3', value: 30},
      {id: 'Q3_1', value: 10},
      {id: 'Q3_2', value: 20},
      {id: 'Q4_1', value: 10},
      {id: 'Q4_2', value: 20},
      {id: 'Q5_1', value: 5},
      {id: 'Q5_2', value: 10},
      {id: 'Q5_3', value: 15},
      {id: 'Q6_1', value: 5},
      {id: 'Q6_2', value: 10},
      {id: 'Q6_3', value: 15},
      {id: 'Q7_1', value: 5},
      {id: 'Q7_2', value: 10},
      {id: 'Q7_3', value: 15},
      {id: 'Q7_4', value: 20},
      {id: 'Q8_1', value: 10},
      {id: 'Q8_2', value: 20},
      {id: 'Q8_3', value: 30}
    ],
    val2,
    val3;

  for (i = 0; i < arr1.length; i++)
  {
    el = document.getElementById(arr1[i].id); 
    if (el != null && el.checked)
    {
      val2 = arr1[i].value;
      val3 = parseInt(result_1.value);
      val = val2 + val3;
      result_1.value = val;
    }
  }
}

setInterval("SumRow()", 100);

[Fotiman]

Oh, one thing I missed.
Don't pass a string to setInterval. Instead pass the function reference:
setInterval(SumRow, 100);

Note, you do not include the parenthesis when passing a function reference, as the parenthesis would cause the function to execute immediately, and you'd be calling setInterval on the result of the function call as opposed to the function itself.

[Gowri pandiyan]

Thanks for the detailed explanation! :) However, I am still facing the same issue. Please check the below link.

[author.euro.confirmit.com...]

[Fotiman]

Your link gives me a 400 Bad Request error.

[Gowri pandiyan]

Sorry, It didn't work ...please check the below.

[author.euro.confirmit.com...]

[Gowri pandiyan]

I am not certain why link is not working if we paste it in this thread. Below is alternative link

[survey.euro.confirmit.com...]

[Fotiman]

That link worked for me. I suspect the problem is that you simply don't want to be using setInterval. setInterval, as in your example, basically says "run this function every 100ms". And your function is incremental because it's getting the *current* value from result_1, then adding to that value, and then assigning the incremented value back to result_1. So the next time through the loop, it repeats this process and this time the *current* value will be what it was at the result of the previous call.

[Gowri pandiyan]

Thanks Fotiman! I tried anonymous function as well. But no luck

(function ()
{

Script

})();

[Fotiman]

Anonymous function?! I never said anything about an anonymous function.
The point I'm making is that you don't want to use setInterval. Instead, you want to only call your SumRow method when it needs to be recalculated. For example, when someone clicks on one of the radio buttons that causes the value to change.
In addition, you probably need to change the loop so that it doesn't incrementally add to result_1. For example:

function SumRow() {
  var i,
    el,
    result_1 = document.getElementById('result_1'),
    val = 0,
    arr1 = [
      {id: 'Q1_1', value: 5},
      {id: 'Q1_2', value: 10},
      {id: 'Q1_3', value: 15},
      {id: 'Q1_4', value: 20},
      {id: 'Q1_5', value: 25},
      {id: 'Q2_1', value: 10},
      {id: 'Q2_2', value: 20},
      {id: 'Q2_3', value: 30},
      {id: 'Q3_1', value: 10},
      {id: 'Q3_2', value: 20},
      {id: 'Q4_1', value: 10},
      {id: 'Q4_2', value: 20},
      {id: 'Q5_1', value: 5},
      {id: 'Q5_2', value: 10},
      {id: 'Q5_3', value: 15},
      {id: 'Q6_1', value: 5},
      {id: 'Q6_2', value: 10},
      {id: 'Q6_3', value: 15},
      {id: 'Q7_1', value: 5},
      {id: 'Q7_2', value: 10},
      {id: 'Q7_3', value: 15},
      {id: 'Q7_4', value: 20},
      {id: 'Q8_1', value: 10},
      {id: 'Q8_2', value: 20},
      {id: 'Q8_3', value: 30}
    ];

    for (i = 0; i < arr1.length; i++)
    {
      el = document.getElementById(arr1[i].id); 
      if (el != null && el.checked)
      {
        val += arr1[i].value;
      }
    }
    result_1.value = val;
  }
}

[Gowri pandiyan]

Bravo! It works like a charm :) Thanks a ton, Fotiman! you are awesome!