EAGLESIX-2695 Ordered functions to match the c++ code. Minor grammar changes only.

lib/pipeline/expressions/LetExpression.js

@@ -1,10 +1,12 @@
 "use strict";
 
+Expression.registerExpression("$let", LetExpression.parse);
+
 var LetExpression = module.exports = function LetExpression(vars, subExpression){
-	if (arguments.length !== 2) throw new Error("Two args expected");
+	//if (arguments.length !== 2) throw new Error("Two args expected");
 	this._variables = vars;
 	this._subExpression = subExpression;
-}, klass = LetExpression, Expression = require("./Expression"), base = Expression, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
+}, klass = LetExpression, Expression = require("./FixedArityExpressionT")(klass, 2), base = Expression, proto = klass.prototype = Object.create(base.prototype, {constructor:{value:klass}});
 
 // DEPENDENCIES
 var Variables = require("./Variables"),
@@ -19,13 +21,21 @@ proto.parse = function parse(expr, vpsIn){
 	}
 
 	if(typeof(expr.$let) !== 'object' || (expr.$let instanceof Array)) {
-		throw new Error("$let only supports an object as it's argument:16874");
+		throw new Error("$let only supports an object as its argument: 16874");
 	}
 
 	var args = expr.$let,
 		varsElem = args.vars,
-		inElem = args['in'];
-
+		inElem = args['in']; // args.in; ??
+
+	//NOTE: DEVIATION FROM MONGO: 1. These if statements are in a loop in the c++ version,
+	// 2. 'vars' and 'in' are each mandatory here. in the c++ code you only need one of the two.
+	// 3. Below, we croak if there are moore than 2 arguments.  The original does not have this limitation, specifically.
+	// Upon further review, I think our code is more accurate.  The c++ code will accept if there are multiple 'in'
+	// or 'var' values. The previous ones will be overwritten by newer ones.
+	//
+	// Final note - I think this code is fine.
+	//
 	if(!varsElem) {
 		throw new Error("Missing 'vars' parameter to $let: 16876");
 	}
@@ -33,6 +43,9 @@ proto.parse = function parse(expr, vpsIn){
 		throw new Error("Missing 'in' parameter to $let: 16877");
 	}
 
+	// Should this be !== 2?  Why would we have fewer than 2 arguments?  Why do we even care what the length of the
+	// array is? It may be an optimization of sorts. But what we're really wanting here is, 'If any keys are not "in"
+	// or "vars" then we need to bugcheck.'
 	if(Object.keys(args).length > 2) {
 		var bogus = Object.keys(args).filter(function(x) {return !(x === 'in' || x === 'vars');});
 		throw new Error("Unrecognized parameter to $let: " + bogus.join(",") + "- 16875");
@@ -60,6 +73,8 @@ proto.optimize = function optimize() {
 
 	for(var id in this._variables){
 		for(var name in this._variables[id]) {
+			//NOTE: DEVIATION FROM MONGO: This is actually ok. The c++ code does this with a single map. The js structure
+			// is nested objects.
 			this._variables[id][name] = this._variables[id][name].optimize();
 		}
 	}
@@ -69,15 +84,15 @@ proto.optimize = function optimize() {
 	return this;
 };
 
-proto.addDependencies = function addDependencies(deps, path){
+proto.serialize = function serialize(explain) {
+	var vars = {};
 	for(var id in this._variables) {
 		for(var name in this._variables[id]) {
-			this._variables[id][name].addDependencies(deps);
+			vars[name] = this._variables[id][name];
 		}
 	}
-	this._subExpression.addDependencies(deps, path);
-	return deps;
 
+	return {$let: {vars:vars, 'in':this._subExpression.serialize(explain)}};
 };
 
 proto.evaluateInternal = function evaluateInternal(vars) {
@@ -90,16 +105,15 @@ proto.evaluateInternal = function evaluateInternal(vars) {
 	return this._subExpression.evaluateInternal(vars);
 };
 
-
-proto.serialize = function serialize(explain) {
-	var vars = {};
+proto.addDependencies = function addDependencies(deps, path){
 	for(var id in this._variables) {
 		for(var name in this._variables[id]) {
-			vars[name] = this._variables[id][name];
+			this._variables[id][name].addDependencies(deps);
 		}
 	}
+	this._subExpression.addDependencies(deps, path);
+	return deps; //NOTE: DEVIATION FROM MONGO: The c++ version does not return a value. We seem to use the returned value
+					// (or something from a different method named
+					// addDependencies) in many places.
 
-	return {$let: {vars:vars, 'in':this._subExpression.serialize(explain)}};
 };
-
-Expression.registerExpression("$let", LetExpression.parse);