1
\$\begingroup\$

I often find myself writing code as follows...

var startX, startY, dragging = false, element = document.getElementById("whatever");

var start = function(e){
  dragging = true;
  startX = e.pageX;
  startY = e.pageY;
  element.style.opacity = 0.5;
}

var move = function(e){
  if (!dragging) return;
  var deltaX = startX - e.pageX;
  var deltaY = startY - e.pageY;
  //apply to element
}

var end = function(e){
  dragging = false;
  element.style.opacity = 1;
}

element.addEventListener("mousedown", start, false);
document.addEventListener("mousemove", move, false);
document.addEventListener("mouseup", end, false);

Now, my problem is really basic, it feels to me that the functions should be auxiliary and belong to the bottom part of my program, but I don't know how to attach the functions to the event listeners without declaring them first. How do I restructure my code so that it is more evident what I'm going to do with these functions?

\$\endgroup\$
2
\$\begingroup\$

You can use hoisting. Declare the functions wherever you want, in this case, later, but declare them this way:

function start(e){

}

When declared this way, they are hoisted (read about it here), and can be used before the declaration.

Your code, restructured, would look like this:

var startX, startY, dragging = false, element = document.getElementById("whatever");



element.addEventListener("mousedown", start, false);
document.addEventListener("mousemove", move, false);
document.addEventListener("mouseup", end, false);

function start(e){
  dragging = true;
  startX = e.pageX;
  startY = e.pageY;
  element.style.opacity = 0.5;
}

function move(e){
  if (!dragging) return;
  var deltaX = startX - e.pageX;
  var deltaY = startY - e.pageY;
  //apply to element
}

function end(e){
  dragging = false;
  element.style.opacity = 1;
}
\$\endgroup\$
  • \$\begingroup\$ I've been using javascript for so long and I had no idea about hoisting, thanks so much! \$\endgroup\$ – methodofaction Oct 14 '12 at 15:40
  • \$\begingroup\$ @Duopixel Sure. Not many people do. \$\endgroup\$ – Some Guy Oct 14 '12 at 15:59
2
\$\begingroup\$

You could simply declare the functions inline, like:

element.addEventListener("mousedown", function (e){
  dragging = true;
  startX = e.pageX;
  startY = e.pageY;
  element.style.opacity = 0.5;
}, false);

And likewise for the other 2 functions.

But that can get messy in a hurry. Especially because startX, startY and dragging would still be outside.

So instead you could put the handler in an object literal, just to keep them separated:

var dragEvents, element = document.getElementById("whatever"),

dragEvents = (function () {
  var dragging = false, startX, startY;
  return {
    start: function (e){
      dragging = true;
      startX = e.pageX;
      startY = e.pageY;
      element.style.opacity = 0.5;
    },

    move: function (e){
      if (!dragging) return;
      var deltaX = startX - e.pageX;
      var deltaY = startY - e.pageY;
      //apply to element
    },

    end: function(e){
      dragging = false;
      element.style.opacity = 1;
    }
  };
}());

element.addEventListener("mousedown", dragEvents.start, false);
document.addEventListener("mousemove", dragEvents.move, false);
document.addEventListener("mouseup", dragEvents.end, false);

I've wrapped it in an immediately invoked function so the dragging-related vars can be kept in there too.

However, might as well go the extra step and put everything in a function, instead of relying on element being a closure:

function makeDraggable(elementId) {
  var startX,
      startY,
      dragging = false,
      element = document.getElementById(elementId);

  function start(e){
    dragging = true;
    startX = e.pageX;
    startY = e.pageY;
    element.style.opacity = 0.5;
  }

  function move(e){
    if (!dragging) return;
    var deltaX = startX - e.pageX;
    var deltaY = startY - e.pageY;
    //apply to element
  }

  function end(e){
    dragging = false;
    element.style.opacity = 1;
  }
  element.addEventListener("mousedown", start, false);
  document.addEventListener("mousemove", move, false);
  document.addEventListener("mouseup", end, false);
}

The last suggestion I'd make would be to skip the dragging variable in favor of simply adding the move and end handlers on mouse down, and removing them on mouse up, like so:

function makeDraggable(elementId) {
  var startX,
      startY,
      element = document.getElementById(elementId);

  function move(e){
    var deltaX = startX - e.pageX;
    var deltaY = startY - e.pageY;
    //apply to element
  }

  function end(e){
    element.style.opacity = 1;
    element.removeEventListener("mousedown", move, false);
    element.removeEventListener("mousedown", end, false);
  }

  element.addEventListener("mousedown", function (e){
    startX = e.pageX;
    startY = e.pageY;
    element.style.opacity = 0.5;
    document.addEventListener("mousemove", move, false);
    document.addEventListener("mouseup", end, false);
  }, false);
}
\$\endgroup\$
  • \$\begingroup\$ Why is dragging a bad pattern? I've read that keeping state is bad in programming, but I'm not sure if it's related to this. \$\endgroup\$ – methodofaction Oct 14 '12 at 15:42
  • \$\begingroup\$ @Duopixel Personally, I just find it more straightforward to only have the event handlers there, when they're needed, instead of having them fire for every mousemove and mouseup. No need to explicitly keep state \$\endgroup\$ – Flambino Oct 14 '12 at 16:20

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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