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) Getting a Handle on the DOM Selecting Elements with the Same Class Name

Refactoring this code using if-loop within the for-loop

Hello,

I was attempting to refactor this code to enhance my understanding and practice. I attempted to refactor the two for-loops into a for-loop with an if-loop inside, the problem I am having is that only two of the 3 list items are changing color to red and I can't figure out why the last list item isn't changing to red. Can anyone share some insight? Thanks!

const myList = document.getElementsByTagName('li');
const errorNotPurple = document.getElementsByClassName('error-not-purple');

for (let i=0; i < myList.length; i++) {
  if (errorNotPurple) {
    for (let j = 0; j < errorNotPurple.length; j++) {
      errorNotPurple[j].style.color = 'red';
    }
  }
  myList[i].style.color = 'purple';
}

/*
for(let i=0; i < errorNotPurple.length; i++) {
  errorNotPurple[i].style.color = 'red';
}
*/
Kevin Gates
Kevin Gates
15,053 Points

adding the js language just inside your first set of backticks to make this more easily readable.

const myList = document.getElementsByTagName('li');
const errorNotPurple = document.getElementsByClassName('error-not-purple');
for(let i=0; i < myList.length; i++) {
  if(errorNotPurple){
    for(let j = 0; j < errorNotPurple.length; j++) {
      errorNotPurple[j].style.color = 'red';
}
  }
  myList[i].style.color = 'purple';
}




/*
for(let i=0; i < errorNotPurple.length; i++) {
  errorNotPurple[i].style.color = 'red';
}
*/

1 Answer

Wiktor Bednarz
Wiktor Bednarz
18,647 Points

Hey there Jason,

I don't understand why would you want to nest an "error-not-purple" loop within "myList" loop. You've got to ask yourself a question - what the program does in that case? Let's try to break your code apart and see what it's attempting to do:

for(let i=0; i < myList.length; i++) {
  //do something
}

this is pretty straightforward, you want the application to perform some task for every item it encounters within myList array.

for(let i=0; i < myList.length; i++) {
  if(errorNotPurple) {
    //do something
  }
}

So now you want to perform an action only if the errorNotPurple statement returns true. However errorNotPurple is an array, and it doesn't change its values during your loop. So in the end this statement always returns true and your loop does "something" same amount of times as it did before you added the if statement.

for(let i=0; i < myList.length; i++) {
  if(errorNotPurple) {
    for(let j = 0; j < errorNotPurple.length; j++) {
      //do something
    }
  }
}

so now you add a second loop that performs some task for every item in errorNotPurple array. So now let's say you have 7 items in myList array - the outer loop performs an operation for each of those items. For errorNotPurple array you have 3 items. So now whenever you perform an action for myList array loop, you perform 3 actions for errorNotPurple loop. That makes a total of 7x3 = 21 total operations which your application performs.

for(let i=0; i < myList.length; i++) {
  if(errorNotPurple) {
    for(let j = 0; j < errorNotPurple.length; j++) {
      errorNotPurple[j].style.color = 'red';
    }
  }
}

ok so the task that is executed 21 times is change of css color property to red on an item corresponding to current j in your loop. The j value in your loop beheaves like that j=0 -> j=1 -> j=2 -> j=0 ->....-> j=2. So it constantly assings all of the errorNotPurple items (those items are: errorNotPurple[0], errorNotPurple[1], errorNotPurple[2]), even though they've been assigned to it the first time the loop ran already. After this loop finishes ALL of the errorNotPurple have been assigned to color 'red' 7 times each.

for(let i=0; i < myList.length; i++) {
  if(errorNotPurple) {
    for(let j = 0; j < errorNotPurple.length; j++) {
      errorNotPurple[j].style.color = 'red';
    }
  }
 myList[i].style.color = 'purple';
}

and finally at the end of all those operations you're taking your i value (which at the end of the myList loop is equal to 6 - the maximum value it reaches) and you want myList[i] item to change the color to purple. It so happens that this last item is also the one with 'errorNotPurple' class:

<li class="error-not-purple">oranges</li>

so you end up with 2 items with color purple and 5 items with color red.

As you can see, this is one complicated mess and that application performs a lot of unnecessary tasks. If your goal was to create one for loop which would assign color to all the elements properly instead of two seperate loops, then it'd be something like:

for (let i=0; i<myList.length; i++) {
  if (myList[i].className === 'error-not-purple') { // you check if your item's class name matches 'error-not-purple' string
    myList[i].style.color = 'red'; // if it does then it's color is red
  } else {
    myList[i].style.color = 'purple'; // if it doesn't then it's color is purple
  }
}

I just wanted to show you how important it is to sit back and slowly, step by step try to understand what your code is achieving after you wrote it. Especially after it runs with no errors and doesn't perform what you wanted it to perform. I encourage you to take these kind of excersises whenever you are lost in what's the problem with your code. Trying to dissect it to a logical chain will help you understand what really is going on.

I hope that this answer will be helpfull to you, best of luck in coding,

Wiktor Bednarz