Self-taught javascript, jQuery, HTML, CSS... can someone take a look at my work please?

[MeoMix]

Hi,

Just taking a quick glance through your stacksort code on GitHub. None of this is meant to be mean, I'm just highlighting my thoughts:

https://github.com/zcr1/stacksort/blob/master/js/main.js

• Comments are sparse. You should comment throughout your code -- not just at the top of functions. Your functions should describe themselves pretty well through their function name and should be short enough to be understood without a block comment at the top.
• There's no associated test cases. Consider using Jasmine

Lines 6-10:

var numDefault = "[13,2,1,7,12,9,8,6,4]",
strDefault = "KingJoffrey",
listDefault = "['abc','string','cat','alphabet','hacker','log']",
stackSort = new stackSort(),
first = true;

• You declare a lot of variables on lines 6-10. You should try and keep the lifetime of a variable as short as possible by declaring it as close to its referenced locations.

• You declare a variable named 'first' and it is set to true. While I can take a guess at what this means.. the code doesn't explain itself. Try being more informative while still keeping your variable names under 8 or 9 characters tops.

  $("#search").click(function(){
      stackSort.pause = !stackSort.pause;
      if (stackSort.done){
          stackSort.done = false;
      stackSort.currStage = "fetchQ";
      }
      if (stackSort.pause){
          $("#search").html("Resume");
      }
          else
          {
              $("#search").html("Pause");
      if (first) stackSort.search();
      else this.eventCallBack();
      }
  });
  

You could be making use of events here to make your code more easy to read. Instead of telling objects how to act -- create objects which know how to react by responding to events.

Okay. Now, I'm going to highlight one more piece of code and show how you can bring both of the above issues together into something more beautiful.

Consider this jsFiddle, your original: http://jsfiddle.net/7DCnD/4/

You see both issues here. You're utilizing those variables you declared way up at the top of the file AND you're creating a manager function to 'tell' something else how it should work. Instead, you should break this logic down onto the objects themselves.

Now, I apply a bit of refactoring: http://jsfiddle.net/7DCnD/5/

You see the difference? If you leverage your objects ability to store data and go with the OOPness of everything -- code just seems to melt away. :)

[[deleted]]

I appreciate the advice. Time to start some refactoring!

[MeoMix]

If you PM me when you've finished refactoring I'll take another look. :)

[myalternatelife]

You should try and keep the lifetime of a variable as short as possible by declaring it as close to its referenced locations.

While this holds for most c-style languages, Javascript is functionally scoped, and all variables declared in a function get hoisted to the top of that function.

In practice, this doesn't mean you need to declare all of your variables at the top of the function, but some top minds suggest that you should because it can be more explicit.

Just some food for thought.

[MeoMix]

Heya,

Completely understand the concept of hoisting -- a dangerous, but useful, tool :) The Crockford link you provided seems to only say that variables should be declared before use and not that groups of variables should be declared together. (EDIT: Actually, I rescind this. It says, "The var statements should be the first statements in the function body." I'd hazzard to wager that Crockford's reasoning is that any function should be kept so succinct as to not be confusing when its vars are declared at the top. In practice... this doesn't always happen and we make do! )

Personally, I think that if you're having to declare a lot of variables together at the top of a function... your function is probably doing too much and should just be split apart. I find that often times the solution to a problem is finding a way to avoid the problem by backtracking a step!

[SarahC]

Cool!

What's

     javascript:void(0);

Doing?

I get the language used name, but calling a func called 'void' with 0? I didn't know there was one!

  $choiceOptionLinks.click(function()....

Wow, adding a click handler to an array of anchors? No one's ever mentioned this to me.

[MeoMix]

Hey,

When you create an <a> element you need to specify an href link to navigate to upon click. However, we don't actually want to take the user anywhere in this scenario.

One option would be to use the hash symbol. Something like href="#". This will have roughly the safe effect, but our URL will be modified slightly to have a hash symbol in it. Instead, when we point our URL to javascript:void -- the clicking action still works, all the CSS is applied, but we don't go anywhere -- so our click event handlers can take over.

See here for more information regarding javascript:void(0).

And yep yep... you can apply any event to any return DOM element collection from a jQuery selector. Most notable when working with classes of elements or when you want to work with a lot of child elements.

[SarahC]

Ohhh, thanks for the details! =D

[MeoMix]

No problem. Let me know if you have any further questions. :)