DRY javascript code

Question

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 appreciated. How do I declare variables array or even functions for something like this?

$(document).ready(function () {

  $("#copy2").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav2").animate({
    'position': 'absolute',
    'top': '380px',
    opacity: 1
  }, 1);
  $("#copy3").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav3").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy4").animate({
    'position': 'absolute',
    'top': '-380px',
    opacity: 1
  }, 1);
  $(".media-nav4").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy5").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav5").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy6").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav6").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy7").animate({
    'position': 'absolute',
    'top': '10px',
    opacity: 1
  }, 1);
  $(".media-nav7").animate({
    'position': 'absolute',
    'margin-top': '450px',
    opacity: 1
  }, 1);
  $("#copy8").animate({
    'position': 'absolute',
    'top': '10px',
    opacity: 1
  }, 1);
  $(".media-nav8").animate({
    'position': 'absolute',
    'margin-top': '450px',
    opacity: 1
  }, 1);
  $("#copy9").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav9").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy10").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav10").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy11").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav11").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy12").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav12").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy13").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav13").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy14").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav14").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy15").animate({
    'position': 'absolute',
    'top': '-390px',
    opacity: 1
  }, 1);
  $(".media-nav15").animate({
    'position': 'absolute',
    'top': '400px',
    opacity: 1
  }, 1);
  $("#copy16").animate({
    'position': 'absolute',
    'top': '10px',
    opacity: 1
  }, 1);

  $(function () {

    setInterval(function () {

      $("#copy2").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav2").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy3").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav3").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy4").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav4").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy5").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav5").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy6").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav6").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy7").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $(".media-nav7").filter(":onScreen").animate({
        'position': 'absolute',
        'margin-top': '250px',
        opacity: 1
      }, 1700);
      $("#copy8").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $(".media-nav8").filter(":onScreen").animate({
        'position': 'absolute',
        'margin-top': '250px',
        opacity: 1
      }, 1700);
      $("#copy9").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav9").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy10").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav10").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy11").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav11").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy12").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav12").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy13").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav13").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy14").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav14").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy15").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '0px',
        opacity: 1
      }, 1700);
      $(".media-nav15").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700);
      $("#copy16").filter(":onScreen").animate({
        'position': 'absolute',
        'top': '250px',
        opacity: 1
      }, 1700)
    }, 1)

  })

});

I think that the reason you're getting high CPU usage has to do with the fact that you're doing a lot of animations, not because the code is repetitive. If you factored out the redundancies I don't think it'd change anything on CPU usage since it'd be the same visual output anyway. — frenchie
– frenchie, Commented Dec 29, 2013 at 2:42
If you want to reduce CPU usage, I would recommend using CSS transforms instead of jQuery. Here's a link to help you for your rewrite: dev.opera.com/articles/view/css3-vs-jquery-animations — frenchie
– frenchie, Commented Dec 29, 2013 at 2:44
Can you put some or all of the elements in a wrapper and animate just that, or add a class for similar-animated elements? — Appleshell
– Appleshell, Commented Dec 29, 2013 at 2:53

Joseph · Accepted Answer · 2013-12-29 03:10:13Z

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 checker
setInterval(function(){
  $.each(targets,function(index,target){
    if(target.element.is(':onScreen') target.element.animate(target.onScreenAnimationOptions);
  });
},1000);

Josh Crozier · Accepted Answer · 2013-12-29 03:04:45Z

1

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

edited Dec 29, 2013 at 3:04

Josh Crozier

242k56 gold badges401 silver badges316 bronze badges

answered Dec 29, 2013 at 2:48

zsotyooo

312 bronze badges

Comments

Nick · Accepted Answer · 2013-12-29 04:33:16Z

1

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.

edited Dec 29, 2013 at 4:33

answered Dec 29, 2013 at 2:52

Nick

6,04512 gold badges56 silver badges81 bronze badges

Collectives™ on Stack Overflow

DRY javascript code

3 Answers 3

Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related