Adding classes with JavaScript

Question

I am working again and got this to work. Is this the best way to write this code?

$(window).load(function(){
var $window = $(window),
$sticky = $('#contentSideIner'),
elTop = $sticky.offset().top;
$window.scroll(function() {
$sticky.toggleClass('sticky', $window.scrollTop() > elTop);
});
$window.scroll(function() {
$sticky.toggleClass('stickyBottom', $window.scrollTop() > elTop + 4685);
});
});

I had such a hard time to get this function to work in the first place. For the longest time I was told it was not executing at all.

• 200_success: Welcome to Code Review! Could you tell us what you really want to accomplish with this code — what scrolling behaviour should the user see as a result? Could you post the relevant CSS too? And maybe your HTML as well, if it might help us understand what you are trying to accomplish?

Answer 1

This can be simplified to this:

$(window).scroll(function() {
 var $sticky = $('#contentSideIner'),
 elTop = $sticky.offset().top, 
 wTop = $(window).scrollTop();
 $sticky.toggleClass('sticky', wTop > elTop)
 .toggleClass('stickyBottom', wTop > elTop + 4685);
});

1. The window object already exists, so no need to wait until it loads before attaching an event handler to it.
2. Don't cache objects outside of the event handler unless it's really needed (consumes more memory, code is less encapsulated, makes your page susceptible to problems if layout or DOM elements change, etc...)
3. Use jQuery chaining for multiple method calls on the same jQuery object
4. Only use one event handler and put both .toggleClass() calls in the same event handler.