2016-02-16 3 views
-6

Я редактировал некоторый jQuery, и кажется, что просто добавляет к IF инструкции IF. То, что делает функция или что делают операторы if, на самом деле не так важно. Я просто интересно, если есть лучший/уборщик способ написать следующее:Чище/лучший способ написать функцию jQuery

scrollToTop: function() { 
    var offset = 160; 
    var duration = 500; 
    jQuery(window).scroll(function() { 
     if (jQuery(this).scrollTop() > offset) { 
     jQuery('#top-link-block').fadeIn(duration); 
     } else { 
     jQuery('#top-link-block').fadeOut(duration); 
     } 
     if (jQuery('#top-link-block').offset().top + jQuery('#top-link-block').height() >= jQuery('#footer-wrapper').offset().top - 10) 
     jQuery('#top-link-block').css('position', 'absolute'); 
     if (jQuery(document).scrollTop() + window.innerHeight < jQuery('#footer-wrapper').offset().top) 
     jQuery('#top-link-block').css('position', 'fixed'); 
     if (jQuery('#fixed-toolbar-menu')[0]) { 
     jQuery('#top-link-block').css({ 
      bottom: 150 
     }); 
     } 
    }); 
+1

Что она делает «IS» очень важно, это помогает решить «лучший способ» закодировать данную функцию. –

+2

Также этот вопрос лучше подходит для http://codereview.stackexchange.com/ –

+2

Какая проблема (если есть) у вас с этим кодом? просто трудно читать? или он не работает так, как ожидалось, или является неустойчивым/неустойчивым. Он будет работать немного странно, если вы будете прокручивать быстро между 150 и 170 раз. –

ответ

0

Ну, один постоянно запрашивая один и те же объекты DOM является не только повторами, но дорого. Вы всегда должны записывать результаты запроса с помощью переменной. Делая это в одиночку, будет очистить код:

var $tlb = jQuery('#top-link-block'); 

ПРИМЕЧАНИЯ: Я предлагаю предварив переменный результат JQuery с $, чтобы напомнить вам, что это JQuery объект обертки, а не традиционный объект DOM.

Теперь, вы можете просто использовать эту переменную снова и снова:

if ($tlb.offset...)... 

Кроме того, вы используете это значение:

jQuery('#footer-wrapper').offset().top 

более чем один раз, так что я бы сохранить это значение в переменной также:

var $footWrapOffsetTop = jQuery('#footer-wrapper').offset().top; 

И тогда вы можете использовать это там, где это необходимо, без запроса объекта d разархивируйте значение offset.top.

Кроме того, это:

jQuery(window) 

расточительно, потому что, так как вы не захватывая результат запроса и только с помощью запроса, так что вы можете прикрепить обработчик события к окну. Использование JQuery для получения ссылки на глобальный объект окна делает не что иное, как выполнение поиска по объекту, который всегда доступен. Просто используйте:

window.addEventListener("scroll", function(){...}); 

Единственную пользу для выполнения запросов окна будет воспользоваться завернутым посаженными функциями JQuery, но вы не делаете это здесь.

Вот очищены версия (с использованием передовых методов):

scrollToTop: function() { 

     var offset = 160; 
     var duration = 500; 

     // Result is JQuery wrapped-set object: 
     var $tlb = jQuery('#top-link-block'); 

     // Result is top value: 
     var footWrapOffsetTop = $('#footer-wrapper').offset().top; 

     window.addEventListener('scroll', (function() { 

     // Don't take advantage of optional curly braces with blocks 
     // of only one statement. It can lead to bugs. 
     if (window.scrollTop() > offset){ 
      $tlb.fadeIn(duration) : $tlb.fadeOut(duration); 
     } 

     if ($tlb.offset().top + $tlb.height() >= footWrapOffsetTop - 10) { 
      $tlb.css('position', 'absolute'); 
     } 

     if ($(document).scrollTop() + window.innerHeight < footWrapOffsetTop){ 
      $tlb.css('position', 'fixed'); 
     } 

     if ($('#fixed-toolbar-menu')[0]){ 
      $tlb.css({ bottom: 150 }); 
     } 
     }); 
+0

Почему голос? Это напрямую решает вопрос и повышает производительность. –

Смежные вопросы