refs #139, #138: Performance based changes - financetime instanceof, …

…string constructor removal, remove Array.join

* Adding financetime tests to ensure scale of null and undefined are handled gracefully
* Removing String constructor, profiling appears quicker without
* Removing usage of Array.join. Array.join is very slow in chrome

src/plot/axisannotation.js

@@ -138,19 +138,19 @@ function backgroundPath(accessor, axis, height, width, point, neg) {
         if(height/2 < point) pt = height/2;
         else h = height/2-point;
 
-        return 'M 0 ' + value + ' l ' + String(neg*Math.max(axis.innerTickSize(), 1)) + ' ' + String(-pt) +
-          ' l 0 ' + String(-h) + ' l ' + (neg*width) + ' 0 l 0 ' + height +
-          ' l ' + (neg*-width) + ' 0 l 0 ' + String(-h);
+        return 'M 0 ' + value + ' l ' + (neg*Math.max(axis.innerTickSize(), 1)) + ' ' + (-pt) +
+          ' l 0 ' + (-h) + ' l ' + (neg*width) + ' 0 l 0 ' + height +
+          ' l ' + (neg*-width) + ' 0 l 0 ' + (-h);
       case 'top':
       case 'bottom':
         var w = 0;
 
         if(width/2 < point) pt = width/2;
         else w = width/2-point;
 
-        return 'M ' + value + ' 0 l ' + String(-pt) + ' ' + (neg*Math.max(axis.innerTickSize(), 1)) +
-          ' l ' + String(-w) + ' 0 l 0 ' + String(neg*height) + ' l ' + width + ' 0 l 0 ' + (neg*-height) +
-          ' l ' + String(-w) + ' 0';
+        return 'M ' + value + ' 0 l ' + (-pt) + ' ' + (neg*Math.max(axis.innerTickSize(), 1)) +
+          ' l ' + (-w) + ' 0 l 0 ' + (neg*height) + ' l ' + width + ' 0 l 0 ' + (neg*-height) +
+          ' l ' + (-w) + ' 0';
       default: throw "Unsupported axis.orient() = " + axis.orient();
     }
   };

src/plot/candlestick.js

@@ -35,26 +35,17 @@ module.exports = function(d3_scale_linear, d3_extent, accessor_ohlc, plot, plotM
           width = p.width(x);
 
       return function(d) {
-        var path = [],
-            open = y(accessor.o(d)),
+        var open = y(accessor.o(d)),
             close = y(accessor.c(d)),
-            xValue = x(accessor.d(d)) - width/2;
-
-        path.push(
-          'M', xValue, open,
-          'l', width, 0
-        );
+            xValue = x(accessor.d(d)) - width/2,
+            path = 'M ' + xValue + ' ' + open + ' l ' + width + ' ' + 0;
 
         // Draw body only if there is a body (there is no stroke, so will not appear anyway)
         if(open != close) {
-          path.push(
-            'L', xValue + width, close,
-            'l', -width, 0,
-            'L', xValue, open
-          );
+          path += ' L ' + (xValue + width) + ' ' + close + ' l ' + -width + ' ' + 0 + ' L ' + xValue  + ' ' + open;
         }
 
-        return path.join(' ');
+        return path;
       };
     }
 
@@ -65,32 +56,18 @@ module.exports = function(d3_scale_linear, d3_extent, accessor_ohlc, plot, plotM
         width = p.width(x);
 
       return function(d) {
-        var path = [],
-            open = y(accessor.o(d)),
+        var open = y(accessor.o(d)),
             close = y(accessor.c(d)),
             xPoint = x(accessor.d(d)),
-            xValue = xPoint - width/2;
-
-        // Top
-        path.push(
-          'M', xPoint, y(accessor.h(d)),
-          'L', xPoint, Math.min(open, close)
-        );
+            xValue = xPoint - width/2,
+            path = 'M ' + xPoint + ' ' + y(accessor.h(d)) +' L ' + xPoint + ' '+ Math.min(open, close); // Top
 
         // Draw another cross wick if there is no body
         if(open == close) {
-          path.push(
-            'M', xValue, open,
-            'l', width, 0
-          );
+          path += ' M ' + xValue + ' ' + open + ' l ' + width + ' ' + 0;
         }
         // Bottom
-        path.push(
-          'M', xPoint, Math.max(open, close),
-          'L', xPoint, y(accessor.l(d))
-        );
-
-        return path.join(' ');
+        return path + ' M ' + xPoint + ' ' + Math.max(open, close) + ' L ' + xPoint + ' ' + y(accessor.l(d));
       };
     }
 

