JavaScript JavaScript and the DOM Traversing the DOM Getting the First and Last Child

Adam Lyles Kauffman
Adam Lyles Kauffman
11,325 Points

Optimize my solution to the challenge. PLEASEE

felt like i repeated my self many times and had unnecessary code i wasn't sure how to work around. For example i felt the for loop around my updatebtn and removebtn was unnecessary. I also felt like i could have used a newer loop type.

<!DOCTYPE html>
<html>
  <head>
    <title>JavaScript and the DOM</title>
    <link rel="stylesheet" href="css/style.css">
  </head>
  <body>
    <h1 id="myHeading">JavaScript and the DOM</h1>
    <p>Making a web page interactive</p>
    <button id="toggleList">Hide list</button>
    <div class="list">
      <p class="description">Things that are purple:</p>
      <input type="text" class="description">
      <button class="description">Change list description</button>
      <ul>
        <li>grapes</li>
        <li>amethyst</li>
        <li>lavender</li>
        <li>plums</li>
      </ul>
      <input type="text" class="addItemInput">
      <button class="addItemButton">Add item</button>
    </div>
    <script src="app.js"></script>
  </body>
</html>
const toggleList = document.getElementById('toggleList');
const listDiv = document.querySelector('.list');
const descriptionInput = document.querySelector('input.description');
const descriptionP = document.querySelector('p.description');
const descriptionButton = document.querySelector('button.description');
const listUl = listDiv.querySelector('ul');
const addItemInput = document.querySelector('input.addItemInput');
const addItemButton = document.querySelector('button.addItemButton');
const lis = listUl.children;
const firstli = listUl.firstElementChild;
const lastli = listUl.lastElementChild;
firstli.style.backgroundColor = 'grey';
lastli.style.backgroundColor = '#999';

    function attachlistButtons(li){
        if(li !== firstli){
    let up = document.createElement('button');
    up.className = 'up';
    up.textContent = 'UP';
    li.appendChild(up);
    }

        if(li !== lastli){
    let down = document.createElement('button');
    down.className = 'down';
    down.textContent = 'DOWN';
    li.appendChild(down);
        }

    let remove = document.createElement('button');
    remove.className = 'remove';
    remove.textContent = 'REMOVE';
    li.appendChild(remove);
} 

function reformbtn(){
for(let i = 0; i < lis.length; i+=1 ){
    attachlistButtons(lis[i]);
}}

function callremovebtn(){
for(let i = 0; i < lis.length; i+=1){
removebtn(lis[i]);
}}

function callupdatebtn(){
for(let i = 0; i < lis.length; i+=1){
updatebtn(lis[i]);
}}

function removebtn(li){
 if(li.textContent.endsWith('UPDOWNREMOVE')){
 console.log(li);
  console.log(li.firstElementChild);
    count = 0;
   while(count <= 2){
   li.removeChild(li.firstElementChild);
    count++;
}
 } else 
    count = 0;
   while(count <= 1){
   li.removeChild(li.firstElementChild);
    count++;
}
}

function updatebtn(li){
 if(li.textContent === listUl.firstElementChild.textContent){
    let down = document.createElement('button');
    down.className = 'down';
    down.textContent = 'DOWN';
    li.appendChild(down);

    let remove = document.createElement('button');
    remove.className = 'remove';
    remove.textContent = 'REMOVE';
    li.appendChild(remove);
 } else if (li.textContent === listUl.lastElementChild.textContent){
     let up = document.createElement('button');
    up.className = 'up';
    up.textContent = 'UP';
    li.appendChild(up);

    remove = document.createElement('button');
    remove.className = 'remove';
    remove.textContent = 'REMOVE';
    li.appendChild(remove);

} else {
    up = document.createElement('button');
    up.className = 'up';
    up.textContent = 'UP';
    li.appendChild(up);

    down = document.createElement('button');
    down.className = 'down';
    down.textContent = 'DOWN';
    li.appendChild(down);

    remove = document.createElement('button');
    remove.className = 'remove';
    remove.textContent = 'REMOVE';
    li.appendChild(remove);
}}





reformbtn();






listUl.addEventListener('click', (event) => {
      let li = event.target.parentNode;
      let prevLi = li.previousElementSibling;
      let ul = li.parentNode;
      const lichildren = li.children;
  if (event.target.tagName == 'BUTTON') {
    if (event.target.className == 'remove') {
      ul.removeChild(li);
    }
    if (event.target.className == 'up') {
        console.log(li.textContent);
      if (prevLi) {
        ul.insertBefore(li, prevLi);
        callremovebtn();
        callupdatebtn();
      }   
    } 
    if (event.target.className == 'down') {
      let li = event.target.parentNode;
      let nextLi = li.nextElementSibling;
      let ul = li.parentNode;
        if (nextLi) {
        ul.insertBefore(nextLi, li);
       callremovebtn();
        callupdatebtn()
        }
//        if(li.textContent === lastli.textContent){
//            removebtn(li, 'down');
//        }
  }
}});

toggleList.addEventListener('click', () => {
  if (listDiv.style.display == 'none') {
    toggleList.textContent = 'Hide list';
    listDiv.style.display = 'block';
  } else {
    toggleList.textContent = 'Show list';                        
    listDiv.style.display = 'none';
  }                         
});

descriptionButton.addEventListener('click', () => {
  descriptionP.innerHTML = descriptionInput.value + ':';
  descriptionInput.value = '';
});

addItemButton.addEventListener('click', () => {
  let ul = document.getElementsByTagName('ul')[0];
  let li = document.createElement('li');
  li.textContent = addItemInput.value;
  ul.appendChild(li);
  attachlistButtons(li);
 callremovebtn();
 callupdatebtn();
  addItemInput.value = '';
});

1 Answer

Steven Parker
Steven Parker
163,772 Points

It's not a huge improvement, but you could use a "for...of" loop for the place in question:

function callupdatebtn() {
    for(let li of lis) updatebtn(li);
}

Also, since the function isn't used anywhere else, it might make sense to just put the body of the other function directly inside the loop and have just one function that updates all buttons.

Then to make code more "DRY", look for places where similar things are done repeatedly, and make the repeated part into a function where any differences are passed in as parameters. For example:

function attachListButtons(li) {
  function addButton(name) {
    const btn = document.createElement("button");
    btn.className = name;
    btn.textContent = name.toUpperCase();
    li.appendChild(btn);
  }
  if (li !== firstli) addButton("up");
  if (li !== lastli)  addButton("down");
  addButton("remove");
}
Adam Lyles Kauffman
Adam Lyles Kauffman
11,325 Points

much appreciated Steven, you have been a big help in my journey though javascript!!

Steven Parker
Steven Parker
163,772 Points

Adam Kauffman — Glad to help. If this answered the question you can mark it solved by choosing a "best answer".
And happy coding!