Skip to main content
Code Review

Return to Answer

replaced http://stackoverflow.com/ with https://stackoverflow.com/
Source Link

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? Why are globals bad?

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?

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?

Source Link
Tushar
  • 3k
  • 1
  • 22
  • 28

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 fadeInParagraphThis 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);
 });
 }
});
default

AltStyle によって変換されたページ (->オリジナル) /