3
\$\begingroup\$

I feel like the main point of inefficiency is how I create "notches" and must loop through all of the created notches every time the value for the slider is changed.

function sliderLoop() {
    for (var i = slideNotchArray.length - 1; i >= 0; i--) {
      if ($(".slider").val() >= slideNotchArray[i].value) {
        console.log(slideNotchArray[i].slide);
        teamSlider.goTo(slideNotchArray[i].slide);
        break;
      }
    }
 }

It's an exponential nightmare as each call can loop through every notch object, and there can be multiple calls a second. This is the only solution I could have thought of though, and I am more than welcome to someone else's suggestions. I attempted to mitigate the severity by including a break once the first notch has been found, as to not loop through the rest, but I feel as though it is still quite heavy.

https://codesandbox.io/s/currying-bush-h3xyz

My major concerns are in relation to

  function sliderLoop() {
    for (var i = slideNotchArray.length - 1; i >= 0; i--) {
      if ($(".slider").val() >= slideNotchArray[i].value) {
        console.log(slideNotchArray[i].slide);
        teamSlider.goTo(slideNotchArray[i].slide);
        break;
      }
    }
  }

  // Fires slider loop every time the value changes, I had to use mousemove as .change() would only fire once the mouse click was relseased.
  $(".slider").on("change mousemove", function() {
    sliderLoop();
  });

The "change mousemove" event listener, calls sliderLoop multiple times a second. sliderLoop() contains a for loop, which could potentially iterate through many (10+) array index's. Calling a for loop which iterates through so many index's multiple times a second makes me believe this is an inefficient method of going about a slider navigation, however, I am at a loss as to how to improve the code while maintaining the functionality. Would you believe this to be an issue worth worrying about? If so, how could I go about improving this?

\$\endgroup\$
1
\$\begingroup\$

A technique that would likely improve this situation is a debounced function. That way the function to be run will only run at certain intervals instead of each time the events occur.

From the Blog of JavaScript consultant David Walsh:

For those of you who don't know what a debounce function does, it limits the rate at which a function can fire. A quick example: you have a resize listener on the window which does some element dimension calculations and (possibly) repositions a few elements. That isn't a heavy task in itself but being repeatedly fired after numerous resizes will really slow your site down. Why not limit the rate at which the function can fire?

Here's the basic JavaScript debounce function (as taken from Underscore.js):

// Returns a function, that, as long as it continues to be invoked, will not
// be triggered. The function will be called after it stops being called for
// N milliseconds. If `immediate` is passed, trigger the function on the
// leading edge, instead of the trailing.
function debounce(func, wait, immediate) {
    var timeout;
    return function() {
        var context = this, args = arguments;
        var later = function() {
            timeout = null;
            if (!immediate) func.apply(context, args);
        };
        var callNow = immediate && !timeout;
        clearTimeout(timeout);
        timeout = setTimeout(later, wait);
        if (callNow) func.apply(context, args);
    };
};

1

That can be used to create a debounced function that only gets called at an interval - e.g. 250 milliseconds (feel free to adjust to your needs):

var debouncedFunc = debounce(sliderLoop, 250);

And use that function in the callback of the event handler:

// Fires slider loop every time the value changes, I had to use mousemove as .change() would only fire once the mouse click was relseased.
$(".slider").on("change mousemove", function() {
  debouncedFunc();
});

That could also be simplified to the following, since the extra lambda function/closure is pointless:

// Fires slider loop every time the value changes, I had to use mousemove as .change() would only fire once the mouse click was relseased.
$(".slider").on("change mousemove", debouncedFunc);

1https://davidwalsh.name/javascript-debounce-function

| improve this answer | |
\$\endgroup\$

Not the answer you're looking for? Browse other questions tagged or ask your own question.