Various animation functions

Question

I been learning web development for 1.5 month now, and I am trying to improve my code. I can achieve what I want, but I think my code is really bad.

For example, I have a bunch of jQuery animations and a bunch of function to 'stop' those animations when another animation is starting. The code seems very inelegant, so how should I improve it? I been learning how to write a jQuery plugin, but I don't think it will help in this case.

$(document).ready(function(){
 $("#diagonalLine").show(500);
 $("#line").show(1300);
 $("#start").show(1500);
 $("#centerButton").click(function(){
 $("#diagonalLine").hide(1500);
 $("#line").hide(1300);
 $("#start").hide(500);
 fadeInWaterloo();
 fadeInToronto();
 fadeInTop();
 $("#selectLine").animate({width:'toggle'},1250);;
 $("#diagonalSelectLine").show(1450);
 $("#select").show(1600); 
 });
 $("#waterloo7").mouseenter(function(){
 fadeInTorontoStop();
 fadeInTopStop();
 fadeOutToronto();
 fadeOutTop();
 });
 $("#toronto5").mouseenter(function(){
 fadeInWaterlooStop();
 fadeInTopStop();
 fadeOutWaterloo();
 fadeOutTop();
 //$("#flip").show(1000);
 //$("#infoBox1").slideDown(2000);
 });
 $("#campus").mouseenter(function(){
 fadeInWaterlooStop();
 fadeInTorontoStop();
 fadeOutWaterloo();
 fadeOutToronto();
 });
 $("#waterloo7").mouseleave(function(){
 fadeOutTorontoStop();
 fadeOutTopStop();
 fadeInToronto();
 fadeInTop();
 }); 
 $("#toronto5").mouseleave(function(){
 fadeInWaterloo();
 fadeInTop();
 });
 $("#campus").mouseleave(function(){
 fadeInToronto();
 fadeInWaterloo();
 });
 $("#log-in-button").click(function(){sendLogin(); return false;});
 $("#forgotLogin").click(function(){location.href='signUp.html';});
 $("#signUp").click(function(){location.href='signUp.html';});
});
//-------------------------------------Toronto Animation Functions--------------------------
function fadeInToronto(){
 for (var i=1;i<=5; i++){
 $("#toronto"+i).fadeIn(300+i*200);
 }
}
function fadeOutToronto(){
 for (var i=1; i<=5; i++){
 $("#toronto"+i).fadeOut(1100-i*200);
 }
}
function fadeInTorontoStop(){
 for (var i=1;i<=5; i++){
 $("#toronto"+i).fadeIn().stop();
 }
}
function fadeOutTorontoStop(){
 for (var i=1; i<=5; i++){
 $("#toronto"+i).fadeOut().stop();
 }
}
//-----------------------------------End of Toronto Animation---------------------------
//-----------------------------------Waterloo Animation Functions----------------------------
function fadeInWaterloo(){
 $("#waterloo1").fadeIn(0);
 $("#waterloo2").fadeIn(300);
 $("#waterloo3").fadeIn(1300);
 $("#waterloo4").fadeIn(1600);
 $("#waterloo5").fadeIn(1800);
 $("#waterloo6").fadeIn(2000);
 $("#waterloo7").fadeIn(2000);
}
function fadeInWaterlooStop(){
 for (var i=1;i<=7; i++){
 $("#waterloo"+i).fadeIn().stop();
 }
}
function fadeOutWaterloo(){
 var num = 1100;
 for (var i=1; i<=7; i++){
 $("#waterloo"+i).fadeOut(num);
 num = num-200;
 }
}
function fadeOutWaterlooStop(){
 for (var i=1;i<=7; i++){
 $("#waterloo"+i).fadeOut().stop();
 }
}
//----------------------------------------End of Waterloo Animation-------------------
//-------------------------------------Campus Animation Functions--------------------
function fadeInTop(){
 for (var i=1;i<=4; i++){
 var id = "#top"+i;
 $(id).fadeIn(300+i*200);
 }
 $("#campus").fadeIn(1100);
}
function fadeOutTop(){
 $("#top1").fadeOut(1100);
 $("#top2").fadeOut(1100);
 $("#top3").fadeOut(900);
 $("#top4").fadeOut(700);
 $("#campus").fadeOut(500);
}
function fadeInTopStop(){
 for (var i=1;i<=4; i++){
 var id = "#top"+i;
 $(id).fadeIn().stop();
 }
 $("#campus").fadeIn().stop();
}
function fadeOutTopStop(){
 $("#top1").fadeOut().stop();
 $("#top2").fadeOut().stop();
 $("#top3").fadeOut().stop();
 $("#top4").fadeOut().stop();
 $("#campus").fadeOut().stop();
}
//-------------------------------------End of Campus Animation---------------

• elclanrs: The first step to improve your code is to get rid of id's and use common classes.

Answer 1

The general rule is if you're doing something with a static element inside a event, you should cache that reference to this element.

$("#button").click(function(){
 $("#diagonalLine").hide(1500);
}

or

document.getElementById('button').onclick = function(){
 document.getElementById('diagonalLine').style.display='none';
}

Everytime the user click on #button it traverses the DOM to getElementById and find #diagonalLine. With jQuery there's much more overhead involved, it create a new jQuery object, parses the string #diagonalLine, find the element, bind it to the new object and returns it. There are all sort of thing jQuery do under hood to make it "magic".

var diagonalLine = $("#diagonalLine");
$("#centerButton").click(function(){
 diagonalLine.hide(1500);
}
var diagonalLine = document.getElementById('diagonalLine');
document.getElementById('centerButton').onclick = function(){
 diagonalLine.style.display='none';
}

You can also use closures for caching references to elements

$('#button').click=(function(){
 var diagonalLine = $("#diagonalLine");
 return function(){
 diagonalLine.hide(1500);
 }
})();
document.querySelector('#button').onclick=(function(){
 var diagonalLine = document.querySelector('#diagonalLine');
 return function(){
 diagonalLine.style.display='none';
 }
})();

• icktoofay: To be pedantic, getElementById is usually implemented using a hash table rather than traversing the DOM, which makes it fairly quick. That said, constructing the jQuery object may make it much more expensive as you point out.
• Roman Susi: It may be good practice to name variables with dollar prefix to get a hint it's jQuery object: var $diagonalLine = $("#diagonalLine")
• Victor: I think it comes to personal preference, IMO it adds pollution to the code.

Answer 2

You could certainly use classes in many places to decrease code duplication. For instance, you could add a top class to all of your "top" elements. In that way, you could stop and fadeOut all in one line:

$(".top").fadeOut().stop();

Similarly, this:

for (var i=1;i<=4; i++){
 var id = "#top"+i;
 $(id).fadeIn().stop();
}

would be just this:

$(".top").fadeIn().stop();