jQuery code critque

Question

Thought I'd post here. My first hour on jQuery, actually first programing ever done. Would love to learn whats not right and how it could be better.

$(function() {

function hide_me()
//A place to specify which elements you want hidden, on page load.
{
    $("li.credentials").hide();

}
function first_bow()
//The div right_column takes a bow on initial load.
{
    $("div#right-column").show("drop");

}
function bigpeek()
//The third column toggles in/out. All elements under div right_column.
{
    $("div#right-column").toggle("drop", "fast");

}

function smallpeek()
//Smaller snippets like credentials or user assignment flying in/out.
{
    $("li.credentials").toggle("drop", "fast");

}

$(document).ready(function() {
    $("*").ready(hide_me);
    $("*").ready(first_bow);
    $(".btn-new-email").click(bigpeek);
    $(".button").click(smallpeek);
    $(".icon-delete").mouseover(function() {
        $(this).effect("bounce", "fast");
    });
});
});

Thanks, didn't know it existed. Will use that for the next run. — DSM, Mar 29 '11 at 02:30

score 2 · Answer 1 · answered Mar 29 '11 at 02:27

2

The best thing to learn about programming is how to effectively re-use code. In your code, you have set up some functions that you yourself claim will do a bunch of the same thing. So instead, you could make it better by only writing code to do the repeated task once.

For one example, instead of creating a function where you place a bunch of things that need to be hidden, I would add a class to the elements that should be hidden, and then hide all those elements:

function hide_me()
//Hides anything with the "hide-me-onload" class
{
    $(".hide-me-onload").hide();
}

answered Mar 29 '11 at 02:27

Sean Adkinson

8,425
3
45
64

Thanks, Sean. Is that a CSS class? – DSM Mar 29 '11 at 02:30
@DSM - yes, but there doesn't have to be any CSS rules defined for it. Often, when using jQuery, you'll assign non-existent css classes to your DOM elements to serve as markers and/or state. – Brandon Mar 29 '11 at 02:40

Ketan · Answer 2 · 2011-03-29T17:02:44.650

2

$(function () {
 ...
}

is the same as

$(document).ready(function() {
 ...
}

So you can move the method calls from inside your $(document).ready() to be inside your $(function(){}). Also try to use IDs instead of class names wherever possible. Something like this will go through the entire DOM to look for an element

$(".item")

Be more specific

$("#itemID") // use IDs instead of Classes
//If you have to use class name then you can speed up the selector by adding the element tag before it
$("div.item")

edited Mar 29 '11 at 17:02

answered Mar 29 '11 at 04:56

Ketan

4,270
1
15
9

$(function () { ... } is not even close to $(document).ready(function() { ... } First is a method call right after the method is loaded into browser; the second is running after the whole page is loaded. – Andrey Mar 29 '11 at 16:56
damn, I didn't know that! +1. Can you edit your answer so I can change my vote on it? It locked my vote unless you change it. – Andrey Mar 29 '11 at 17:00

score 1 · Answer 3 · answered Mar 29 '11 at 05:05

Using $("*").ready() within $(document).ready() is redundant... you already know using all of the elements are ready! Also, in general using the universal selector $('*') is very inefficient.

So, the first two lines of your $(document).ready() can just be:

hide_me();
first_bow();

Other than that and a couple of issues with organization and nomenclature you're off to a great start, keep it up!

jQuery code critque

3 Answers3