# How to optimize this code in jQuery?

I have the following code to get the ID of a <a href="#ID"> </ a> and go to their respective div <div id="" />:

$('a[href=#certificados]').click(function(e){ e.preventDefault();$("html, body").animate({ scrollTop: $('#certificados').offset().top }, 1000); });$('a[href=#team]').click(function(e){
e.preventDefault();
$("html, body").animate({ scrollTop:$('#team').offset().top }, 1000);
});

$('a[href=#house]').click(function(e){ e.preventDefault();$("html, body").animate({ scrollTop: $('#house').offset().top }, 1000); });$('a[href=#contact]').click(function(e){
e.preventDefault();
$("html, body").animate({ scrollTop:$('#contact').offset().top }, 1000);
});


How could I optimize the code?

-

## migrated from stackoverflow.comJan 14 at 13:57

This question came from our site for professional and enthusiast programmers.

Get the "respective" div in a more general way from the link's href attribute:

$('a[href=#certificados], a[href=#team], a[href=#house], a[href=#contact]').click(function(e){ e.preventDefault();$("html, body").animate({ scrollTop: $(this.hash).offset().top }, 1000); // this.hash would be equivalent to$(this).attr("href") in your case
});


Probably you also can use a much better selector now - maybe selecting those links by a common class.

-
Bad thing about this solution is maintaining the list of items and the slower look-up. You would be better off using a common classname. –  epascarello Jan 14 at 14:25
@epascarello: That's what I suggested in the last sentence :-) –  Bergi Jan 14 at 14:36
Just hoping the OP saw it. :) –  epascarello Jan 14 at 14:46

If you want to make the code DRYer, you can do

['#certificados', '#team', '#house', '#contact'].forEach(function(anchor){
$("a[href="+anchor+"]").click(function(e){ e.preventDefault();$("html, body").animate({ scrollTop: $(anchor).offset().top }, 1000); }); })  If all internal links are to be handled this way, do $('a[href^="#"]').click(function(e){
e.preventDefault();
$("html, body").animate({ scrollTop:$(this.hash).offset().top }, 1000);
});

-
Why a forEach? jQuery supports multiple elements in a selector! –  epascarello Jan 14 at 14:01
@epascarello to not compute n and to be dryer on the a[href=...] thing –  dystroy Jan 14 at 14:01
Maybe rename n to anchor ?, +1 –  konijn Jan 14 at 14:04
this.href does not work –  Bergi Jan 14 at 14:14
this.href returns the full URL, You would need to use this.getAttribute("href") or $(this).attr("href"); or hash jsfiddle.net/7439C/1 – epascarello Jan 14 at 14:22 You can make an extension, that will let you specify what link scrolls to where, without creating a hard dependancy between the href attribute and the target identity: $.fn.scrollTo = function(target){
return this.click(function(e){
e.preventDefault();
$("html, body").animate({ scrollTop:$(target).offset().top }, 1000);
});
};

$('a[href=#certificados]').scrollTo('#certificados');$('a[href=#team]').scrollTo('#team');
$('a[href=#house]').scrollTo('#house');$('a[href=#contact]').scrollTo('#contact');

-
Nice, but this doesn't optimize it at all. –  Silviu Burcea Jan 14 at 22:36
@SilviuBurcea: Funny that you should single out this answer and point out that it won't optimise the code (and I assume that you only mean optimise for speed, not any other aspect), as neither of the previous answers does that either... –  Guffa Jan 14 at 23:13

Add a class to the anchors, like scrollable.

$('.scrollable').click(function(e){ e.preventDefault();$("html, body").animate({ scrollTop: \$(this.hash).offset().top }, 1000);
});


Each is called under the hood, no crazy functions, just KISS.

-