Hi Nick, I took a quick look at your code. Your comments already point out a few things you want to fix, so I’ve not mentioned any of those. My comments are:
1. Lines 34 and 36 can be combined (could be var slice = Array.prototype.slice,
noodles;).
2. On line 47 – I typically like to put the public interface for a library at
the bottom of the file. Nothing technically wrong with putting it at the top, I
just find it easier to follow when I define the individual pieces first and
then export the public interface.
3. I see you’ve used the timed array processing technique from High Performance
JavaScript. Glad you’re having fun with it.
4. There’s a handful of places where you use underscore as a named function
argument. I prefer to always use explicit names so there’s no confusion. I’d
also suggest that some of the other function arguments could be a bit longer to
better indicate their usage.
5. Both in every() and some() you have a try-catch around a call to reduce().
However, since the internals for reduce() use timers to delay processing, you
won’t catch any errors that occur after 50ms (the timer code runs in the global
scope, outside of the try-catch).
6. In every() and some(), it would be helpful to have some comments about
arguments, as this code is a bit confusing:
if ( arguments.length < 3) { callback = testFn || function ()
{}; testFn = function (o, next) { return next(!!o);
}; }I had to read it a few times before I realized (I think)
that when there are two arguments, the second is considered the callback
instead of the test function. I’m not sure if that’s intentional or not, but I
can’t think of a reason why you’d want do this (wouldn’t you just be iterating
over the items in the array without doing anything to those items?).
Overall, looks like a nifty bit of coding. I hope these comments help.
-Nicholas
_____________________________________________________
Nicholas C. Zakas
Twitter: @slicknet
Blog: http://www.nczonline.net/
From: Nick Fitzgerald
Sent: Friday, January 07, 2011 12:31 PM
To: [email protected]
Subject: [JSMentors] Request for code review: noodles - Async/non-blocking/CPS
versions of the higher order functions on `Array.prototype`
Hello everyone!
Noodles 277 lines long, but with over 100 lines of comments I hope I'm not
asking for too much when requesting code review :)
Its pretty well known in the JS circle that if you are iterating over a really
big list and doing a fairly expensive operation on each iteration, you can
block the browser's UI thread and make it look like it froze for a few seconds.
A common way to get around this (which is often easier than actually making
your algorithms more performant) is to use some asynchronous version of
`Array.prototype.forEach` so that the browser can have a moment to update the
UI between every few iterations. This is what I mean by non-blocking.
I thought it would be cool to have an async version for most of the HOFs on
Array.prototype. Then I thought, well what if the operation on each iteration
is asynchronous? And so I transformed it to use continuation passing style and
callbacks.
Right now, this is what is implemented: reduce, map, filter, forEach, every,
and some. Everything is implemented in terms of reduce (other than reduce
itself of course).
I would love to get some feedback on this mini project!
Thanks in advance to everyone!
Project: https://github.com/fitzgen/noodles
Main file: https://github.com/fitzgen/noodles/blob/master/noodles.js
_Nick_
--
To view archived discussions from the original JSMentors Mailman list:
http://www.mail-archive.com/[email protected]/
To search via a non-Google archive, visit here:
http://www.mail-archive.com/[email protected]/
To unsubscribe from this group, send email to
[email protected]
--
To view archived discussions from the original JSMentors Mailman list:
http://www.mail-archive.com/[email protected]/
To search via a non-Google archive, visit here:
http://www.mail-archive.com/[email protected]/
To unsubscribe from this group, send email to
[email protected]
<<wlEmoticon-smile[1].png>>
