Whats the wrong with this for loop?

[samvb]

Hi,

i am trying to pass text from one select box to another select box. The logic is if 10 are added, no more passing must happen. Also if an item is already added, it mustn't be added again. I am using the for loop to check the existence of an item but it is not working: what am i doing wrong?

function PassSelectValues(){
//pass values from select boxes to select boxes
var counter;
var select = document.getElementById("addedapples");//add to this
var srcselect_id = document.getElementById("srcapples");//id of the src select box
var srcselect_name= srcselect_id.options[srcselect_id.selectedIndex].text;
var srcselect_code= srcselect_id.options[srcselect_id.selectedIndex].value;
var ttladded=select.options.length;//how many apples had been added already?
alert(ttladded);
//is the item already added?
if(ttladded==10){
 alert("10 apples already passed");
             return false;    
}
if(ttladded==0){
     select.options[select.options.length] = new Option(srcselect_name, srcselect_code);
     }
 
for(counter=0;counter<ttladded;counter++){
var existvalue=select.options[counter].value;
 
     if(existvalue==srcselect_code){
 
        alert("Item is already in the list");
 
        break;
        return;
          }// if(existvalue==srcselect_code){
 
    else if(existvalue!=srcselect_code){//add it
 
      select.options[select.options.length] = new Option(srcselect_name, srcselect_code);
    }//existvalue!==srcselect_code
 
 
    }//end for
 
 
 
}//end of function
 

why isn't counter incrementing at all? The alert message box does appear saying item exists but the item gets added anyway.

[gits]

it is a quite obvious mistake - you loop through all existing options to check whether a node should be appended. you always start at option 0 so it might be that the option is added and the condition fires on the next item to check ... so you might move the add-code outside of the loop and use a flag instead like this:

function PassSelectValues() {
    var counter;
    var select         = document.getElementById("addedapples");
    var srcselect_id   = document.getElementById("srcapples");
    var srcselect_name = srcselect_id.options[srcselect_id.selectedIndex].text;
    var srcselect_code = srcselect_id.options[srcselect_id.selectedIndex].value;
 
    var ttladded = select.options.length;
 
    var optionToAdd = new Option(
        srcselect_name, srcselect_code
    );
 
    if(ttladded == 10) {
        alert("10 apples already passed");
        return;    
    }
 
    if(ttladded == 0) {
        select.options[select.options.length] = optionToAdd;
    }
 
    var doNotAdd = false;
 
    for(counter = 0; counter < ttladded; counter++) {
        var existvalue = select.options[counter].value;
 
        if(existvalue == srcselect_code) {
            alert("Item is already in the list");
            doNotAdd = true;
            break;
        } 
    }
 
    if (!doNotAdd) {
       select.options[select.options.length] = optionToAdd;
    }
}
 

[samvb]

thank you!

you saved my day big time!