cowboy / javascript-sync-async-foreach

An optionally-asynchronous forEach with an interesting interface.
MIT License
76 stars 11 forks source link

Add sparse array support like a boss. #1

Closed jdalton closed 12 years ago

jdalton commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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

jdalton commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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 commented 12 years ago

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

cowboy commented 12 years ago

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!