epochjs / epoch

A general purpose, real-time visualization library.
http://epochjs.github.io/epoch
MIT License
4.97k stars 278 forks source link

Add hasOwnProperty guard #151

Closed jamesarosen closed 9 years ago

jamesarosen commented 9 years ago

@duretti @rsandor @jadeapplegate @oso-mate

CoffeeScript's for own x, y of z adds a hasOwnProperty guard to the iteration. That's important in general, but particularly when Array.prototype has been extended.

oso-mate commented 9 years ago

Is that own to ensure only properties of the object, not the chain? I didn't know this was a thing, cool!

jamesarosen commented 9 years ago
for x, y of z
  doSomething(x, y);

compiles to

var x, y;

for (x in z) {
  y = z[x];
  doSomething(x, y);
}

whereas

for own x, y of z
  doSomething(x, y);

compiles to

var x, y,
  __hasProp = {}.hasOwnProperty;

for (x in z) {
  if (!__hasProp.call(z, x)) continue;
  y = z[x];
  doSomething(x, y);
}

Object.prototype.hasOwnProperty will ignore inherited properties.

rsandor commented 9 years ago

@jamesarosen - Good call. I'll run the basic and rendering tests myself to double check functionality.

jamesarosen commented 9 years ago

@rsandor ps: I'm having trouble with cake test; each test complains that Epoch is undefined. (After running cake build.)

rsandor commented 9 years ago

@jamesarosen - Sorry for the delay, that's very interesting. I am not sure why it is happening since I am able to run tests just fine on my end. When you run cake build on its own does it throw out any errors?

rsandor commented 9 years ago

@jamesarosen - Unit tests are passing just fine, heatmap rendering tests are broken in both your branch and master (thus unrelated to your changes). Gonna address those in another pull. Merging your changes now.