Simple animation with Javascript and JQuery

Question

I'm learning Javascript and JQuery, and just finished a chapter on JQuery animations.So i decided to give it a try and implement a simple animation on Html document i had.

Here is the code:

jQuery(document).ready(function($) {
 var animInfo = {
 iter : 1, // Iterator
 elementCount : 4, // Number of ( <div class="soc(?)"></div><p></p> ) structures to animate
 socAnimSpeed : 500, // soc - Section One Content Animation Speed
 socpAnimSpeed: 1000 // scop - Section One Content <p> Animation Speed
 };
 animSoc( animInfo ); // Start the Animations
});
/* Logic Behind the Functions
 function
 animSoc(obj) ---- If Iterator is <= to Number of Elements,<<<<<< 
 | v ^ 
 | v ^ 
 ---- Animate Section One Content( Iter value ) ^
 ( Width ) ^
 v ^
 function v ^
 animSocp(obj) --- Animate Section One Content( Iter value ) <p> ^
 | ( FadeIn ) ^ 
 | v ^ 
 | v ^ 
 ---- Increase the Iterator by One >>>>>>>>>>>>>>>
*/
function animSoc( obj ){ 
 if ( obj.iter <= obj.elementCount ){
 $( ".soc" + obj.iter ).animate( { width: "60%" }, obj.socAnimSpeed, 
 function(){
 animSocp( obj );
 }); 
 }
 return;
}
function animSocp( obj ){
 $( ".soc" + obj.iter + " + p" ).fadeIn( obj.socpAnimSpeed, 
 function() {
 obj.iter++;
 animSoc( obj );
 });
}

The things I am most concerned about :

• Is this considered bad practice ? If yes why ?
• Commenting of the Code Sample. Is it good or bad ?
• Errors.
• Your professional opinion.

Answer 1

Naming

Use meaningful names.

• config sounds better than animInfo.
• iter- Although the comment states what the variable contains it's better to name it iterator and remove the comment.
• animSoc: What does it do? Animate the width of element to some %. This can be named as animateWidth.
• Similarly, animSocp can be renamed to fadeInParagraph^{This might need comment to explain its purpose}

Globals. Required?

animSoc and animSocp are global variables. As, these functions are only called from ready they can be moved inside it. See Why are globals bad?

Once, these are moved inside ready the parameters to the function need not to be passed as they will be accessible.

Passing $ as argument to ready

This is done when there are other libraries which uses $ to access it's features or $ is global variable defined. When $ passed as argument to ready, instead of using jQuery each time $ can be used(saves typing). Here, $ is used outside of ready. This means, there are no conflicts on $ and thus using jQuery to refer to jQuery object is not required and passing $ as parameter is not required.

return;

At the end of animSoc, return statement is used but nothing is returned. Also, when this function is called the returned value is never used, so, it can be removed.

Static elementCount

What if tomorrow other developer(or future you) remove/add some elements in the page but forgot to update this value?. I'll suggest to use jQuery to get the number of elements in DOM.

elementCount: $('div.myClass').length,

$('div[class^="soc"]').length can also be used when there is only one class applied to the elements. As, corresponding HTML code is not shown, I'll suggest to add one more class to all elements and use it as selector. In such case eq can be used which works with index.

jslint/jshint

Use JSLint/JSHint regularly to check for errors and potential problems in code.

Here's code with above changes.

$(document).ready(function() {
 'use strict';
 var config = {
 iterator: 1,
 elementCount: $('div[class^="soc"]').length,
 socAnimSpeed: 500, // soc - Section One Content Animation Speed
 socpAnimSpeed: 1000 // scop - Section One Content <p> Animation Speed
 };
 animateWidth(config);
 function animateWidth() {
 if (config.iterator <= config.elementCount) {
 $(".soc" + config.iterator).animate({
 width: "60%"
 }, config.socAnimSpeed, fadeInParagraph);
 }
 }
 function fadeInParagraph() {
 $(".soc" + config.iterator + " + p").fadeIn(config.socpAnimSpeed,
 function() {
 config.iterator++;
 animateWidth(config);
 });
 }
});

Answer 2

Nothing wrong with it specifically. The circular calls are a bit confusing, but it has a simple enough logic to figure out what's going on.

One thing I would do is rename the function names to make them better reflect their purpose (animSoc and animSocp). For example animateToWidth and animateFadeIn or something similar.