Processing image blocks using a task runner

Question

This code works, but I'm sure it isn't written according to "best practices" for closures. On the other hand, at least it's intuitive to me...

the taskRunner object runs functions on setTimeout, so as not to block the UI for too long. I'm breaking this function into chunks and running one chunk at a time. I'm passing i into my function so it will be part of the closure, but I know I've seen this done more elegantly - I just can't wrap my head around how to do it right now.

var tr = new taskRunner();
 //I'm sure there's a better way to do a closure, but works
 for(var i=0;i<blocks;i++){
 var blockFunc = (function(i){
 var innerFunc = function(){
 var blockLen=Math.min(32768,len-(i*32768));
 stream.push(i==(blocks-1)?0x01:0x00);
 Array.prototype.push.apply(stream, blockLen.bytes16sw() );
 Array.prototype.push.apply(stream, (~blockLen).bytes16sw() );
 var id=imageData.slice(i*32768,i*32768+blockLen);
 Array.prototype.push.apply(stream, id );
 }
 return innerFunc;
 }(i));
 tr.AddTask(blockFunc);
 }

Answer 1

IMO, inlined functions add clutter. I think it's clearer and cleaner to use a named function.

function makeBlockFunc(i) {
 return function(){
 var blockLen=Math.min(32768,len-(i*32768));
 stream.push(i==(blocks-1)?0x01:0x00);
 Array.prototype.push.apply(stream, blockLen.bytes16sw() );
 Array.prototype.push.apply(stream, (~blockLen).bytes16sw() );
 var id=imageData.slice(i*32768,i*32768+blockLen);
 Array.prototype.push.apply(stream, id );
 };
}

Then invoke the function in the loop, passing i.

var tr = new taskRunner();
for(var i=0;i<blocks;i++){
 tr.AddTask(makeBlockFunc(i));
}

Also, you didn't really need an inner and outer function. You could have done the .AddTask() directly. Here the returned function is added directly into the tr.

• Aerik: Oh, that's much prettier at least. Though I don't think I want makeBlockFunc to be global in scope - but I do see now that it's not very clean to re-declare it in the loop.
• user1689607: @Aerik: It doesn't necessarily need to be global. It can be in the same scope where the rest of your code is, or you can add it to a namespace if you're using one. But nothing in the function itself relies on the surrounding variable scope. As long as you have access to it, you can create and store it anywhere.

Answer 2

Try this :

function blockFunc(i) {
 var n = i * 32768;
 return function() {
 var blockLen = Math.min(32768, len - n);
 stream.push(
 i==(blocks-1) ? 0x01 : 0x00,
 blockLen.bytes16sw(),
 (~blockLen).bytes16sw(),
 imageData.slice(n, n + blockLen)
 );
 };
}
var tr = new taskRunner();
for(var i=0; i<blocks; i++){
 tr.AddTask(blockFunc(i));
}

Notes :

• If stream.push() works, then there's no need to use Array.prototype.push.apply(...).
• With multiple arguments, .push() will push each arg in turn.
• With var n = i * 32768 in the outer function, the calculation is performed once per i.
• It may also be possible to move the var blockLen = ... calculation into the outer function, depending on whether or not len needs to be read "live" each time the task is run.

Answer 3

You could just bind to stream with i as a passed parameter, Though Im wondering why you are using Array.prototype.push when you could just use stream.push(), unless I've infered wrong, and stream isn't an array:

var tr = new taskRunner();
for(var i=0;i<blocks;i++){
 tr.AddTask((function (i) {
 var blockLen = Math.min(32768, len - (i*32768));
 this.push(i == (blocks - 1) ? 0x01 : 0x00);
 Array.prototype.push.apply(this, blockLen.bytes16sw() );
 Array.prototype.push.apply(this, (~blockLen).bytes16sw() );
 var id = imageData.slice(i*32768, i*32768 + blockLen);
 Array.prototype.push.apply(this, id );
 }).bind(stream,i));
}

• Aerik: Hmm... I don't know either - I'll have to look. I found this function and I'm making it asynchronous with a callback. (It's a javascript implementation of Canvas.toDataURL, since it isn't implemented on Android - but it was too slow to run synchronously)