Merge pull request chartist-js#645 from hansmaad/fix643-getBounds

Fix chartist-js#643 Prevent infinite loop in getBounds if bounds.valueRange is very small
Waicung · Mar 18, 2016 · 796992d · 796992d
2 parents 3459590 + bfdc8f2
commit 796992d
Show file tree

Hide file tree

Showing 2 changed files with 105 additions and 4 deletions.
diff --git a/src/scripts/core.js b/src/scripts/core.js
@@ -727,6 +727,10 @@ var Chartist = {
       }
     }
 
+    // step must not be less than EPSILON to create values that can be represented as floating number.
+    var EPSILON = 2.221E-16;
+    bounds.step = Math.max(bounds.step, EPSILON);
+
     // Narrow min and max based on new step
     newMin = bounds.min;
     newMax = bounds.max;
@@ -740,11 +744,12 @@ var Chartist = {
     bounds.max = newMax;
     bounds.range = bounds.max - bounds.min;
 
-    bounds.values = [];
-    for (i = bounds.min; i <= bounds.max; i += bounds.step) {
-      bounds.values.push(Chartist.roundWithPrecision(i));
+    var values = [];
+    for (i = bounds.min; i <= bounds.max; i += bounds.step) {      
+      var value = Chartist.roundWithPrecision(i);      
+      value != values[values.length - 1] && values.push(i);
     }
-
+    bounds.values = values;
     return bounds;
   };
 

diff --git a/test/spec/spec-core.js b/test/spec/spec-core.js
@@ -302,4 +302,100 @@ describe('Chartist core', function() {
     });
 
   });
+
+  describe('getBounds', function() {
+
+    it('should return 10 steps', function() {
+      var bounds = Chartist.getBounds(100, { high: 10, low: 1 }, 10, false);
+      expect(bounds.min).toBe(1);
+      expect(bounds.max).toBe(10);
+      expect(bounds.values).toEqual([1, 2, 3, 4, 5, 6, 7, 8, 9, 10]);
+    });  
+
+    it('should return 5 steps', function() {
+      var bounds = Chartist.getBounds(100, { high: 10, low: 1 }, 20, false);
+      expect(bounds.min).toBe(1);
+      expect(bounds.max).toBe(10);
+      expect(bounds.values).toEqual([1, 3, 5, 7, 9]);
+      // Is this correct behaviour? Should it include 10?
+    });  
+
+    it('should return non integer steps', function() {
+      var bounds = Chartist.getBounds(100, { high: 2, low: 1 }, 20, false);
+      expect(bounds.min).toBe(1);
+      expect(bounds.max).toBe(2);
+      expect(bounds.values).toEqual([ 1, 1.25, 1.5, 1.75, 2 ]);
+    });
+
+    it('should return integer steps only', function() {
+      var bounds = Chartist.getBounds(100, { high: 3, low: 1 }, 20, true);
+      expect(bounds.min).toBe(1);
+      expect(bounds.max).toBe(3);
+      expect(bounds.values).toEqual([ 1, 2, 3 ]);
+    });
+
+    it('should return single integer step', function() {
+      var bounds = Chartist.getBounds(100, { high: 2, low: 1 }, 20, true);
+      expect(bounds.min).toBe(1);
+      expect(bounds.max).toBe(2);
+      expect(bounds.values).toEqual([ 1, 2,]);
+    });
+
+    it('should floor/ceil min/max', function() {
+      var bounds = Chartist.getBounds(100, { high: 9.9, low: 1.01 }, 20, false);
+      expect(bounds.min).toBe(1);
+      expect(bounds.max).toBe(10);
+      expect(bounds.values).toEqual([1, 3, 5, 7, 9]);
+      // Is this correct behaviour? Should it include 10?
+    });
+
+    it('should floor/ceil min/max for non integers', function() {
+      var bounds = Chartist.getBounds(100, { high: 2.9, low: 1.01 }, 20, false);
+      expect(bounds.min).toBe(1);
+      expect(bounds.max).toBe(3);
+      expect(bounds.values).toEqual([1, 1.5, 2, 2.5, 3]);
+    });
+
+    it('should floor/ceil min/max if integers only', function() {
+      var bounds = Chartist.getBounds(100, { high: 2.9, low: 1.01 }, 20, true);
+      expect(bounds.min).toBe(1);
+      expect(bounds.max).toBe(3);
+      expect(bounds.values).toEqual([1, 2, 3]);
+    });
+
+    it('should return neg and pos values', function() {
+      var bounds = Chartist.getBounds(100, { high: 1.9, low: -0.9 }, 20, false);
+      expect(bounds.min).toBe(-1);
+      expect(bounds.max).toBe(2);
+      expect(bounds.values).toEqual([-1, 0, 1, 2]);
+    });
+
+    it('should return two steps if no space', function() {
+      var bounds = Chartist.getBounds(100, { high: 5, low: 0 }, 45, false);
+      expect(bounds.min).toBe(0);
+      expect(bounds.max).toBe(5);
+      expect(bounds.values).toEqual([0, 4]);
+      // Is this correct behaviour? Should it be [0, 5]?
+    });
+
+    it('should return single step if no space', function() {
+      var bounds = Chartist.getBounds(100, { high: 5, low: 0 }, 80, false);
+      expect(bounds.min).toBe(0);
+      expect(bounds.max).toBe(5);
+      expect(bounds.values).toEqual([0]);
+      // Is this correct behaviour? Should it be [0, 5]?
+    });
+
+    it('should return single step if range is less than epsilon', function() {
+      var bounds = Chartist.getBounds(100, { high: 1.0000000000000002, low: 1 }, 20, false);
+      expect(bounds.min).toBe(1);
+      expect(bounds.max).toBe(1.0000000000000002);
+      expect(bounds.low).toBe(1);
+      expect(bounds.high).toBe(1.0000000000000002);
+      expect(bounds.values).toEqual([1]);
+    });
+
+  });
+
+
 });