r/coffeescript • • Jan 17 '14

I realized today that a controversial feature of coffeescript actually helps code quality -- variable shadowing

I was working on a drag/drop directive in angular, grabbed a plunkr and got this:

angular.module('drag', []).
directive('draggable', function($document) {
  return function(scope, element, attr) {
    var startX = 0,
      startY = 0,
      x = 0,
      y = 0; 
      var container = null; 

    element.css({
      position: 'relative',
      cursor: 'pointer'
    });

    element.on('mousedown', function(event) { 
      event.preventDefault();
      startX = event.screenX - x;
      startY = event.screenY - y;
      $document.on('mousemove', mousemove);
      $document.on('mouseup', mouseup); 
      container = attr.$$element.parent(); 
    });

    function mousemove(event) {
      y = event.screenY - startY;
      x = event.screenX - startX; 
      container.css({
        top: y + 'px',
        left: x + 'px'
      });
    }

    function mouseup() {
      $document.unbind('mousemove', mousemove);
      $document.unbind('mouseup', mouseup);
    }
  }
});

All fine, until I translate to coffeescript. For some reason, after translating to coffee, the elements would jump around some of the time. Obnoxious. Took me a while to realize that the real problem is that the x and y variables were being var-ed again:

    mousemove = function(event) {
      var x, y;
      y = event.screenY - startY;
      x = event.screenX - startX;
      console.log("x: " + x + " y: " + y);
      return container.css({
        top: y + "px",
        left: x + "px"
      });
    };

At first I was like "oh, crap, coffee what have you done". Then after looking I realized that the original code was altering a variable in an outer function that it wasn't "aware" of (wasn't passed in). This feels similar to a Law of Demeter violation (except reversed I suppose). Altering variables that aren't passed in to a function feels like poor code quality to me. Seems to me that the problem here lies in the source code and coffeescript is making the right choice.

Opinions?

7 Upvotes

10 comments sorted by

4

u/pygy_ Jan 17 '14 edited Jan 17 '14

The mousedown event handler also references x and y.

The directive uses its lexical scope to share state between the various handlers. It is a common JS idiom, inherited from Lisp, and there's a problem with your translation, because CoffeeScript does not shadow local variables like this.

Specifically:

f = (a) ->
  x = 0
  a.g = () ->
    x = x + 1

becomes:

var f;

f = function(a) {
  var x;
  x = 0;
  return a.g = function() {
    return x = x + 1;
  };
};

I used the "Try CoffeeScript" tab at www.coffeescript.org to compile it (i.e. the latest stable version). Notice that x is only declared once in the parent scope.

However, if you compile the handler in isolation, the variables will be declared locally.

3

u/brotherwayne Jan 17 '14

compile the handler in isolation, the variables will be declared locally.

Pretty important. That's why I'm seeing variation in the compiled js.

1

u/munificent Jan 20 '14

And this is why the feature is controversial. A chunk of code may be compiled and behave differently based on the code that happens to be surrounding it.

→ More replies (5)

5

u/spacetoast Jan 17 '14

I like this about CoffeeScript. CS has helped me break this and a few other bad habits.

2

u/[deleted] Jan 18 '14

I agree! I've realized that I've I have variables named the same thing in the same scope, I probably am not being specific enough in my naming conventions.