CODEWITHSUNDEEP
javascripthtml
ShadyBears
ShadyBears

Reputation: 4185

This Javascript is extremely slow

What I'm trying to accomplish:

I am trying to update abbr's title to the value of what is in the ".Left" and ".Right" classes, respectively. I've come up with a jQuery solution but it was too expensive in terms of load time and I am trying to come up with a Javascript solution but I'm stuck. I'm new to JS so I have a lot to learn about the DOM.

The HTML I gave repeats itself 100 times (same exact skeleton). How can I make my code more efficient? Also, I'm getting a "cannot set property title of null" error.

var ratedSection = document.getElementsByClassName('RatedSection');
var ratedQuest = document.getElementsByClassName('RatedQuest');
var selector = document.getElementsByClassName('Selector');

for (var i = 0; i < ratedSection.length; i++) { //loops through ratedsection
  var left = ratedSection[i].querySelector('.Left');
  var right = ratedSection[i].querySelector('.Right');

  left = left.innerHTML.replace(/<br>/g, " ");
  right = right.innerHTML.replace(/<br>/g, " ");

  for (var j = 0; j < ratedQuest.length; j++) {
    for (var k = 0; k < selector.length; k++) {
      var index = ratedQuest[j].querySelector('input').value;

      if (index == 1)
        selector[k].querySelector('abbr').title = left;

      else if (index == max)
        selector[k].querySelector('abbr').title = right;
    }
  }
}
<table class="RatedSection">
  <tr class="Header">
    <td class="Left">Stuff
      <br>inside
      <br>here
    </td>
    <td class="Right">More
      <br>stuff
      <br>inside
      <br>here
    </td>
  </tr>
  <tr class="RatedQuest">
    <td class="Selector">
      <abbr title>7</abbr>
      <abbr title>1</abbr>
    </td>
  </tr>
</table>

Upvotes: 0

Views: 73

Answers (1)

salezica
salezica

Reputation: 77089

Let's remove some clutter and take a look at the looping code:

for(var i = 0; i < ratedSection.length; i++)
  for(var j = 0; j < ratedQuest.length; j++)
    for(var k = 0; k < selector.length; k++)

I'm guessing you wanted to write this (pseudocode):

FOR EACH section IN PAGE
  FOR EACH quest INSIDE section
    FOR EACH selector INSIDE quest

But you're actually doing this:

FOR EACH section IN PAGE
  FOR EACH quest IN PAGE
    FOR EACH selector IN PAGE

Every pass of the loop, you're looking at a whole page of quest/selector items, not only at those inside the current section. You're getting the references to quests and selectors from document, not from the current section.

Instead of iterating 100 times, you're iterating 10000 times. Use querySelector on the section object when you enter the first loop

Upvotes: 7

Related Questions