0

In my program I have added the Array.prototype.equals() function listed in this SO post.

I have the following function to perform deep-copies of arrays (I am using this strictly for logging purposes - so that I can see what my arrays look like at various points in time in the Chrome console.)

function deepCopy(array) {
    return $.extend(true, [], array);
}

I then use this as such:

console.log("Sorting metrics list by metric name...");
console.log("Metrics list before sort:");
console.log(deepCopy(metrics));
metrics.sort(compare);
console.log("Metrics list after sort:");
console.log(deepCopy(metrics));

For reference, my compare function is:

function compare(a, b) {
  return a.name.localeCompare(b.name);
}

When I view my metrics array in the Chrome console after the deepCopy(), the equals method defined for the Array.prototype is IN the array! Picture:

enter image description here

As you can see from the screen shot, I check the actual value of the metrics array after everything has run, and it clearly does not contain the equals method. This is my reasoning for believing it has something to do with how jQuery().extend handles copying arrays.

Any ideas on how to fix my deepCopy function to stop this? (Or change my prototype function, if needed.)

Community
  • 1
  • 1
Matthew Herbst
  • 29,477
  • 23
  • 85
  • 128
  • 1
    Your specific problem aside, it's generally considered a bad idea to attach things to default type prototypes. – Etheryte Oct 31 '14 at 02:15
  • @Nit is there a better way to implement an array equals function? The method I used from the SO post I reference seems to be the most common one I could find. – Matthew Herbst Oct 31 '14 at 02:21
  • 2
    This is specifically because you've added a method to the `prototype`. It has nothing to do with using `$.extend` or your `deepCopy`. Add the `prototype` method to `Array`, create a new array, and `console.log` it - you'll see it. `JSON.stringify` or `.toString()` it and you won't see it. The default enumerability of a `prototype` method is to enumerate it, so that's why it's showing up. Using `Object.defineProperty` can "fix" it – Ian Oct 31 '14 at 02:23
  • They likely do a `for-in` enumeration on arrays. This type of enumeration includes prototype properties. That's very unfortunate if they actually do that. –  Oct 31 '14 at 02:24
  • Please post the contents of `deepCopy()` and example of the data structure "metrics" would be. And yeah, I agree with Nit, running off of native prototypes is a bad idea. – Scott Sword Oct 31 '14 at 02:24
  • 1
    @Ian: After using `.extend` as he shows in the question, if you do `result.hasOwnProperty("equals");` you get a `true` result, so jQuery is apparently including prototype properties in the process. –  Oct 31 '14 at 02:26
  • @Ian Thanks for pointing that out. So this begs the question then, how do I create the equals method without this problem? Should I just create a standalone function that takes two arrays? – Matthew Herbst Oct 31 '14 at 02:29
  • ...demo: http://jsfiddle.net/t1bx809L/ –  Oct 31 '14 at 02:29
  • @squint Hmm good point, I didn't bother testing that far once I found that it looked like it was just a `console.log` thing. While it's being copied, I don't see how it's posing a problem for the OP. It's undesired, but I don't think it's an actual problem, is it? – Ian Oct 31 '14 at 02:29
  • @Ian: Probably only a problem if he's doing the same ill-advised enumeration of the Array using `for-in` that jQuery is apparently doing. –  Oct 31 '14 at 02:32
  • @squint Exactly. Other than, I wasn't sure what else could go wrong. I just looked in the jQuery source...unless I'm reading it wrong, it looks like the first thing they do is `for..in` the passed object/array. Which would cause the problems, as pointed out – Ian Oct 31 '14 at 02:33
  • @MatthewHerbst Technically yes, a standalone function would work. Or, if you can deal with the browser support, `Object.defineProperty` can change the enumerability. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/defineProperty – Ian Oct 31 '14 at 02:34
  • @Ian Thanks. So, just to understand what you an squint were talking about: this is a problem with how console.log() handles enumeration of the array? – Matthew Herbst Oct 31 '14 at 02:37
  • @Ian: Yeah, that's unfortunate. Probably too late for them to change it now since it could probably break a little code here and there that relies on that behavior. –  Oct 31 '14 at 02:37
  • 1
    @MatthewHerbst Depending on how you plan to use the array, it should be fine. Technically, what it seems to be doing is copying the **prototype** `equals` method onto the actual instance as a method. Which is doing nothing functionality-wise, except on that copied instance, it's "overriding" **its** `prototype` method (but they reference the same function). As long as **you** treat arrays normally (which jQuery doesn't seem to be doing), you should be fine...even with the code you have. `console.log` is working correctly - it's showing the enumerable property you added to the `prototype` – Ian Oct 31 '14 at 02:40
  • @squint Yep, unfortunate. Here's where I was referencing (if you care): https://github.com/jquery/jquery/blob/1.11.1/dist/jquery.js#L205 – Ian Oct 31 '14 at 02:41
  • @MatthewHerbst: As Ian showed, jQuery is using `for-in` to enumerate your Array. This is bad JS citizenship IMO, but it's probably too dug in for them to correct at this point. However, as long as you don't use `for-in` to enumerate an Array *(which you almost never should)*, you'll be fine. –  Oct 31 '14 at 02:46
  • @Ian: Thanks for the link. Always good to see it first hand. –  Oct 31 '14 at 02:46
  • 2
    @MatthewHerbst: ...there will be those who will try to speak in absolute terms that you must never modify objects like native prototypes. Just be aware that there is absolutely no reason for such a restriction as long as you embrace coding practices that are *aware* of such modifications. –  Oct 31 '14 at 02:53
  • @Ian Thanks for helping me out - really appreciate it! – Matthew Herbst Oct 31 '14 at 06:02
  • @squint Thanks for helping me out - really appreciate it! – Matthew Herbst Oct 31 '14 at 06:03

1 Answers1

2

Two issues here:

  • you're adding an enumerable property on the Array prototype
  • a console oddity: console.log is different from expression evaluation

$.extends loops over all enumerable properties of the source object and adds them as own instance properties (not inherited properties) of the target object. If you make your prototype property non-enumerable, it won't be copied directly onto the target:

Object.defineProperty(Array.prototype, "equals", {
    enumerable: false,
    value: function(array) { /* ... */ }
});

(See some helpful reading: Don’t modify objects you don’t own.)

Your final test where you type metrics into the console doesn't show you the equals property, but it's actually still there. Consider the following console session in Chrome, where we add an enumerable property on the Array prototype and then use an array with $.extends:

> Array.prototype.foo = "egg";
"egg"

> $.extend(true, [], ['bar', 'baz'])
["bar", "baz"]

> console.log($.extend(true, [], ['bar', 'baz']))
["bar", "baz", foo: "egg"] 

> Object.getOwnPropertyNames( $.extend(true, [], ['bar', 'baz']) )
["0", "1", "length", "foo"]

The non-index properties only show up when we explictly use console.log; they don't show up when we just evaluate the expression, even though getOwnPropertyNames confirms they're there.

apsillers
  • 112,806
  • 17
  • 235
  • 239
  • 2
    `foo = "egg"`... That's a new one for me. Is that like Egg Foo Young? –  Oct 31 '14 at 02:48