Return an error immediately, rather than lumping it in with the resul…

…ts from the other invocations. Also, no longer suppress null and undefined results. Lastly, results are always returned in order now, even for the group function.
alexkwolfe · Aug 16, 2010 · 9c88141 · 9c88141
1 parent f4e1df3
commit 9c88141
Show file tree

Hide file tree

Showing 4 changed files with 115 additions and 35 deletions.
diff --git a/README.md b/README.md
@@ -28,16 +28,29 @@ A flow control library for node.js for executing multiple functions as a group o
 	gang.chain(functions, function(colors) {
 		puts("All colors were collected: " + colors);
 	});
+
+## Function results
+
+You must invoke the `done` function in each of your functions. This notifies the `group` or
+`chain` process that the function is done executing and provides an opportunity to return
+results. 
+
+Once all the `done` functions have been invoked, the callback is invoked with a parameter
+that contains an array of the function results. Note that the results are in the same order
+that the functions are declared, even for the `group` function. So if you're looking for the
+"return value" of the second function you can examine the results at the 1st array index.
 
-## Error handling
+## Handling null and undefined results
 
-If an error occurs in a function, pass the error to the `done` method. The error will be included
-among the values passed to the callback function.
+If the `done` method is called with a `null` parameter or no parameter, then `null` or
+`undefined` will be returned with other function values to the callback.
+
+## Error handling
 
-When an error occurs during chained execution, the chain stops and the values collected (including
-the error) are returned to the callback function.
+If an error occurs in a function, pass the error to the `done` method.  When an error occurs 
+execution stops and the error is returned to the callback. Other result values are discarded
 
-Check for errors in the values passed to the callback function and behave accordingly.
+It's up to you to check for errors in the values passed to the callback function and behave accordingly.
 
 	var gang = require('gang-bang'),
 	    puts = require('sys').puts;
@@ -52,10 +65,11 @@ Check for errors in the values passed to the callback function and behave accord
 
 	// execute functions concurrently, and callback when all functions have been called
 	gang.group(functions, function(colors) {
-		puts("Two colors and one error were collected: " + colors);
+		puts("Colors is an error: " + colors);
 	});
 
-	// execute each one after the other, and callback when the error occurrs
+	// execute each one after the other, and callback when the error occurs
+	// non-error results are discarded.
 	gang.chain(functions, function(colors) {
-		puts("One color and one error were collected: " + colors);
+		puts("Colors is an error: " + colors);
 	});
