Cleaning up website code

I am a bit of noob to jQuery so I got this jQuery code what I think can be way faster.

Does someone want to help me clean it up? Explaining why you did what would be real nice but not needed.

The working site is online here.

onResize = function() {
$('#spinner').delay(1000).fadeOut(500); 
// blog
setTimeout(function() {
    $('article .article_image').each(function(){
        var articleImageHeight = $(this).find('img').height();
        var articleContentHeight = $(this).parent('article').find(".article_content").height();
        var articleImageWidth = $(this).find('img').height();
        var articleContentWidth = $(this).parent('article').find(".article_content").height();

        $(this).height(articleContentHeight);    

        if (articleImageHeight  menuHeight + 15) {
        bannerPos.addClass("banner_position");
    } else {
        bannerPos.removeClass("banner_position");
    }
});  

var bannerHeight = ($(".banner_image").height() -10 +'px');
$(".banner").css({'height': bannerHeight});

// menu
var value = $('body').width() / (11);
$('#magic-line').css('top',value - 9);

var el, leftPos, newWidth,
    mainNav = $("menu.main_menu ul");
var magicLine = $("#magic-line");
magicLine
    .width($("menu.main_menu ul li.active a").width())
    .css("left", $("menu.main_menu ul li.active a").position().left)
    .data("origLeft", magicLine.position().left)
    .data("origWidth", magicLine.width());

$("menu.main_menu ul li a").hover(function() {
    el = $(this);
    leftPos = el.position().left;
    newWidth = el.parent().width();
    magicLine.stop().animate({
        left: leftPos,
        width: newWidth
    },500);
}, function() {
    magicLine.stop().animate({
        left: magicLine.data("origLeft"),
        width: magicLine.data("origWidth")
    },500);    
});

}
$(window).load('resize', onResize);
$(window).bind('resize', onResize);

And also the HTML if you don't understand my jQuery code:

```

Made in 030

Your code is indeed a Mess. But it works, so you have that going for yourself.

When writing JS it is very important to keep the global scope clean. Try and polute it as little as possible. The less you expose, the less can go wrong. Just image having a var image in 2 pieces of code, each responding to the same event, but referencing a different image. The nightmare.

An easy way to keep the dom clean is an IIFE. This also gives use a way of dependency management. If a piece of your code uses jQuery, ask for it:

(function ($) {
    //now we can use the $ safely
})(jQuery);

Now we are sure that $ is mapped to jQuery. You also have a (very) small speed advantage because the compiler doesn't have to look up parent scopes for the jQuery object (Some people tend to really like micro-optimisations).

Now that we have succesfully made the dome a cleaner place. We can start writing. A good rule of thumb is to always define all your variables at the top of your script. this way it is very easy to see what you are using. All var statements are eventually moved to the top of a script by the compiler (useless micro-optimisation n2).

(function ($, window) {
    var $articleImages = $('article .article_image');

    $(window).on('resize', function() {
        $articleImages.each(function() {
            var $image = $(this).find('image'),
                $content = $(this).parent('article').find('.article_content');

            $(this).height($content.height());

            if ($image.height() <= $content.height()) {
                $image.addClass('smallerheight');
                $image.removeClass('biggerheight');
            } else {
                $image.addClass('biggerheight');
                $image.removeClass('smallerheight');
            }
        });
    });
})(jQuery, window);

Note that I removed the setTimeOut in your script. I'm not sure why it is there. It seems as if you are not reacting to the resize event but to the spinner is fadedOut event. And why is the spinner fadingOut? Is it waiting for something? It just seems odd to have all these delays. Why make the user wait? To look at some fancy spinner?

Other changes I made is DOM lookups. Everytime you lookup a piece of dom multiple times, place it in a variable. Note that all my jquery object variables start with a $. Makes it easier to see what is what.

I also removed the DOM lookup of all the articles outside of the setTimeOut function. Now we lookup the articles in the DOM only once.

I also encapsulated the logic of the articles in its own little closure. Why you ask? a spinner has nothing to do with articles. And they have nothing to do with a banner, the menu, ... So have them seperated. A good thing woul be to clean up the rest by yourself as a good pracice.

StackExchange Code Review Q#72115, answer score: 4