From edf4e24e213fdd2e40efd0b5a7feba5d0160e452 Mon Sep 17 00:00:00 2001
From: jos <wjosdejong@gmail.com>
Date: Sat, 2 Jan 2016 20:57:13 +0100
Subject: [PATCH] Some refactoring

---
 lib/expression/node/IndexNode.js  | 16 +++++++-----
 lib/expression/node/UpdateNode.js | 43 +++++++++++++++----------------
 2 files changed, 30 insertions(+), 29 deletions(-)

diff --git a/lib/expression/node/IndexNode.js b/lib/expression/node/IndexNode.js
index 4fd866515..dc1337f1b 100644
--- a/lib/expression/node/IndexNode.js
+++ b/lib/expression/node/IndexNode.js
@@ -51,18 +51,18 @@ function factory (type, config, load, typed) {
    *                          the name of the argument, and the value is `true`.
    *                          The object may not be mutated, but must be
    *                          extended instead.
-   * @return {string} js
+   * @return {string} code
    * @private
    */
   IndexNode.prototype._compile = function (defs, args) {
     var childArgs = Object.create(args);
-    var sizeNeeded = this.ranges.some(IndexNode.containsEndSymbol);
 
-    var index = this.compileIndex(defs, childArgs);
+    var compiledIndex = this.compileIndex(defs, childArgs);
+    var index = compiledIndex.code;
     var object = this.object._compile(defs, childArgs);
 
     // if some parameters use the 'end' parameter, we need to calculate the size
-    if (sizeNeeded) {
+    if (compiledIndex.sizeNeeded) {
       return '(function () {' +
           '  var obj = ' + object + ';' +
           '  var size = math.size(obj).valueOf();' +
@@ -83,9 +83,8 @@ function factory (type, config, load, typed) {
    *                                the name of the argument, and the value is `true`.
    *                                The object may not be mutated, but must be
    *                                extended instead.
-   * @return {string} js
+   * @return {{code: string, sizeNeeded: boolean}}
    */
-  // TODO: let compileIndex return an object which denotes whether the matrix size is needed
   IndexNode.prototype.compileIndex = function (defs, args) {
     // helper function to create a Range from start, step and end
     defs.range = function (start, end, step) {
@@ -144,7 +143,10 @@ function factory (type, config, load, typed) {
       }
     });
 
-    return 'math.index(' + ranges.join(', ') + ')';
+    return {
+      sizeNeeded: this.ranges.some(IndexNode.containsEndSymbol),
+      code: 'math.index(' + ranges.join(', ') + ')'
+    };
   };
 
   /**
diff --git a/lib/expression/node/UpdateNode.js b/lib/expression/node/UpdateNode.js
index 1fe263c7c..63ea670d8 100644
--- a/lib/expression/node/UpdateNode.js
+++ b/lib/expression/node/UpdateNode.js
@@ -47,38 +47,37 @@ function factory (type, config, load, typed) {
    * @private
    */
   UpdateNode.prototype._compile = function (defs, args) {
-    var symbol = this.index.filter(function (node) { return node.isSymbolNode; })[0];
-    var sizeNeeded = this.index.ranges.some(IndexNode.containsEndSymbol);
-    var multipleIndexes = this.index.object.isIndexNode;
     var replacement = this.expr._compile(defs, args);
+    var compiledIndex = this.index.compileIndex(defs, args);
+    var symbol = this.index.filter(function (node) {
+      return node.isSymbolNode;
+    })[0];
+    var indexNodes = this.filter(function (node) {
+      return node.isIndexNode;
+    });
 
-    if (multipleIndexes || sizeNeeded) {
+    if (indexNodes.length > 1 || compiledIndex.sizeNeeded) {
       // TODO: refactor this code, it's too complicated
       var childArgs = args;
-      var code = this.index
-          .filter(function (node) {
-            return node.isIndexNode;
-          })
-          .reduce(function (code, node) {
-            childArgs = Object.create(childArgs); // childArgs can be mutated by compileRanges
-            var index = node.compileIndex(defs, childArgs);
-            var sizeNeeded = node.ranges.some(IndexNode.containsEndSymbol);
+      var code = indexNodes.reduce(function (code, node) {
+        childArgs = Object.create(childArgs); // childArgs can be mutated by compileRanges
+        var compiledIndex = node.compileIndex(defs, childArgs);
+        var index = compiledIndex.code;
+        var inner = code ? '(' + code + ')(math.subset(obj, index))' : replacement;
 
-            var inner = code ? '(' + code + ')(math.subset(obj, index))' : replacement;
-
-            return 'function (obj) {\n' +
-                (sizeNeeded ? 'var size = math.size(obj).valueOf();\n' : '') +
-                'var index = ' + index + ';\n' +
-                'return math.subset(obj, index, ' + inner + ');\n' +
-                '}';
-          }, null);
+        return 'function (obj) {\n' +
+            (compiledIndex.sizeNeeded ? 'var size = math.size(obj).valueOf();\n' : '') +
+            'var index = ' + index + ';\n' +
+            'return math.subset(obj, index, ' + inner + ');\n' +
+            '}';
+      }, null);
 
       return 'scope["' + symbol.name + '"] = (' + code + ')(' + symbol._compile(defs, args) + ')';
     }
     else {
-      // simple, single subset replacement. No need for closures and nesting.
-      var index = this.index.compileIndex(defs, args);
+      // simple, single subset replacement. No need for closures and nesting stuff.
       var object = this.index.object._compile(defs, args);
+      var index = compiledIndex.code;
 
       return 'scope["' + symbol.name + '"] = ' +
           'math.subset(' + object + ', ' + index + ', ' + replacement + ')';