I have following code for a content scroller. This has been implemented with basic jquery. Iam trying to convert the same using advanced javascript concepts. Guide me to implement the same.
Working Code :
$(document).ready(function() {
var currentPosition = 0,
slideWidth = 112,
slides = $('#scroller ul li'),
numberOfSlides = slides.length;
scrollerContainer = $('#scroller ul');
$(scrollerContainer).css('width', slideWidth * numberOfSlides);
manageControls(currentPosition);
$('.control').click(function(){
currentPosition = ($(this).attr('id')=='rightControl') ? currentPosition+1 : currentPosition-1;
manageControls(currentPosition);
$(scrollerContainer).animate({
'marginLeft' : slideWidth*(-(currentPosition*3))
});
});
function manageControls(position){
if(position==0){ $('#leftControl').hide() }
else{ $('#leftControl').show() }
if((position*3)==numberOfSlides-1 || (position*3)==numberOfSlides-3 || (position*3)==numberOfSlides-2) { $('#rightControl').hide() }
else{ $('#rightControl').show() }
}
});
Advanced Code Structure : Initial understanding
if(!scroller) { var scroller = {}; }
scroller =
{
videos : {
moveThumbnails : function() {
},
gotonextThumbnails : function() {
var obj = this;
},
gotopreviousThumbnails : function() {
var obj = this;
},
init : function() {
var obj = this;
}
},
init : function() {
var obj = this;
this.videos.init();
}
}
Working example : http://jsfiddle.net/ylokesh/9EyEu/
-
2\$\begingroup\$ I don't intend to troll, but do you have a definition of what's advanced and what's not in JavaScript? \$\endgroup\$Tharabas– Tharabas2012年05月20日 18:34:33 +00:00Commented May 20, 2012 at 18:34
-
\$\begingroup\$ @Tharabas at a beginner level, we used to write javascript where we marely use Object communication. Advanced javascript concpets like modular javascript so that later on same code can be converted to plugin easily. Just trying yo get my hands dirty with how object communicates and how we can achieve optimization. \$\endgroup\$Lokesh Yadav– Lokesh Yadav2012年05月21日 17:08:19 +00:00Commented May 21, 2012 at 17:08
1 Answer 1
I can't post a full answer about the "advanced JavaScript" part of your question, but here's what I would change in the actual code:
Use
jQuery.fn.find
methodYou could optimize this
$('#scroller ul li')
with this$('#scroller').find('ul li')
. When using a selector like#someidname
, jQuery can use directly the native methoddocument.getElementById
. When using#someidname something
, then jQuery has to parse the selector, and only after this it can say "oh there's an id string in there !".Don't recreate jQuery objects
When doing
var scrollerContainer = $('#scroller ul');
,scrollerContainer
becomes a jQuery object. Then, you don't need to do$(scrollerContainer).someMethodName()
but you could directly writescrollerContainer.someMethodName()
.Always use triple equals sign
You should (almost) always prefer
===
over==
. The double equal sign may have some unexpected behavior such as"\t \n" == 0
which evaluate totrue
.Don't use jQuery when it's not needed
$(this).attr('id')
could be simplified inthis.id
. This way, you don't construct a new jQuery object (this can be costly when done in loops for example), and theid
property is supported in a cross-browser way so you don't have to worry about it.Cache your DOM selections
$('#leftControl')
and$('#rightControl')
should be cached in variables in the same way as$('#scroller ul')
is. We should avoid doing the same DOM query wherever it's possible.Prefer
jQuery.fn.on
orjQuery.fn.bind
methods over traditional onesBecause:
- they are faster: internally
$('whatever').click(fn)
calls$('whatever').on('click', fn)
, so you can use it directly and avoid a function call, - they are more flexible: you can bind the same function to different events (
$('whatever').on('click mouseenter mouseleave', fn)
), you can namespace your events ($('whatever').on('click.myPlugin', fn)
) which IMO would be useful in your case (your doing some sort of namespacing)
- they are faster: internally
Simplify (Math) expressions
IMO, the code
slideWidth*(-(currentPosition*3))
is not really readable. Why using such parenthesis ? And what about this inner-
sign ?What about writing it this way
-slideWidth*currentPosition*3
?Also, the part
(position*3)==numberOfSlides-1 || (position*3)==numberOfSlides-3 || (position*3)==numberOfSlides-2
can be simplified. Have a look (shorter variable names for the example):(p*3)==n-1 || (p*3)==n-3 || (p*3)==n-2 // which is equivalent to 1==n-p*3 || 3==n-p*3 || 2==n-p*3 // which is equivalent to 1 <= n-p*3 && n-p*3 <= 3 // and also equivalent to (n-p*3-1) in [0,0,0]
so you end up with this:
if( (numberOfSlides-position*3-1) in [0,0,0] ) { ... }
That
(n-p*3-1) in [0,0,0]
part is quite interesting. When doinga in b
you'll search if the arrayb
has the indexa
. This only happens in our case becauseb
is an array. So we use a arrayb = [0,0,0]
which has 3 elements. 3 comes from the expressionn-p*3
, this is the maximum X of the threen-p*X
forms seen above. Then, since[0,0,0]
has 3 elements, its index are 0, 1, and 2. So the test if the expressionn-p*3
equals to one of 1, 2, or 3, we have to subtract-1
.To explain it on another way, we can say that
0 in [0,0,0]
,1 in [0,0,0]
, and2 in [0,0,0]
will both be true (because we're using the array indexes and not its values), and all others numerical values will evaluate the expression tofalse
. Since what you want you is to check ifn-p*3
is of one 1, 2 or 3, if we remove 1 to this expression, we retrieve our values 0, 1, and 2 from above which will evaluate the expression totrue
.If you think this one is too tricky (and to be honest, it is !), you can do the following, which avoid code repetition and where the intent is explicit and readable (I would recommend this technique over the
index i [0,0,0]
one which is implicit and less readable IMO)var temp = numberOfSlides - position * 3; if( 1 <= temp && temp <= 3 ) { ... }
Hope that helps :)
-
\$\begingroup\$ Nice in on array of index trick. \$\endgroup\$Larry Battle– Larry Battle2012年05月20日 23:26:52 +00:00Commented May 20, 2012 at 23:26
-
2\$\begingroup\$ The
in [0,0,0]
"trick" is terribly unreadable. I wouldn't recommend it. \$\endgroup\$RoToRa– RoToRa2012年05月21日 14:22:45 +00:00Commented May 21, 2012 at 14:22 -
\$\begingroup\$ I'd support @RoToRa here. The trick will trick yourself after not having read that line of code for a while. Better extract the value and compare it in a more readable fashion like
var v = numberOfSlides - position * 3 - 1; if (v >= 0 && v <= 2) ...
as @pomeh recommended the line before the trick. \$\endgroup\$Tharabas– Tharabas2012年05月21日 19:46:58 +00:00Commented May 21, 2012 at 19:46 -
\$\begingroup\$ @RoToRa you're right. This is the kind of "high level" optimization, or the "art" to use JavaScript's leakiness to achieve a unexpected behavior. While I'm agree with you that this not readable and unclear (I've updated the answer to point it out), this could be the kind of trick used in "high level" optimization or "high level" of code compression, but that may represent a very low percentage or real-world applications ;) Anyway, I think this kind of JavaScript trick is funny, and that's why I've included it, but I would say don't use it unless you really know what're doing :) \$\endgroup\$pomeh– pomeh2012年05月21日 19:54:09 +00:00Commented May 21, 2012 at 19:54
-
\$\begingroup\$ Maybe add use function binding instead of
var that = this
, etc? +1 for using.on()
instead of.click()
. \$\endgroup\$Cobby– Cobby2012年05月22日 06:28:31 +00:00Commented May 22, 2012 at 6:28