Join the Stack Overflow Community
Stack Overflow is a community of 6.5 million programmers, just like you, helping each other.
Join them; it only takes a minute:
Sign up

The function does not seem to delete the Node containing the specified value unless it is first value (in this case 'apples'). The for loop also has to execute twice before deletion of any kind. Why is that so?

function removeSpec()
{
    var query = document.getElementById('spec').value;  /* Value inputted by user */
    elements = document.getElementsByClassName('fruit'); /* Get the li elements in the list */
    var myList = document.getElementById("myList3"); /* Var to reference the list */
    var length = (document.getElementsByClassName('fruit').length); /* # of li elements */
    var checker = 'false'; /* boolean-ish value to determine if value was found */

    for(var counter = 0; counter < length; counter ++)
    {
        if (elements[counter].textContent == query )
        {
             alert("Counter : " + counter);
             myList.removeChild(myList.childNodes[ (counter) ]);
             checker="true";
        }
    }
  if ( checker == "false") 
   {
       alert("Not Found");
   }
}

The corresponding HTML:

  <ul id="myList3">
                <li class="fruit" >Apples</li>
                <li class="fruit" >Oranges</li>
                <li class="fruit" >Banannas</li>
                <li class="fruit">Strawberry</li>
   </ul>
   <form> 
           Value: <input type="text" name="" value="" id="spec">
   <br><br>
    </form>
    <button type="button" style="height:20px;width:200px" href="javascript:void(0)" onclick="removeSpec()" > 

        Remove Specified 
   </button>
share|improve this question
    
Where did you find this code that you don't understand? Who wrote it? – Bergi 55 mins ago

childNodes returns a list of all child nodes. That includes text nodes. Between every <li> element you have a text node that contains spaces and a line break. So, childNodes returns a list of 9 nodes, but you are assuming list of 4 nodes (document.getElementsByClassName('fruit').length).

You could use .children instead of .childNodes. .children returns a list of only element nodes. Or better yet, use elements, since that's what you are iterating over.

You also need to stop iterating after you found an removed a node, otherwise you will be trying to access a position that doesn't exist anymore.

function removeSpec()
{
    var query = document.getElementById('spec').value;  /* Value inputted by user */
    elements = document.getElementsByClassName('fruit'); /* Get the li elements in the list */
    var myList = document.getElementById("myList3"); /* Var to reference the list */
    var length = (document.getElementsByClassName('fruit').length); /* # of li elements */
    var checker = 'false'; /* boolean-ish value to determine if value was found */

    for(var counter = 0; counter < length; counter ++)
    {
        if (elements[counter].textContent == query )
        {
             myList.removeChild(myList.children[ (counter) ]);
             // better: myList.removeChild(elements[counter]);
             checker="true";
             break;
        }
    }
  if ( checker == "false") 
   {
       alert("Not Found");
   }
}
<ul id="myList3">
                <li class="fruit" >Apples</li>
                <li class="fruit" >Oranges</li>
                <li class="fruit" >Banannas</li>
                <li class="fruit">Strawberry</li>
   </ul>
   <form> 
           Value: <input type="text" name="" value="" id="spec">
   <br><br>
    </form>
    <button type="button" style="height:20px;width:200px" href="javascript:void(0)" onclick="removeSpec()" > 

        Remove Specified 
   </button>


There are other things that could be improved (e.g. why not assign an actual boolean value to checker?), but they are not related to your question.

share|improve this answer
1  
Your answer will be more useful to othe… wait never mind :) – Pierre Arlaud 3 hours ago

I run this code. you should add this line

elements[counter].remove();

instead of this line

myList.removeChild(myList.childNodes[ (counter) ]);
share|improve this answer
1  
Your answer will be more useful to others if you explain why. – Felix Kling 7 hours ago

Instead of for loop you can consider of doing it the below way.

check this snippet

function removeSpec() {
  var query = document.getElementById('spec').value; /* Value inputted by user */
  var elements = document.getElementsByClassName('fruit'); /* Get the li elements in the list */
  var myList = document.getElementById("myList3"); /* Var to reference the list */
  var length = (document.getElementsByClassName('fruit').length); /* # of li elements */
  var checker = 'false'; /* boolean-ish value to determine if value was found */

  myList.querySelectorAll('li').forEach(function(item) {
    if (item.innerHTML == query)
      item.remove();
  });
}
<ul id="myList3">
  <li class="fruit">Apples</li>
  <li class="fruit">Oranges</li>
  <li class="fruit">Banannas</li>
  <li class="fruit">Strawberry</li>
</ul>
<form>
  Value:
  <input type="text" name="" value="" id="spec">
  <br>
  <br>
</form>
<button type="button" style="height:20px;width:200px" href="javascript:void(0)" onclick="removeSpec()">

  Remove Specified
</button>

Hope it helps

share|improve this answer

This might sound crazy, but Chrome seems to parse your HTML unordered list into the following:

NodeList[9]
0: text
1: li.fruit
2: text
3: li.fruit
4: text
5: li.fruit
6: text
7: li.fruit
8: text
length: 9
__proto__: NodeList

Essentially, it appears to be creating a text node in your unordered list for each newline inside the tag. This also explains why deletion only occurs after you call the function a second time - it deletes the text node first, then it deletes the actual element on its second try.

Simple converting your HTML to the following form solves the problem (but is not very pretty):

<ul id="myList3"><li class="fruit">Apples</li><li class="fruit">Oranges</li><li class="fruit">Banannas</li><li class="fruit">Strawberry</li></ul>

There are some workarounds that you can try using. For example, you could try using the childNode.remove() method instead, though not all browsers support this.

Alternatively, something like this might also work:

selectedChildNode.parentNode.removeChild(selectedChildNode);
share|improve this answer
4  
Every HTML parser should parse it that way. It's how HTML/DOM works. – Felix Kling 7 hours ago

here the problem is in myList.removeChild(myList.childNodes[ (counter) ]); because myList.childNodes node return 8 values instead of 4. We have elements array with 4 nodes, hence the removing from elements array yields a proper result

Try the code snippet below,

function removeSpec() {
        var query = document.getElementById('spec').value;
        elements = document.getElementsByClassName('fruit');

        var myList = document.getElementById("myList3"); 
        var length = elements.length; 
        var checker = 'false'; 

        for(var counter = 0; counter < length; counter ++)
        {           
            if (elements[counter].textContent == query )
            {
                 alert("Counter : " + counter);
                 myList.removeChild(elements[counter]);
                 checker="true";
            }
        }
      if ( checker == "false") 
       {
           alert("Not Found");
       }
}
share|improve this answer
2  
Your answer will be more useful to others if you explain why. – Felix Kling 7 hours ago
    
here the problem is in myList.removeChild(myList.childNodes[ (counter) ]); because myList.childNodes node return 8 values instead of 4. We have elements array with 4 nodes, hence the removing from elements array yields a proper result – selvassn 7 hours ago
2  
You can edit your answer to include that information. – Felix Kling 7 hours ago
1  
Done. Thanks!!! – selvassn 7 hours ago

myList is an array of li element so removeChild on myList is logically not correct.

Also, myList.childNodes doesn't make sense here.

Try

myList[counter].parentNode.removeChild(myList[counter]);
share|improve this answer
3  
Your answer will be more useful to others if you explain why. – Felix Kling 7 hours ago

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.