1
\$\begingroup\$

I have the following function written in JS:

if (filterButton.classList.contains("selected")) {
    for ( var j=0; j < marker.length; j++ ) {     
       marker[j].visible = true;
       marker[j].hotspot = true;
} 
    filterButton.classList.remove("selected");
} else {  
    for ( var j=0; j < marker.length; j++ ) {            
       if (marker[j].category !== category){
           marker[j].visible = false;
           marker[j].hotspot = false;
       } else {
           marker[j].visible = true;
           marker[j].hotspot = true;
       } 
   }
    for (var f=0; f<filter.length; f++) {
        filter[f].classList.remove("selected");
   }
    filterButton.classList.add("selected");
}

Due to the excessive number of "for loops" I want to convert it to Jquery as it would be neater. I have made some progress, however I am not sure how to change the marker[j] to a jquery selector. marker is an array of objects, and I am not experienced with how to change it to Jquery.

Here is what I have done so far:

if (filterButton.hasClass("selected")) {
    for ( var j=0; j < marker.length; j++ ) {     
       marker[j].visible = true;
       marker[j].hotspot = true;
} 
    filterButton.removeClass("selected");
} else {  
    for ( var j=0; j < marker.length; j++ ) {            
       if (marker[j].category !== category){
           marker[j].visible = false;
           marker[j].hotspot = false;
       } else {
           marker[j].visible = true;
           marker[j].hotspot = true;
       } 
   }
   $('.filter').removeClass("selected");
   filterButton.addClass("selected");
}
\$\endgroup\$
3
  • 1
    \$\begingroup\$ What exactly does this code do? Rather then tell us you are converting from plain JavaScript to jQuery, explain what you are using the code for. \$\endgroup\$ Commented Apr 13, 2022 at 12:12
  • 3
    \$\begingroup\$ I don't understand this aversion to "for loops". They are not even nested, so what's the problem? Anyway, you don't use JQuery to get rid of "for loops", you can use standard Javascript array functions for that, just remember: For them to function, they need to loop over an array. \$\endgroup\$ Commented Apr 13, 2022 at 13:16
  • 1
    \$\begingroup\$ As the others said, using jQuery won't "remove loops", it just hides them better. Additionally if your goal by "removing loops" is to optimize/speed up your code, then jQuery is the wrong direction. jQuery is a heavy-weight library which will only slow down your code and make the download larger. \$\endgroup\$ Commented Apr 20, 2022 at 21:04

1 Answer 1

Reset to default
2
\$\begingroup\$

You aren't going to eliminate the loops, but you can push some of those loops into the jQuery API. The only loop that can be eliminated is setting the visibility and hotspot properties to true inside the if:

if (filterButton.hasClass("selected")) {
    marker.prop({ visible: true, hotspot: true });
    filterButton.removeClass("selected");
}

The jQuery.prop function accepts an object consisting of key-value pairs so you can set multiple DOM properties in one method call. Of course, this still performs a loop over the marker collection, but at least your code doesn't need to write it.

In the else statement, you can use the jQuery.each function. Again, a loop is still executed, but you aren't the one writing this code.

} else {
    marker.each(() => {
        this.visible = this.category === category;
        this.hotspot = this.category === category;
    });

   $('.filter').removeClass("selected");
   filterButton.addClass("selected");
}

The most notable change is removing the if-else statement that sets the visible and hotspot properties based on the category. Remember that every comparison operator in JavaScript returns a boolean value. You can simply assign that directly to a property in the form of:

obj.boolProperty = a === b
obj.boolProperty = a !== b
obj.boolProperty = a < b
// etc...

The full code with my suggestions is below:

if (filterButton.hasClass("selected")) {
    marker.prop({ visible: true, hotspot: true });
    filterButton.removeClass("selected");
} else {
    marker.each(() => {
        this.visible = this.category === category;
        this.hotspot = this.category === category;
    });

   $('.filter').removeClass("selected");
   filterButton.addClass("selected");
}

Lastly, I recommend renaming marker to markers if you expect there to be more than one element in that jQuery collection. Technically each jQuery object is a collection, but name the variable according to your expectations. The filterButton variable name is appropriate if you believe that jQuery object should only have one element in it. Since the original code assumes marker has multiple elements, I recommend a pluralized name for the variable.

Instead of thinking "get rid of excessive loops" think "write idiomatic jQuery". For that, consult the jQuery documentation.

\$\endgroup\$

You must log in to answer this question.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.