src/plot/macd.js

@@ -40,7 +40,7 @@ module.exports = function(accessor_macd, plot, plotMixin) {  // Injected depende
           xValue = x(accessor.d(d)) - width/2;
 
         return 'M ' + xValue + ' ' + zero + ' l 0 ' + height + ' l ' + width +
-          ' 0 l 0 ' + String(-height);
+          ' 0 l 0 ' + (-height);
       };
     }
 

src/plot/plot.js

@@ -75,15 +75,24 @@ module.exports = function(d3_svg_line, d3_select) {
   }
 
   function appendPlotTypePath(g, data, plotNames, direction) {
-    g.selectAll('path.' + plotNames.join('.') + '.' + direction).data(function(d) { return [d.filter(data)]; })
-      .enter().append('path').attr('class', plotNames.join(' ') + ' ' + direction);
+    g.selectAll('path.' + arrayJoin(plotNames, '.') + '.' + direction).data(function(d) { return [d.filter(data)]; })
+      .enter().append('path').attr('class', arrayJoin(plotNames, ' ') + ' ' + direction);
   }
 
   function barWidth(x) {
     if(x.band !== undefined) return Math.max(x.band(), 1);
     else return 3; // If it's not a finance time, the user should specify the band calculation (or constant) on the plot
   }
 
