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 trialAdam Lyles Kauffman
14,016 PointsOptimize 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
231,270 PointsIt'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
14,016 PointsAdam Lyles Kauffman
14,016 Pointsmuch appreciated Steven, you have been a big help in my journey though javascript!!
Steven Parker
231,270 PointsSteven Parker
231,270 PointsAdam Kauffman — Glad to help. If this answered the question you can mark it solved by choosing a "best answer".
And happy coding!