diff --git a/lib/gang-bang.js b/lib/gang-bang.js
@@ -12,9 +12,13 @@ exports.group = function (fxns, cb) {
 
 	var items_left_to_execute = fxns.length;
 	var results = [];
-	var callGroupFunction = function(fxn) {
+	var callGroupFunction = function(i, fxn) {
 		var done = function(result) {
-			results.push(result);
+			if (result instanceof Error)
+				return cb(result);
+
+			results[i] = result;
+
 			items_left_to_execute--;
 			if (!items_left_to_execute) 
 				cb(results);
@@ -23,7 +27,7 @@ exports.group = function (fxns, cb) {
 	};
 
 	for ( var i = 0; i < fxns.length; i++) {
-		callGroupFunction(fxns[i]);
+		callGroupFunction(i, fxns[i]);
 	}
 }
 /**
@@ -39,10 +43,12 @@ exports.chain = function (fxns, callback) {
     var callNext = function() {
 		var done = function(result) {
             pos++;
-			if (result) 
-				results.push(result);
+			if (result instanceof Error)
+				return callback(result);
+
+			results.push(result);
 
-            if (!(result instanceof Error) && fxns.length > pos) { 
+            if (fxns.length > pos) { 
                 callNext();
 			} else {
 				callback(results);

diff --git a/test/unit/chain_test.js b/test/unit/chain_test.js
@@ -31,8 +31,40 @@ exports.testResults = function(assert) {
 	});
 };
 
+exports.testNullResultsAreNotDiscarded = function(assert) {
+	assert.expect(1);
+
+	// order the results by setting a timeout. this "proves" concurrency.
+	var functions = [
+		function(done) {  done('red'); },
+		function(done) {  done(null); },
+		function(done) {  done('blue'); }
+	];
+
+	gang.group(functions, function(colors) {
+		assert.same(['red', null, 'blue'], colors);
+		assert.done();
+	});
+};
+
+exports.testUndefinedResultsNotDiscarded = function(assert) {
+	assert.expect(1);
+
+	// order the results by setting a timeout. this "proves" concurrency.
+	var functions = [
+		function(done) { done('red'); },
+		function(done) { done(); },
+		function(done) { done('blue'); },
+	];
+
+	gang.group(functions, function(colors) {
+		assert.same(['red', undefined, 'blue'], colors);
+		assert.done();
+	});
+};
+
 exports.testStopsOnError = function(assert) {
-	assert.expect(4);
+	assert.expect(2);
 
 	var functions = [
 		function(done) { done('red'); },
@@ -41,10 +73,8 @@ exports.testStopsOnError = function(assert) {
 	];
 
 	gang.chain(functions, function(colors) {
-		assert.equals(2, colors.length);
-		assert.equals('red', colors[0]);
-		assert.ok(colors[1] instanceof Error);
-		assert.equals('green', colors[1].message);
+		assert.ok(colors instanceof Error);
+		assert.equals('green', colors.message);
 		assert.done();
 	});
 };
diff --git a/test/unit/group_test.js b/test/unit/group_test.js
@@ -17,36 +17,66 @@ exports.testAllFunctionsCalled = function(assert) {
 }
 
 exports.testResults = function(assert) {
-	assert.expect(1);
+	assert.expect(2);
 
-	// order the results by setting a timeout. this "proves" concurrency.
+	// serial execution will take at least 900ms, but this is concurrently
+	// executed so it should take ~300ms
+	var started = new Date();
 	var functions = [
 		function(done) {  setTimeout(function() { done('red'); }, 300); },
-		function(done) {  setTimeout(function() { done('green'); }, 100); },
-		function(done) {  setTimeout(function() { done('blue'); }, 200); },
+		function(done) {  setTimeout(function() { done('green'); }, 300); },
+		function(done) {  setTimeout(function() { done('blue'); }, 300); },
 	];
 
 	gang.group(functions, function(colors) {
-		assert.same(['green', 'blue', 'red'], colors);
+		assert.ok(new Date() - started < 350);
+		assert.same(['red', 'green', 'blue'], colors);
 		assert.done();
 	});
 };
 
-exports.testDoesNotStopOnError = function(assert) {
-	assert.expect(5);
+exports.testNullResultsAreNotDiscarded = function(assert) {
+	assert.expect(1);
+
+	var functions = [
+		function(done) { done('red'); },
+		function(done) { done(null); },
+		function(done) { done('blue'); }
+	];
+
+	gang.group(functions, function(colors) {
+		assert.same(['red', null, 'blue'], colors);
+		assert.done();
+	});
+};
+
+exports.testUndefinedResultsAreNotDiscarded = function(assert) {
+	assert.expect(1);
 
 	var functions = [
-		function(done) {  setTimeout(function() { done('red'); }, 100); },
-		function(done) {  setTimeout(function() { done(new Error('green')); }, 200); },
-		function(done) {  setTimeout(function() { done('blue'); }, 300); },
+		function(done) { done('red'); },
+		function(done) { done(); },
+		function(done) { done('blue'); }
+	];
+
+	gang.group(functions, function(colors) {
+		assert.same(['red', undefined, 'blue'], colors);
+		assert.done();
+	});
+};
+
+exports.testStopsOnError = function(assert) {
+	assert.expect(2);
+
+	var functions = [
+		function(done) { done('red'); },
+		function(done) { done(new Error('green')); },
+		function(done) { done('blue'); }
 	];
 
 	gang.group(functions, function(colors) {
-		assert.equals(3, colors.length);
-		assert.equals('red', colors[0]);
-		assert.ok(colors[1] instanceof Error);
-		assert.equals('green', colors[1].message);
-		assert.equals('blue', colors[2]);
+		assert.ok(colors instanceof Error);
+		assert.equals('green', colors.message);
 		assert.done();
 	});
 };