+  function arrayJoin(array, delimiter) {
+    if(!array.length) return;
+    var result = array[0];
+    for(var i = 1; i < array.length; i++) {
+      result += delimiter + array[i];
+    }
+    return result;
+  }
+
   return {
     dataMapper: {
       unity: function(d) { return d; },
@@ -139,7 +148,7 @@ module.exports = function(d3_svg_line, d3_select) {
      */
     joinPath: function(path) {
       return function(data) {
-        return data.map(path()).join(' ');
+        return arrayJoin(data.map(path()), ' ');
       };
     },
 

src/plot/supstance.js

@@ -74,12 +74,9 @@ function refresh(g, plot, accessor, x, y, annotationSelection, annotation) {
 
 function supstancePath(accessor, x, y) {
   return function(d) {
-    var path = [],
-        range = x.range();
+    var range = x.range();
 
-    path.push('M', range[0], y(accessor(d)));
-    path.push('L', range[range.length-1], y(accessor(d)));
-
-    return path.join(' ');
+    return 'M ' + range[0] + ' ' + y(accessor(d)) +
+      ' L ' + range[range.length-1] + ' ' + y(accessor(d));
   };
 }

src/plot/trendline.js

@@ -102,12 +102,8 @@ function refresh(g, accessor, x, y) {
 
 function trendlinePath(accessor, x, y) {
   return function(d) {
-    var path = [];
-
-    path.push('M', x(accessor.sd(d)), y(accessor.sv(d)));
-    path.push('L', x(accessor.ed(d)), y(accessor.ev(d)));
-
-    return path.join(' ');
+    return 'M ' + x(accessor.sd(d))+ ' ' + y(accessor.sv(d)) +
+      ' L ' + x(accessor.ed(d)) + ' ' + y(accessor.ev(d));
   };
 }
 

src/plot/volume.js

@@ -38,7 +38,7 @@ module.exports = function(accessor_volume, plot, plotMixin) {  // Injected depen
           xValue = x(accessor.d(d)) - width/2;
 
         return 'M ' + xValue + ' ' + zero + ' l 0 ' + height + ' l ' + width +
-          ' 0 l 0 ' + String(-height);
+          ' 0 l 0 ' + (-height);
       };
     }
 

src/scale/financetime.js

@@ -34,7 +34,7 @@ module.exports = function(d3_scale_linear, d3_time, d3_bisect, techan_util_rebin
      * @returns {*}
      */
     function scale(x, offset) {
-      var mappedIndex = dateIndexMap[x.getTime ? x.getTime() : +x];
+      var mappedIndex = dateIndexMap[x instanceof Date ? x.getTime() : +x];
       offset = offset || 0;
 
       // Make sure the value has been mapped, if not, determine if it's just before, round in, or just after domain

src/svg/arrow.js

@@ -10,7 +10,7 @@ module.exports = function(d3_functor) {  // Injected dependencies
         tail = d3_functor(true);
 
     function arrow(d, i) {
-      var path = [],
+      var path,
           x = fx(d, i),
           y = fy(d, i),
           w = width(d, i),
@@ -22,32 +22,31 @@ module.exports = function(d3_functor) {  // Injected dependencies
           pw = w/2,         // Point width
           ph = t ? h/2 : h; // Point Height
 
-      path.push('M', x, y);
+      path = 'M ' + x + ' ' + y;
 
       switch(o) {
         case 'up':
         case 'down':
-          path.push('l', -pw, neg*ph, 'l', ws, 0);
-          if(t) path.push('l', 0, neg*ph);
-          path.push('l', ws, 0);
-          if(t) path.push('l', 0, -neg*ph);
-          path.push('l', ws, 0);
+          path += ' l ' + -pw + ' ' + neg*ph + ' l ' + ws + ' ' + 0;
+          if(t) path += ' l ' + 0 + ' ' + neg*ph;
+          path += ' l ' + ws + ' ' + 0;
+          if(t) path += ' l ' + 0 + ' ' + -neg*ph;
+          path += ' l ' + ws + ' ' + 0;
           break;
 
         case 'left':
         case 'right':
-          path.push('l', neg*ph, -pw, 'l', 0, ws);
-          if(t) path.push('l', neg*ph, 0);
-          path.push('l', 0, ws);
-          if(t) path.push('l', -neg*ph, 0);
-          path.push('l', 0, ws);
+          path += ' l ' + neg*ph + ' ' + -pw + ' l ' + 0 + ' ' + ws;
+          if(t) path += ' l ' + neg*ph + ' ' + 0;
+          path += ' l ' + 0 + ' ' + ws;
+          if(t) path += ' l ' + -neg*ph + ' ' + 0;
+          path += ' l ' + 0 + ' ' + ws;
           break;
 
         default: throw "Unsupported arrow.orient() = " + orient;
       }
 
-      path.push('z');
-      return path.join(' ');
+      return path + ' z';
     }
 
     arrow.x = function(_) {

test/spec/bundle/plot/plotSpec.js

@@ -19,6 +19,19 @@ techanModule('plot/plot', function(specBuilder) {
         x = techan.scale.financetime();
       });
 
+      describe('And I have a path generator that returns a path', function() {
+        function pathGenerator() {
+          return function(d) {
+            return 'L 123 M 1 Z';
+          };
+        }
+
+        it('Then joinPath will join the paths together to a single string', function() {
+          // Don't care about values in the data array
+          expect(plot.joinPath(pathGenerator)([undefined, undefined, undefined])).toEqual('L 123 M 1 Z L 123 M 1 Z L 123 M 1 Z');
+        });
+      });
+
       describe('And scale has a small domain and large range', function() {
         beforeEach(function() {
           x.range([0, 1000]);

test/spec/bundle/scale/financetimeSpec.js

@@ -37,6 +37,14 @@ techanModule('scale/financetime', function(specBuilder) {
           expect(financetime.band()).toEqual(80);
         });
 
+        it('Then scale of undefined should not throw error and result in value just outside of the greater range extent (+undefined === NaN)', function() {
+          expect(financetime(undefined)).toEqual(1100);
+        });
+
+        it('Then scale of null should not throw error and result in value just outside of the lesser extent (+null === 0)', function() {
+          expect(financetime(null)).toEqual(0);
+        });
+
         it('Then scale of first index should return min widened range', function() {
           expect(Math.round(financetime(data[0])*1000)/1000).toEqual(100);
         });