New at jQuery, lack of structure

Question

My job required me to learn jQuery the last couple of weeks, but it's a mess and I do not know how to structure my code in an acceptable manner. I come from a Java and PHP background and have never touched JavaScript before.

I've searched the web for some guidelines, but I could not find anything of real value.

function answer(id, value) {
    $("input#" + id).val(value);
}

$("table.grid").ready(function() {
    if ($.browser.msie && $.browser.version == "7.0") {
        var z = 1000;

        $("table, tr, th, td, div").each(function() {
            $(this).css("z-index", z);
            z -= 10;
        });
    }

    $("td[rowspan=2]").parent().addClass("no-border");
    $("td.answer-head").prevAll("td.radio").each(function(index) {
        $(this).addClass("answer-tail");
    });
    $("td.radio").click(function() {
        var $that = $(this);
        var $left = $that.prevAll("td.radio").andSelf();
        var $right = $that.nextAll("td.radio");
        var $parent = $that.parent();
        var $cousin = null;

        if ($parent.children().first().attr("rowspan") != undefined) {
            $cousin = $parent.next("tr").children("td.radio").eq($left.length - 1);
        } else if ($parent.prev("tr").children().first().attr("rowspan") != undefined) {
            $cousin = $parent.prev("tr").children("td.radio").eq($left.length - 1);
        }

        if ($cousin !== null) {
            $left = $left.add($cousin).add($cousin.prevAll("td.radio"));
            $right = $right.add($cousin.nextAll("td.radio"));
        }

        $left.each(function(index) {
            var $that = $(this);

            if ($that.hasClass("answer-head")) {
                $that.addClass("click-head");
            } else if ($that.hasClass("answer-tail")) {
                $that.addClass("click-tail");
            } else {
                $that.addClass("click");
            }
        });
        $right.each(function(index) {
            $(this).removeClass("click-head").removeClass("click-tail").removeClass("click");
        });
    }).hover(function() {
        var $that = $(this);
        var $left = $that.prevAll("td.radio").andSelf();
        var $right = $that.nextAll("td.radio");
        var $parent = $that.parent();
        var $cousin = null;

        if ($parent.children().first().attr("rowspan") != undefined) {
            $cousin = $parent.next("tr").children("td.radio").eq($left.length - 1);
        } else if ($parent.prev("tr").children().first().attr("rowspan") != undefined) {
            $cousin = $parent.prev("tr").children("td.radio").eq($left.length - 1);
        }

        if ($cousin !== null) {
            $left = $left.add($cousin).add($cousin.prevAll("td.radio"));
            $right = $right.add($cousin.nextAll("td.radio"));
        }

        var $help = $that.add($cousin).find("div.help").first();
        $help.css("display", "inline-block");

        $left.each(function(index) {
            $that = $(this);

            if ($that.hasClass("answer-head")) {
                $that.addClass("hover-head");
            } else if ($that.hasClass("answer-tail")) {
                $that.addClass("hover-tail");
            } else {
                $that.addClass("hover");
            }
        });
    }, function() {
        var $that = $(this);
        var $all = $that.siblings("td.radio").andSelf();
        var $parent = $that.parent();

        if ($parent.children().first().attr("rowspan") != undefined) {
            $all = $all.add($parent.next("tr").children("td.radio"));
        } else if ($parent.prev("tr").children().first().attr("rowspan") != undefined) {
            $all = $all.add($parent.prev("tr").children("td.radio"));
        }

        $all.find("div.help").css("display", "none");

        $all.each(function(index) {
            $(this).removeClass("hover-head").removeClass("hover-tail").removeClass("hover");
        });
    });
});

Above are my hundred lines of code. The script does exactly what I want to do, but it does look ugly, doesn't it? Could you provide a guide/tutorial on how to structure this mess up or give me some pointers?

Answer 1

once you define a group of objects with jquery, you only have to loop through them if you want each one to do something different.

the each function actually loops through each item, allowing you do do something different to each object.

for example, you put

$all.find("div.help").css("display", "none");

    $all.each(function(index) {
        $(this).removeClass("hover-head").removeClass("hover-tail").removeClass("hover");
    });

but while your looping with the each function, you are doing the exact same thing to every object, so the loop is not needed. you can use:

$all.find("div.help").css("display", "none").removeClass("hover-head").removeClass("hover-tail").removeClass("hover");

and to make it even simpler, with many jquery functions, you can seperate multiple classes and ids with a space and actually shrink all of that code to this

$all.find("div.help").css("display", "none").removeClass("hover-head hover-tail hover");

and to take it one step further, display none can be achieved with hide() - and you can display items with show()

$all.find("div.help").hide().removeClass("hover-head hover-tail hover");

EDIT: TO HELP CLARIFY TO YOUR RESPONSE

you can try this, its hard for me to test without all of your working code.

$("td.radio").click(function() {
    var $that = $(this);

    doStuff($that,'click');
    $that.nextAll("td.radio").removeClass("click-head click-tail click");   

}).hover(function() {
    var $that = $(this);

    doStuff($that, 'hover');
    var $help = $that.add($cousin).find("div.help").first();
    $help.css("display", "inline-block");


}
// rest of your code

then add this function

function doStuff(eL , which){
        var $that=$(eL);
        var $left = $that.prevAll("td.radio").andSelf();
        var $right = $that.nextAll("td.radio");
        var $parent = $that.parent();
        var $cousin = null;

        if ($parent.children().first().attr("rowspan") != undefined) {
            $cousin = $parent.next("tr").children("td.radio").eq($left.length - 1);
        } else if ($parent.prev("tr").children().first().attr("rowspan") != undefined) {
            $cousin = $parent.prev("tr").children("td.radio").eq($left.length - 1);
        }

        if ($cousin !== null) {
            $left = $left.add($cousin).add($cousin.prevAll("td.radio"));
            $right = $right.add($cousin.nextAll("td.radio"));
        }

        $left.each(function(index) {
            var $that = $(this);

            if ($that.hasClass("answer-head")) {
                $that.addClass( which + "-head");
            } else if ($that.hasClass("answer-tail")) {
                $that.addClass( which + "-tail");
            } else {
                $that.addClass( which );
            }
        });

}