Add sparse array support like a boss. #1

Closed
jdalton opened this Issue Nov 11, 2011 · 15 comments

2 participants

@jdalton

For extra bonus points you should make this method closer to the ES5.1 one.
So this:

forEach(arr, eachFn, doneFn)

would become

forEach(object, callback [, options]);

options would have callback params like onDone as well as a bound prop or something.

Also, internally you should resolve length like var length = object.length >>> 0; and support sparse arrays (in operator checks).

I have a method similar to this in benchmark.js (also allows async calling).

@cowboy
Owner

What would the bound option do? If it's for specifying the this value of the callback, that doesn't make sense as a specific this value is already used. Is it for something else?

Also, why this: var length = object.length >>> 0; ?

@jdalton

What would the bound option do? If it's for specifying the this value of the callback, that doesn't make sense as a specific this value is already used.

Oh, you should probably change that then.

Also, why this: var length = object.length >>> 0; ?

It's a way to resolve a valid Array length (ToUint32).

@cowboy
Owner

I definitely won't change the way this works, that was the entire point of this lib. Not the forEach iteration part, which can be (and has been) done any one of a bazillion ways.

@cowboy
Owner

Also is that right-shift technique for resolving a valid Array length documented? I understand how >>> works, but why (specifically) is that recommended for handling Array lengths?

@jdalton

Also is that right-shift technique for resolving a valid Array length documented? I understand how >>> works, but why (specifically) is that recommended for handling Array lengths?

It's just an easy way to get the ToUint32 that is spec'ed for Array length.

@cowboy
Owner

Ok. So, in what situation(s) is arr.length not enough, and arr.length >>> 0 necessary?

@jdalton

the >>> 0 is just more correct than leaving arr.length as normal.
This would come in to play more on array-like objects which may have a non-standard length value.

var a = { 'length': 4294967299 };
var b = [].slice.call(a);
console.log(b.length); // 3
console.log(a.length >>> 0); // 3

Also don't forget about the sparse arrays (in checks).

@cowboy
Owner

Ok, so when would you ever encounter an invalid length? When would arr.length >>> 0 ever actually be necessary?

Also, would this be a good approach for handling a sparse array? I've never really dealt with them before.

Array.prototype[2] = "foo";

var arr = [];
arr[4] = "bar";

// native forEach iteration:
arr.forEach(console.log.bind(console));
// logs:
// foo 2 [undefined, undefined, undefined, undefined, "bar"]
// bar 4 [undefined, undefined, undefined, undefined, "bar"]

// for loop iteration:
for (var i = 0; i < arr.length; i++) {
  if (i in arr) {
    console.log(arr[i], i, arr);
  }
}
// logs:
// foo 2 [undefined, undefined, undefined, undefined, "bar"]
// bar 4 [undefined, undefined, undefined, undefined, "bar"]
@jdalton

Ok, so when would you ever encounter an invalid length? When would arr.length >>> 0 ever actually be necessary?

If you allow array-like-objects.

// for loop iteration:
for (var i = 0; i < arr.length; i++) {
  if (i in arr) {
    console.log(arr[i], i, arr);
  }
}

Yap you got it, the in operator is the key.

@cowboy
Owner

I still don't understand a situation where someone would actually have an invalid length. What kind of array-like objects have intrinsically invalid lengths?

@jdalton

I still don't understand a situation where someone would actually have an invalid length. What kind of array-like objects have intrinsically invalid lengths?

Because spec allows many methods, including arrays to be this generic, it's a way to sanitize some required props.

@cowboy
Owner

Correct me if I'm wrong, but what you're saying is that because the Array.prototype.forEach sanitizes this.length using >>> 0 that my forEach should as well. Right?

[].forEach.call({length: 4294967299, 0: "a", 2: "b"}, console.log.bind(console));
// logs:
// a 0 { '0': 'a', '2': 'b', length: 4294967299 }
// b 2 { '0': 'a', '2': 'b', length: 4294967299 }

The reason I ask is that it seems a little silly for my code to convert an obviously invalid length into a valid one instead of throwing an exception. Unless I'm going for spec compliance (which I'm not).

@jdalton

Correct me if I'm wrong, but what you're saying is that because the Array.prototype.forEach sanitizes this.length using >>> 0 that my forEach should as well. Right?

[].forEach.call({length: 4294967299, 0: "a", 2: "b", 3: "c" }, console.log.bind(console));
// logs
// a 0 { '0': 'a', '2': 'b', length: 4294967299 }
// b 2 { '0': 'a', '2': 'b', length: 4294967299 }

Yap, you know me, I side w/ spec'ed behavior when possible ;D

The reason I ask is that it seems a little silly for my code to convert an obviously invalid length into a valid one instead of throwing an exception. Unless I'm going for spec compliance (which I'm not).

It's one line, and since you named your method forEach I figured having some resemblance to the spec'ed Array#forEach might be a good idea.

(also don't forget sparse array support, the reason the issue was created)

@cowboy
Owner

Ok, fair enough. The sparse array support made sense, the length stuff didn't. I'll add it all in. Thanks!

@cowboy
Owner

BTW, thanks for fixing my example by adding , 3: "c". I was trying to think of how to properly test that my code was only iterating from 0 to 2, and didn't think of that!

@cowboy cowboy added a commit that referenced this issue Nov 17, 2011
@cowboy Releasing v0.1.2.
* Adding sparse array support.
* Invalid length properties are now sanitized.
* This closes issue #1 (like a boss).
ff46303
@cowboy cowboy closed this Nov 17, 2011
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment