Reducing the amount of script used in for loop

JavaScript
1

I am coming back to a website and have a bit of time to tidy things up, and wondered with the script below, as I do have a few of these chunks, if there a way to use less lines, as every line is unique in that the id’s are different but only one’s css value is different, so wondered if I could only use 2 document.getElementById lines, putting all the same id’s in one line with ‘none’ as the css value, leaving the other line to be on its own with the value of ‘block’

function subShowHidec(obj){
    for(i=1;i<=7;i++){
        if (i == obj) {
            document.getElementById('qqb'+i).style.display = 'none';
	    document.getElementById('qq'+i).style.display = 'none';
	    document.getElementById('qqc'+i).style.display = 'block';
	    document.getElementById('qqd'+i).style.display = 'none';
	    document.getElementById('qqe'+i).style.display = 'none';
	    document.getElementById('qqf'+i).style.display = 'none';
	    document.getElementById('qqg'+i).style.display = 'none';
        } else {
            document.getElementById('qqc'+i).style.display = 'none';
        }
    }
    return false;
}

I am also now developing in Visual Studio and the code block below is having problems. Basically on load the image loads fine, but when I roll-over it the image disappears, when a new image should show instead.

<div class="col span_1_of_4">
<a href="#" onClick="showHide(3);" id="map3">
<img src="~/Images/Contact/MiddleEast-Africa/Off/Middle_East_&_Africa_Grey.png" onMouseOver="this.src='~/Images/Contact/MiddleEast-Africa/On/Middle_East_&_Africa.png'" onMouseOut="this.src='~/Images/Contact/MiddleEast-Africa/Off/Middle_East_&_Africa_Grey.png'" alt="" />
</a>
</div>
2

Start with the standard technique of first hiding everything, then showing only what you’re interested in.

  for(i=1;i<=7;i++){
    document.getElementById('qq'+i).style.display = 'none';
    document.getElementById('qqb'+i).style.display = 'none';
    document.getElementById('qqc'+i).style.display = 'none';
    document.getElementById('qqd'+i).style.display = 'none';
    document.getElementById('qqe'+i).style.display = 'none';
    document.getElementById('qqf'+i).style.display = 'none';
    document.getElementById('qqg'+i).style.display = 'none';
    if (i == obj) {
      document.getElementById('qqc'+i).style.display = 'block';
    }
  }

We can then migrate the getElementById over to querySelector

    document.querySelector('#qq'+i).style.display = 'none';
    document.querySelector('#qqb'+i).style.display = 'none';
    document.querySelector('#qqc'+i).style.display = 'none';
    document.querySelector('#qqd'+i).style.display = 'none';
    document.querySelector('#qqe'+i).style.display = 'none';
    document.querySelector('#qqf'+i).style.display = 'none';
    document.querySelector('#qqg'+i).style.display = 'none';

We can then change that code so that we get all of the appropriate elements at the same time, and hide them all.

    var subs = document.querySelectorAll(
      "#qq" + i + ", #qqb" + i + ", " +
      "#qqc" + i + ", #qqd" + i + ", " +
      "#qqe" + i + ", #qqf" + i + ", " +
      "#qqg" + i);
    subs.forEach(function (sub) {
    	sub.style.display = "none";
    });

We can then move those subIds out to a separate variable, and use an array to form those ids instead.

    var subIds = ["", "b", "c", "d", "e", "f", "g"].map(function (suffix) {
      return "#qq" + suffix + i;
    });
    var subs = document.querySelectorAll(subIds);

And we can then remove some of the variable names, inlining with arrow notation instead:

    var suffixes = ["", "b", "c", "d", "e", "f", "g"];
    document.querySelectorAll(suffixes.map(suffix => "#qq" + suffix + i))
      .forEach(sub => sub.style.display = "none");

And you end up with code that looks something like this:

function subShowHidec(obj){
  for (i = 1; i <= 7; i += 1) {
    var suffixes = ["", "b", "c", "d", "e", "f", "g"];
    document.querySelectorAll(suffixes.map(suffix => "#qq" + suffix + i))
      .forEach(sub => sub.style.display = "none");
    if (i == obj) {
      document.querySelector("#qqc" + i).style.display = "block";
    }
  }
  return false;
}
3 Likes
3

Wow, thanks Paul. I’ll go through this now and make the changes.

4

Note: this is somewhat counter-intuitive, why should an object equal a number?

5

Yes, that is counter intuitive. I kept his naming there without changing it.
A better name for it would be subIndex.

8

This topic was automatically closed 91 days after the last reply. New replies are no longer allowed.