Dry Javascript Code

Newbie trying to learn javasacript and jquery. Can someone help me DRY out this code. I get high CPU usage when running this code on my html website. Any help is greatly appreciate

Solution 1:

Some things I noticed:

• In the first block of code, I notice that they all have position:absolute and opacity:1. Why not define these styles into a class which all these elements will have.

• Calling animate is redundant. Why not make a "map" of selector and options and just loop through it. Saves you from the redundant animate calls. I'd do classes but the problem is that each element has different options.

• The second block, that starts with $(function () { doesn't need to be wrapped in $(function () {. In fact, that is just a shorthand for $(document).ready().

• setInterval is a crude way to check for visibility of the elements. You should consider using DOM Mutation Events. Disclaimer: It's not implemented on all browsers yet.

• Also, if you happen to notice, your interval is set to run forever. You didn't include a way for the timer to kill itself, it will continually query the DOM for elements. Querying the DOM is a slow process. You should set something like a flag to indicate that everything is on screen and should kill the timer.

• The interval of 1ms is overkill. Some browsers cap this to 4ms. Also, humans consider 200ms or faster as "instant" and 400-600ms as acceptable lag.

• Since you have a static set of elements, why not cache them? Store them in an array, along with the options.

All in all, it should look something like this:

var targets = [
  {
    element : $('#copy2'),
    initialAnimationOptions : {...},
    onScreenAnimationOptions : {...}
  },
  ...
];

// Initial animation
$.each(targets,function(index,target){
  target.element.animate(target.initialAnimationOptions);
});

// The crude checkersetInterval(function(){
  $.each(targets,function(index,target){
    if(target.element.is(':onScreen') target.element.animate(target.onScreenAnimationOptions);
  });
},1000);

Solution 2:

I'd suggest trying CSS transitions, and using $(el).css instead of $(el).animate.

Solution 3:

At the very least, you can get rid of the absolute and opacity parts of the animations because they're not changing anything.

Second, use some better classes so that you change like elements at the same time. Alternatively, you can pass selectors separated by commas to join similar calls. For example:

$(".media-nav3, .media-nav4, .media-nav5, .media-nav6").animate({ 'top': '400px'}, 1000); // 1000 milliseconds

Third, translate is more efficient, as discussed here.

The main problem is that you are making way too many calls to jQuery, each of which carries significant overhead.