Delay and queue animation in a function - how can I optimise this by removing duplicate code?

Question

I have this script that moves a box around the screen and the loops the second part of the movement. I have altered the code to drop another three boxes (4 in total) into the animation so the boxes follow each other around the screen.

I want it to work exactly like it does, but I'm sure there's a much better way of doing this:

Here's a js fiddle: http://jsfiddle.net/NF6LU/

JS

function animateNode() {
 $('.node').animate({top: '425px'}, { duration: 1800, easing : 'linear', queue: true });
 $('.node2').animate({top: '425px'}, { duration: 1800, easing : 'linear', queue: true });
 $('.node3').animate({top: '425px'}, { duration: 1800, easing : 'linear', queue: true });
 $('.node4').animate({top: '425px'}, { duration: 1800, easing : 'linear', queue: true });
 $('.node').animate({marginLeft: '-284px'}, { duration: 2500, easing : 'linear', queue: true });
 $('.node2').animate({marginLeft: '-284px'}, { duration: 2500, easing : 'linear', queue: true });
 $('.node3').animate({marginLeft: '-284px'}, { duration: 2500, easing : 'linear', queue: true });
 $('.node4').animate({marginLeft: '-284px'}, { duration: 2500, easing : 'linear', queue: true });
 $('.node').animate({top: '157px'}, { duration: 1800, easing : 'linear', queue: true });
 $('.node2').animate({top: '157px'}, { duration: 1800, easing : 'linear', queue: true });
 $('.node3').animate({top: '157px'}, { duration: 1800, easing : 'linear', queue: true });
 $('.node4').animate({top: '157px'}, { duration: 1800, easing : 'linear', queue: true });
 $('.node').animate({marginLeft: '264px'}, { duration: 2500, easing : 'linear', queue: true });
 $('.node2').animate({marginLeft: '264px'}, { duration: 2500, easing : 'linear', queue: true });
 $('.node3').animate({marginLeft: '264px'}, { duration: 2500, easing : 'linear', queue: true });
 $('.node4').animate({marginLeft: '264px'}, { duration: 2500, easing : 'linear', queue: true });
}
$('.node').delay(1500).animate({top: '157px'}, { duration: 1000, easing : 'linear', queue: true });
$('.node2').delay(3000).animate({top: '157px'}, { duration: 1000, easing : 'linear', queue: true });
$('.node3').delay(4500).animate({top: '157px'}, { duration: 1000, easing : 'linear', queue: true });
$('.node4').delay(6000).animate({top: '157px'}, { duration: 1000, easing : 'linear', queue: true });
$('.node').animate({marginLeft: '264px'}, { duration: 1500, easing : 'linear', queue: true });
$('.node2').animate({marginLeft: '264px'}, { duration: 1500, easing : 'linear', queue: true });
$('.node3').animate({marginLeft: '264px'}, { duration: 1500, easing : 'linear', queue: true });
$('.node4').animate({marginLeft: '264px'}, { duration: 1500, easing : 'linear', queue: true });
$(function(){
 animateNode();
 setInterval(animateNode, 2000);
});

HTML

<span class="node"></span>
<span class="node2"></span>
<span class="node3"></span>
<span class="node4"></span>

CSS

span.node, span.node2, span.node3, span.node4{
 height: 16px;
 width: 16px;
 background-color: black;
 position: absolute;
 top: 60px;
 left: 50%;
 margin-left: -9px;
}

Answer 1

You are repeating yourself a ton here, you should read up on DRY.

You could store the animations in an array and then execute those animations :

var PROPERTIES = 0 , OPTIONS = 1;
var animations = 
[ 
 [ {top: '425px'} , { duration: 1800, easing : 'linear', queue: true } ],
 [ {marginLeft: '-284px'} , { duration: 2500, easing : 'linear', queue: true } ],
 [ { top: '157px'} , { duration: 1800, easing : 'linear', queue: true } ],
 [ {marginLeft: '264px'} , { duration: 2500, easing : 'linear', queue: true } ]
] 
function animateNode( $node )
{
 animations.forEach( function (animation)
 {
 $node.animate( animation[PROPERTIES] , animation[OPTIONS] ); 
 });
}
function animateNodes()
{
 animateNodes( $('.node') );
 animateNodes( $('.node2') );
 animateNodes( $('.node3') );
 animateNodes( $('.node4') );
}

Ideally of course, you would store the results of $(...) in a variable and use that variable instead of invoking $(...) every single time. Even more ideally, those variables would be in an array over which you can loop.

You can apply this technique to the rest of your code as well.

• konijn: I refactored my answer, but left that finally part ;)
• David Ingledow: @tomdemuyt I get an Uncaught SyntaxError: Unexpected identifier
• konijn: Yes, missing closing bracket on ` $node.animate( animation[PROPERTIES] , animation[OPTIONS] ); ` fixed now.
• James Montagne: I think you wanted to call animateNode (no s at the end) inside of animateNodes.

Answer 2

I agree with @tomdemuyt's overall approach but would suggest a few additional improvements.

$(function(){ 
 var nodes = [
 $('.node'),
 $('.node2'),
 $('.node3'),
 $('.node4')
 ];
 var animations = [ 
 { 
 properties: {top: '425px'}, 
 options: { duration: 1800, easing : 'linear'} 
 },
 { 
 properties: {marginLeft: '-284px'}, 
 options: { duration: 2500, easing : 'linear'} 
 },
 { 
 properties: { top: '157px'}, 
 options: { duration: 1800, easing : 'linear'} 
 },
 { 
 properties: {marginLeft: '264px'}, 
 options: { duration: 2500, easing : 'linear'} 
 }
 ];
 function animateNode( $node ){
 // call each animation in the array for the given node
 animations.forEach( function (animation){
 $node.animate( animation.properties , animation.options ); 
 });
 // once all animations complete, call this function again
 $node.queue(function(next){
 animateNode($node); 
 next();
 });
 }
 function animateNodes(){
 // call the animateNode function for every node 
 nodes.forEach(function($node){
 animateNode($node);
 });
 }
 // do the initial animation for each node, incrementing the initial delay by 1.5s each node
 for(var i = 0; i<nodes.length; i++){
 nodes[i].delay(1500 * (i + 1))
 .animate({top: '157px'}, { duration: 1000, easing : 'linear'})
 .animate({marginLeft: '264px'}, { duration: 1500, easing : 'linear'});
 }
 // initialize the looping animation
 animateNodes();
});

• Store your nodes in an array to make the solution more generic as well as reduce repeated code. This has the added benefit of caching the jquery objects.
• Make your animations an array of objects instead of an array of arrays. This eliminates the need for PROPERTIES and OPTIONS.
• You have an issue that you are calling animateNode every 2 seconds. However, the duration of your animation created by that function is considerably longer than 2 seconds. As a result, your animations are going to continue to build up. The memory usage will continue to grow for as long as the page is running. Open your console and run this fiddle to see the queue length increase http://jsfiddle.net/NF6LU/8/ . Instead, I have eliminated setInterval all together and used Queue to call animateNode once the previous set of animations completes. Alternatively, you could call it as the callback of the final animation.
• You can eliminate queue: true as that is the default.
• Include all of your code in the ready function to avoid polluting the global space. This also ensure the nodes exist before using them. Your fiddle only worked due to the fact that it was set to run the code onload.

http://jsfiddle.net/NF6LU/10/