Welcome to the Treehouse Community

Want to collaborate on code errors? Have bugs you need feedback on? Looking for an extra set of eyes on your latest project? Get support with fellow developers, designers, and programmers of all backgrounds and skill levels here with the Treehouse Community! While you're at it, check out some resources Treehouse students have shared here.

Looking to learn something new?

Treehouse offers a seven day free trial for new students. Get access to thousands of hours of content and join thousands of Treehouse students and alumni in the community today.

Start your free trial

JavaScript JavaScript and the DOM (Retiring) Traversing the DOM Getting the First and Last Child

Posted by Adam Lyles Kauffman
Adam Lyles Kauffman
13,856 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
229,644 Points
Steven Parker
Steven Parker
229,644 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
13,856 Points

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

Steven Parker
Steven Parker
229,644 Points
Steven Parker
Steven Parker
229,644 Points

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