Комментарии (8)

1. 

   Riim

   27.10.2021 19:20

   #23637330

   +2

   (...args) => {
   ...
   lastThis = this;

   this будет от внешней функции, у стрелочной нет своего, а здесь как раз нужно запоминать this производной функции. Тоже самое для debounce.
   
   Для throttle clearTimeout внутри setTimeout не имеет никакого смысла.
   
   Для throttle сразу создаётся таймаут, хотя второго вызова который необходимо отложить ещё нет, то есть если вызовы будут происходить с задержкой большей чем wait, то таймауты будут молотить впустую.
   
   Ещё возможная ситуация для throttle: происходит второй вызов который откладывается, дальше поток выполнения забивается сложными вычислениями, настаёт время выполнить таймаут, но так как поток занят таймаут просто стоит в очереди, и вот тут прямо в расчётах происходит третий вызов. Ваша реализация просто заменит контекст и аргументы, но вызова исходной функции так и не произойдёт, таймаут делающий его так и будет стоять в очереди, хотя прошедшее время позволяет прямо посреди вычислений вызвать исходную функцию отменив стоящий в очереди таймаут.
   
   Для debounce то же есть хорошее улучшение. Допустим происходит 100 вызовов производной функции и задержка между ними всегда меньше wait, то есть вызов исходной функции произойдёт только 1 раз. В вашей реализации будет создано 100 таймаутов из которых 99 будут отменены и 1 сработает. Можно улучшить реализацию следующим образом: при первом вызове создаётся таймаут, при втором он не отменяется, а вместо этого просто обновляется время последнего вызова производной функции, таких вызовов до срабатывания таймаута может произойти несколько, дольше вызывается таймаут и в нём видя, что время последнего вызова обновилось не вызывается исходная функция, а создаётся следующий таймаут на время `wait - (Date.now() - timestamp)`. Таким образом количество создаваемых и отменяемых таймаутов сильно падает, иногда в десятки раз при том же конечном результате.
   
   Грамотные реализации этих двух функций не так уж тривиальны как кажется на первый взгляд. Несколько лет назад используя одну из популярных реализаций throttle с npm столкнулся со странным поведением, полез разбираться в исходники, баг нашёл, а заодно ещё один. Полез смотреть исходники других вариантов, просмотрел всю первую страницу выдачи npm и почти во всех пакетах удалось найти минимум один полноценный баг. В тот момент стало очень грустно от качества кода на npm. Запилил свои реализации: @riim/debounce-throttle , на идеальность не претендую, но делал очень вдумчиво и думаю качество всё же повыше среднего на npm.

   
   

   1. 

      yantishko Автор

      28.10.2021 09:05

      #23638472

      +1

      Спасибо за ревью и рекомендации по улучшению! В след статье с фоллоу апом попробую реализовать эти улучшения и исправления

   
   

   1. 

      yantishko Автор

      05.11.2021 11:40

      #23670146

      попробовал переделать debounce, вроде по логике все верно, но тесты не проходит. I need help :)

      function debounce(func, wait) {
        let timeoutId = null;
        let lastCall = null;
      
        return function (...args) {
          const fnCall = () => {
            if (Date.now() - lastCall < wait) {
              timeoutId = setTimeout(fnCall, wait - (Date.now() - lastCall));
            } else {
              func.apply(this, args);
              lastCall = Date.now();
              timeoutId = null;
            }
          }
          if (!timeoutId) {
            timeoutId = setTimeout(fnCall, wait);
          }
          lastCall = Date.now();
        }
      }

      
      

      1. 

         Riim

         05.11.2021 12:17

         #23670340

         fnCall создаётся внутри производной функции, во-первых, создавать его (fnCall) на каждый её (производной функции) вызов нет необходимости, во-вторых, так в таймаут попадает первая созданная функция, которая запомнила первые this и args, а нужны последние.
         По ссылке в моём комменте выше есть рабочий вариант. Или вы пробуете самостоятельно сделать?

         
         

         1. 

            yantishko Автор

            05.11.2021 12:19

            #23670344

            я пробую самостоятельно, в рамках решения задачи на интервью за пару минут, я как раз по вашему решению и разбирался с этим подходом

1. 

   OkunevPY

   27.10.2021 20:01

   #23637448

   А чего вы ждёте от npm где публикуциются такие же хабра-писатели, где-то подсмотренный, недопонятый код, не внятно описанный что-же он делает и где тут разница, да ещё и с ляпами, за баги или ошибки я бы это не засчитал, баги и ошибки бывают в рабочем коде, а это просто наляпано .

1. 

   aio350

   30.10.2021 01:41

   #23647142

   -1

   // debounce
   // with `this`
   const debounce = (fn, ms) =>
     function (...args) {
       let prevCall = this.lastCall
       this.lastCall = Date.now()
   
       if (prevCall && this.lastCall - prevCall <= ms) {
         clearTimeout(this.timer)
       }
   
       this.timer = setTimeout(() => fn(...args), ms)
     }
   
   // or
   // without `this`
   const _debounce = (fn, ms) => {
     let id = null
   
     return (...args) => {
       clearTimeout(id)
   
       id = setTimeout(() => {
         fn(...args)
   
         clearTimeout(id)
       }, ms)
     }
   }
   
   // throttle
   // with `this`
   const throttle = (fn, ms) =>
     function (...args) {
       let prevCall = this.lastCall
       this.lastCall = Date.now()
   
       if (!prevCall || this.lastCall - prevCall > ms) {
         fn(...args)
       }
     }
   
   // or
   // without `this`
   const _throttle = (fn, ms) => {
     let id = null
   
     return (...args) => {
       if (id) return
   
       fn(...args)
   
       id = setTimeout(() => {
         id = null
   
         clearTimeout(id)
       }, ms)
     }
   }

   
   

   1. 

      aio350

      30.10.2021 10:56

      #23647742

      fix

      id = setTimeout(() => {
        clearTimeout(id)
        
        id = null
      }, ms)