...

View Full Version : help me optimize my code



GaVrA
09-14-2009, 04:05 PM
I have 4 html files which i am loading into my page.

This is the page im talking about:

http://www.crtaci.info/index.php?autocom=majice

First i had this code:


$(function() {
$.ajaxSetup ({
cache: false
});

var ajax_load = "<img style='width:42px;height:42px;display:block;margin:auto;' src='style_images/freshair/load.gif' alt='loading...' />";

$("#majice").html(ajax_load).load("majice20.html", function(){
$(".tShirt").removeAttr('title');
});
$("#show").click(function(){
$("#shown").slideToggle(1000);
});
$("#1").click(function(){
$("#majice").html(ajax_load).load("majice20.html", function(){
$(".tShirt").removeAttr('title');
});
$("ol.majice_navigacija>li").removeClass("selected");
$(this).addClass("selected");
});
$("#2").click(function(){
$("#majice").html(ajax_load).load("majice40.html", function(){
$(".tShirt").removeAttr('title');
});
$("ol.majice_navigacija>li").removeClass("selected");
$(this).addClass("selected");
});
$("#3").click(function(){
$("#majice").html(ajax_load).load("majice60.html", function(){
$(".tShirt").removeAttr('title');
});
$("ol.majice_navigacija>li").removeClass("selected");
$(this).addClass("selected");
});
});

I managed to get to this:


$(function() {
$.ajaxSetup ({
cache: false
});

var ajax_load = "<img style='width:42px;height:42px;display:block;margin:auto;' src='style_images/freshair/load.gif' alt='loading...' />";

var majice_stranica = 20;

function majice(majice_stranica){
$("#majice").html(ajax_load).load("majice"+majice_stranica+".html");
};

$("ol.majice_navigacija>li").click(function(){
majice_stranica = $(this).attr("id");
majice(majice_stranica);
if($(this).attr("class") != "selected"){
$("ol.majice_navigacija>li").removeClass("selected");
$(this).addClass("selected");
};
});

majice(majice_stranica);

$("#show").click(function(){
$("#shown").slideToggle(1000);
});
});

Navigation for this is this html:


<ol class="majice_navigacija">
<li id="20" class="selected"><a href="#1" rel="history">1</a></li>
<li id="40"><a href="#2" rel="history">2</a></li>
<li id="60"><a href="#3" rel="history">3</a></li>
<li id="80"><a href="#4" rel="history">4</a></li>
</ol>

Is there anything else i can do to make this better code? I ask becasue im still pretty noobish when it comes optimization... :)

Spudhead
09-16-2009, 11:00 AM
Nothing obviously needs redoing - only two thing I'd possibly change:

1. Instead of calling $(this) every time you need to access the <li> element, drop a reference to it into a variable:
var $this = $(this);

2. Instead of checking that the <li> "class" attribute doesn't equal the string "selected", use jQuery's hasClass (http://docs.jquery.com/Traversing/hasClass) method:
if (!$this.hasClass("selected")){
// stuff
}

GaVrA
09-16-2009, 01:56 PM
Thnx! :)

I tried both, #2 works, i have no idea how i forgot about that "hasClass"... :D

But #1, if i put it like that and change every $(this) into $this page wont load. White screen.

If i put this where $(this) was, page loads but script aint working...



EZ Archive Ads Plugin for vBulletin Copyright 2006 Computer Help Forum