In terms of jQuery code optimization / quality, could someone suggest the basic rules?

For example, here is my first script:

$(function(){ $block = $('#menu'); $blockPosition = $block.offset().top; $(window).scroll(function(){ $windowPosition = $window.scrollTop(); if ($windowPosition > $blockPosition) { $block.addClass('fix'); }else{ $block.removeClass('fix'); } }); }); 

What comments can be on this code, thanks.

  • four
    add var in front of $block , $blockPosition and $windowPosition to make them local variables - Igor
  • This, in general, is normal to use $ in the names of all variables, but it is usually expected that such a variable will contain some special object (jquery, zepto ...), $blockPosition and $windowPosition contain the usual int . - Arnial
  • 2
    @VitaliyEmeliantsev - "Variables beginning with the $ sign - global" - can I link to the source? - Igor
  • 3
    @Igor, you are right - my statement about the globality of variables with $ in the beginning is fundamentally wrong. $ does not affect the globality of a variable in any way, nor does it affect anything at all. I apologize for misleading my comment. - Vitaly Emelyantsev
  • 2
    As far as I know, it is customary to mark variables with jQuery objects with a dollar at the beginning, so that it is more convenient to understand the code - andreymal

3 answers 3

 $block = $('#menu'); 

Variables must be declared via var so that they do not become global:

  var $block = $('#menu'); 
 $blockPosition = $block.offset().top; 

When writing jQuery code, the names of variables containing jQuery sets are usually started with a dollar sign, so just

 var blockPosition = $block.offset().top; 
 if ($windowPosition > $blockPosition) { $block.addClass('fix'); }else{ $block.removeClass('fix'); } 

There is a wonderful second parameter to the toggleClass function, only it must be boolean:

 $block.toggleClass('fix', $windowPosition > $blockPosition); 
 $windowPosition 

Variables used once can often not be set:

 $block.toggleClass('fix', $window.scrollTop() > $blockPosition); 

$ window

This variable has never been declared anywhere ...

Tell me please, if $ (window) is used in the code more than once, does it need to be cached in a variable and be the jQuery set window?

You should do the same with all other variables.

In general, my opinion is that if something is requested once during the processing of an event , then there is no need to cache it. This allows you to get the actual elements, avoid cluttering the code with unnecessary variables and potentially holding unused dom elements in memory. In addition, some events may not occur, and with the initial caching we do a lot of unnecessary searches.

But there is another opinion - to immediately cache everything you need.

 }else{ 

It is customary to put spaces around brackets.

 } else { 

Summary Code:

 $(function () { var $block = $('#menu'); var blockPosition = $block.offset().top; $(window).scroll(function () { $block.toggleClass('fix', $(window).scrollTop() > blockPosition); }); }); 

Although, to be honest, I have doubts about getting the position of the block immediately after loading the document. Should it be exactly there?

  • Tell me please, if $ (window) is used in the code more than once, does it need to be cached in a variable and be the jQuery set window? - Rodion Polyakov
  • @RodionPolyakov, the answer is supplemented. - Qwertiy
  • $(window) is still worth caching. The reason is simple: the scroll event can be triggered with great frequency. Arguments against saw, in this case, they are not significant. - Dmitriy Simushev
  • @DmitriySimushev, I generally agree about the scroll, but var t=performance.now(); for(var q=0; q<1000000; ++q) $(window); console.log(performance.now()-t); var t=performance.now(); for(var q=0; q<1000000; ++q) $(window); console.log(performance.now()-t); performed in my chrome in less than 9 seconds - a million wraps in 9 seconds - that is, you can perform more than 1000 such operations for a keyframe (16 ms). - Qwertiy
  • one
    @RodionPolyakov, but I did not talk about jQuery 3. It was not mentioned anywhere at all. UPDATE: a, nonal. This is an option without a class name. - Qwertiy

In terms of optimization, I think it is a place to call variables by their proper names and not block . Do not produce one-time variables like $windowPosition . Do not use $ in variables, to be honest, I do not know where they go, but this is clearly not good.

 $(function(){ var menu = $('#menu'), menuOffset = menu.offset().top; $(window).scroll(function(){ if ($window.scrollTop() > menuOffset) { menu.addClass('fix'); } else { menu.removeClass('fix'); } }).trigger('scroll'); }); 

Also corrected the error that he caught himself - if you scroll and stop the page before the jquery is initialized, the menu will not stick, so you need to kick it just in case.

    The main rule is to maintain the same programming style "code conventions"
    Variables called by their action.
    Jquery constructs can be extended to a string. For example:

     $('div').append(table.filter('[data-clone]').clone().removeAttr('data-clone').show()); 

    And so on. It will come with experience ...
    The main thing is to move in the right direction.

     $(function(){ $block = $('#menu'); $(window).scroll(function(){ console.log($(this).scrollTop()); ($(this).scrollTop() > $block.offset().top) ? $block.addClass('fix') : $block.removeClass('fix'); }); }); 
     <script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script> 

    Can add. If you do this:

     $block = $('#menu'); $block.remove(); console.log($block); 

    // 1 object from memory. In the DOM it is not. Ie The variable value is not valid.

    • 1. Does $block.offset().top not change? 2. toggleClass . - Qwertiy
    • one
      You broke the logic, here the script gets the menu offset initially. Yes, and a shorthand with so many characters turns into incomprehensible noodles, especially for beginners, it’s not at all clear visually. If you want to shorten it so you can use the collectors. - Artem Gorlachev
    • Quickly threw. Not tested. - Andrew
    • And $ block.offset (). Top does not change? No, it doesn't change! - Andrew