How can i optimize this JavaScript

Question

I'm using the following JavaScript to toogle some information in my menu.

Is it possible to optimize it so I only have one function instead of n?

$(document).ready(function(){
 $("#km1").click(function(){
   $("#km1tog").slideToggle("slow");
 });
}); 

$(document).ready(function(){
 $("#km2").click(function(){
   $("#km2tog").slideToggle("slow");
 });
}); 

$(document).ready(function(){
 $("#km3").click(function(){
   $("#km3tog").slideToggle("slow");
 });
});

give them a class and use that instead of the id. $('.km').slideToggle('slow'); — Mark Walters
– Mark Walters, Commented Apr 11, 2014 at 9:45
@MarkWalters class will hide all but OP want only particular item to hide. — Zaheer Ahmed
– Zaheer Ahmed, Commented Apr 11, 2014 at 9:46
@ZaheerAhmed. Sorry I meant. $('.km').click(function() { $(this).slideToggle('slow'); }); — Mark Walters
– Mark Walters, Commented Apr 11, 2014 at 9:47

thefourtheye · Accepted Answer · 2014-04-11 09:45:02Z

3

You can use for loop and create an IIFE, to retain the value of i.

$(document).ready(function() {
    for (var i = 1; i <= 3; i += 1) {
        (function(i) {
            $("#km" + i).click(function() {
                $("#km" + i + "tog").slideToggle("slow");
            });
        }(i));
    }
});

answered Apr 11, 2014 at 9:45

thefourtheye

241k53 gold badges466 silver badges505 bronze badges

Sign up to request clarification or add additional context in comments.

Comments

Zaheer Ahmed · Accepted Answer · 2014-04-11 09:44:02Z

3

try:

$(document).ready(function(){
  $("#km1,#km2,#km3").click(function(){
    $("#"+this.id+"tog").slideToggle("slow");
  });
});

answered Apr 11, 2014 at 9:44

Zaheer Ahmed

28.7k12 gold badges77 silver badges113 bronze badges

Comments

beeglebug · Accepted Answer · 2014-04-11 09:48:41Z

1

For a start you only need one $(document).ready call, so that will optimise it slightly. As for the rest, it depends greatly on the structure of the markup.

Ideally you wouldn't use ID's on the elements, you would simply reference them by their relationship, for example:

$(document).ready(function(){
    $(".top-menu").click(function(){
        $(this).find(".sub-menu").slideToggle("slow");
    });
});

answered Apr 11, 2014 at 9:48

beeglebug

3,5421 gold badge25 silver badges40 bronze badges

Comments

putvande · Accepted Answer · 2014-04-11 09:48:01Z

0

Have you tried this?

$(document).ready(function() {
    $.each([1,2,3,4,5,6,7,8,9,10], function(i) {
        $("#km" + i).click(function(){
            $("#km" + i + "tog").slideToggle("slow");
        });
    });
});

edited Apr 11, 2014 at 9:48

putvande

15.3k3 gold badges37 silver badges51 bronze badges

answered Apr 11, 2014 at 9:44

filmor

32.6k6 gold badges53 silver badges48 bronze badges

3 Comments

Cerbrus Over a year ago

$.each with a hardcoded array instead of a for loop? Wut?

filmor Over a year ago

Do you know whether it is hardcoded in the actual example?

Cerbrus Over a year ago

The OP hardcoded 3 id's. That's a bad practice, and shouldn't be encouraged. Especially if the suggested alternative is to hardcode all iterations of a simple for loop. Your code is saying: for(var key in [1,2,3,4,5,6,7,8,9, 10]) instead of for(var i = 1; i <= 10; i++)

Jason Droth · Accepted Answer · 2014-04-11 09:57:04Z

0

If possible add a class to all of your "clickable" divs and try something like this:

$(document).ready(function(){
    $(".km").click(function(){
        id = $(this).attr('id');
        $("#"+id+"tog").slideToggle("slow");
    });
});

answered Apr 11, 2014 at 9:57

Jason Droth

111 silver badge4 bronze badges

1 Comment

Cerbrus Over a year ago

$(this).attr('id');... You might not need jQuery: this.id

Collectives™ on Stack Overflow

How can i optimize this JavaScript

5 Answers 5

Comments

Comments

Comments

3 Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

5 Answers 5

Comments

Comments

Comments

3 Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related