From 69521477a1f77c8c149c166c5c6b77a93bb3a009 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Mon, 16 May 2016 23:31:47 +0200 Subject: [PATCH 1/2] Remove useless hasOwnProperty checks The Chart.helpers.each method uses Object.keys() to iterates on the object *own enumerable properties*, meaning that checking if object.hasOwnProperty() is useless. --- src/core/core.element.js | 2 +- src/core/core.helpers.js | 138 ++++++++++++++++++--------------------- 2 files changed, 66 insertions(+), 74 deletions(-) diff --git a/src/core/core.element.js b/src/core/core.element.js index e4b09d9695d..fd495b08a7f 100644 --- a/src/core/core.element.js +++ b/src/core/core.element.js @@ -39,7 +39,7 @@ module.exports = function(Chart) { helpers.each(this._model, function(value, key) { - if (key[0] === '_' || !this._model.hasOwnProperty(key)) { + if (key[0] === '_') { // Only non-underscored properties } diff --git a/src/core/core.helpers.js b/src/core/core.helpers.js index 5d40947133f..ab370027030 100644 --- a/src/core/core.helpers.js +++ b/src/core/core.helpers.js @@ -35,14 +35,12 @@ module.exports = function(Chart) { helpers.clone = function(obj) { var objClone = {}; helpers.each(obj, function(value, key) { - if (obj.hasOwnProperty(key)) { - if (helpers.isArray(value)) { - objClone[key] = value.slice(0); - } else if (typeof value === 'object' && value !== null) { - objClone[key] = helpers.clone(value); - } else { - objClone[key] = value; - } + if (helpers.isArray(value)) { + objClone[key] = value.slice(0); + } else if (typeof value === 'object' && value !== null) { + objClone[key] = helpers.clone(value); + } else { + objClone[key] = value; } }); return objClone; @@ -55,9 +53,7 @@ module.exports = function(Chart) { } helpers.each(additionalArgs, function(extensionObject) { helpers.each(extensionObject, function(value, key) { - if (extensionObject.hasOwnProperty(key)) { - base[key] = value; - } + base[key] = value; }); }); return base; @@ -67,42 +63,40 @@ module.exports = function(Chart) { var base = helpers.clone(_base); helpers.each(Array.prototype.slice.call(arguments, 1), function(extension) { helpers.each(extension, function(value, key) { - if (extension.hasOwnProperty(key)) { - if (key === 'scales') { - // Scale config merging is complex. Add out own function here for that - base[key] = helpers.scaleMerge(base.hasOwnProperty(key) ? base[key] : {}, value); - - } else if (key === 'scale') { - // Used in polar area & radar charts since there is only one scale - base[key] = helpers.configMerge(base.hasOwnProperty(key) ? base[key] : {}, Chart.scaleService.getScaleDefaults(value.type), value); - } else if (base.hasOwnProperty(key) && helpers.isArray(base[key]) && helpers.isArray(value)) { - // In this case we have an array of objects replacing another array. Rather than doing a strict replace, - // merge. This allows easy scale option merging - var baseArray = base[key]; - - helpers.each(value, function(valueObj, index) { - - if (index < baseArray.length) { - if (typeof baseArray[index] === 'object' && baseArray[index] !== null && typeof valueObj === 'object' && valueObj !== null) { - // Two objects are coming together. Do a merge of them. - baseArray[index] = helpers.configMerge(baseArray[index], valueObj); - } else { - // Just overwrite in this case since there is nothing to merge - baseArray[index] = valueObj; - } + if (key === 'scales') { + // Scale config merging is complex. Add out own function here for that + base[key] = helpers.scaleMerge(base.hasOwnProperty(key) ? base[key] : {}, value); + + } else if (key === 'scale') { + // Used in polar area & radar charts since there is only one scale + base[key] = helpers.configMerge(base.hasOwnProperty(key) ? base[key] : {}, Chart.scaleService.getScaleDefaults(value.type), value); + } else if (base.hasOwnProperty(key) && helpers.isArray(base[key]) && helpers.isArray(value)) { + // In this case we have an array of objects replacing another array. Rather than doing a strict replace, + // merge. This allows easy scale option merging + var baseArray = base[key]; + + helpers.each(value, function(valueObj, index) { + + if (index < baseArray.length) { + if (typeof baseArray[index] === 'object' && baseArray[index] !== null && typeof valueObj === 'object' && valueObj !== null) { + // Two objects are coming together. Do a merge of them. + baseArray[index] = helpers.configMerge(baseArray[index], valueObj); } else { - baseArray.push(valueObj); // nothing to merge + // Just overwrite in this case since there is nothing to merge + baseArray[index] = valueObj; } - }); + } else { + baseArray.push(valueObj); // nothing to merge + } + }); - } else if (base.hasOwnProperty(key) && typeof base[key] === "object" && base[key] !== null && typeof value === "object") { - // If we are overwriting an object with an object, do a merge of the properties. - base[key] = helpers.configMerge(base[key], value); + } else if (base.hasOwnProperty(key) && typeof base[key] === "object" && base[key] !== null && typeof value === "object") { + // If we are overwriting an object with an object, do a merge of the properties. + base[key] = helpers.configMerge(base[key], value); - } else { - // can just overwrite the value in this case - base[key] = value; - } + } else { + // can just overwrite the value in this case + base[key] = value; } }); }); @@ -131,38 +125,36 @@ module.exports = function(Chart) { var base = helpers.clone(_base); helpers.each(extension, function(value, key) { - if (extension.hasOwnProperty(key)) { - if (key === 'xAxes' || key === 'yAxes') { - // These properties are arrays of items - if (base.hasOwnProperty(key)) { - helpers.each(value, function(valueObj, index) { - var axisType = helpers.getValueOrDefault(valueObj.type, key === 'xAxes' ? 'category' : 'linear'); - var axisDefaults = Chart.scaleService.getScaleDefaults(axisType); - if (index >= base[key].length || !base[key][index].type) { - base[key].push(helpers.configMerge(axisDefaults, valueObj)); - } else if (valueObj.type && valueObj.type !== base[key][index].type) { - // Type changed. Bring in the new defaults before we bring in valueObj so that valueObj can override the correct scale defaults - base[key][index] = helpers.configMerge(base[key][index], axisDefaults, valueObj); - } else { - // Type is the same - base[key][index] = helpers.configMerge(base[key][index], valueObj); - } - }); - } else { - base[key] = []; - helpers.each(value, function(valueObj) { - var axisType = helpers.getValueOrDefault(valueObj.type, key === 'xAxes' ? 'category' : 'linear'); - base[key].push(helpers.configMerge(Chart.scaleService.getScaleDefaults(axisType), valueObj)); - }); - } - } else if (base.hasOwnProperty(key) && typeof base[key] === "object" && base[key] !== null && typeof value === "object") { - // If we are overwriting an object with an object, do a merge of the properties. - base[key] = helpers.configMerge(base[key], value); - + if (key === 'xAxes' || key === 'yAxes') { + // These properties are arrays of items + if (base.hasOwnProperty(key)) { + helpers.each(value, function(valueObj, index) { + var axisType = helpers.getValueOrDefault(valueObj.type, key === 'xAxes' ? 'category' : 'linear'); + var axisDefaults = Chart.scaleService.getScaleDefaults(axisType); + if (index >= base[key].length || !base[key][index].type) { + base[key].push(helpers.configMerge(axisDefaults, valueObj)); + } else if (valueObj.type && valueObj.type !== base[key][index].type) { + // Type changed. Bring in the new defaults before we bring in valueObj so that valueObj can override the correct scale defaults + base[key][index] = helpers.configMerge(base[key][index], axisDefaults, valueObj); + } else { + // Type is the same + base[key][index] = helpers.configMerge(base[key][index], valueObj); + } + }); } else { - // can just overwrite the value in this case - base[key] = value; + base[key] = []; + helpers.each(value, function(valueObj) { + var axisType = helpers.getValueOrDefault(valueObj.type, key === 'xAxes' ? 'category' : 'linear'); + base[key].push(helpers.configMerge(Chart.scaleService.getScaleDefaults(axisType), valueObj)); + }); } + } else if (base.hasOwnProperty(key) && typeof base[key] === "object" && base[key] !== null && typeof value === "object") { + // If we are overwriting an object with an object, do a merge of the properties. + base[key] = helpers.configMerge(base[key], value); + + } else { + // can just overwrite the value in this case + base[key] = value; } }); From 93c28a4d5f18ffe11f524afbb9e8e7aeadc91766 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Tue, 17 May 2016 13:32:40 +0200 Subject: [PATCH 2/2] Optimize element.point and controller.line Change some helpers.each() to `for` loops when iterating on a potentially large number of items and use more local variables when appropriate (making the minified build a bit smaller). --- src/controllers/controller.line.js | 234 ++++++++++++++++------------- src/elements/element.point.js | 193 ++++++++++++------------ 2 files changed, 225 insertions(+), 202 deletions(-) diff --git a/src/controllers/controller.line.js b/src/controllers/controller.line.js index 58349034c9b..0807600688a 100644 --- a/src/controllers/controller.line.js +++ b/src/controllers/controller.line.js @@ -23,50 +23,58 @@ module.exports = function(Chart) { } }; - Chart.controllers.line = Chart.DatasetController.extend({ addElements: function() { - var meta = this.getMeta(); + var me = this; + var meta = me.getMeta(); + var data = me.getDataset().data || []; + var value, i, ilen; + meta.dataset = meta.dataset || new Chart.elements.Line({ - _chart: this.chart.chart, - _datasetIndex: this.index, + _chart: me.chart.chart, + _datasetIndex: me.index, _points: meta.data }); - helpers.each(this.getDataset().data, function(value, index) { - meta.data[index] = meta.data[index] || new Chart.elements.Point({ - _chart: this.chart.chart, - _datasetIndex: this.index, - _index: index + for (i=0, ilen=data.length; i 0) { - - ctx.strokeStyle = vm.borderColor || defaultColor; - ctx.lineWidth = helpers.getValueOrDefault(vm.borderWidth, globalOpts.elements.point.borderWidth); - - ctx.fillStyle = vm.backgroundColor || defaultColor; - - var radius = vm.radius; - - var xOffset, - yOffset; + if (isNaN(radius) || radius <= 0) { + return; + } - switch (pointStyle) { - // Default includes circle - default: - ctx.beginPath(); - ctx.arc(x, y, radius, 0, Math.PI * 2); - ctx.closePath(); - ctx.fill(); - break; - case 'triangle': - ctx.beginPath(); - var edgeLength = 3 * radius / Math.sqrt(3); - var height = edgeLength * Math.sqrt(3) / 2; - ctx.moveTo(x - edgeLength / 2, y + height / 3); - ctx.lineTo(x + edgeLength / 2, y + height / 3); - ctx.lineTo(x, y - 2 * height / 3); - ctx.closePath(); - ctx.fill(); - break; - case 'rect': - ctx.fillRect(x - 1 / Math.SQRT2 * radius, y - 1 / Math.SQRT2 * radius, 2 / Math.SQRT2 * radius, 2 / Math.SQRT2 * radius); - ctx.strokeRect(x - 1 / Math.SQRT2 * radius, y - 1 / Math.SQRT2 * radius, 2 / Math.SQRT2 * radius, 2 / Math.SQRT2 * radius); - break; - case 'rectRot': - ctx.translate(x, y); - ctx.rotate(Math.PI / 4); - ctx.fillRect(-1 / Math.SQRT2 * radius, -1 / Math.SQRT2 * radius, 2 / Math.SQRT2 * radius, 2 / Math.SQRT2 * radius); - ctx.strokeRect(-1 / Math.SQRT2 * radius, -1 / Math.SQRT2 * radius, 2 / Math.SQRT2 * radius, 2 / Math.SQRT2 * radius); - ctx.setTransform(1, 0, 0, 1, 0, 0); - break; - case 'cross': - ctx.beginPath(); - ctx.moveTo(x, y + radius); - ctx.lineTo(x, y - radius); - ctx.moveTo(x - radius, y); - ctx.lineTo(x + radius, y); - ctx.closePath(); - break; - case 'crossRot': - ctx.beginPath(); - xOffset = Math.cos(Math.PI / 4) * radius; - yOffset = Math.sin(Math.PI / 4) * radius; - ctx.moveTo(x - xOffset, y - yOffset); - ctx.lineTo(x + xOffset, y + yOffset); - ctx.moveTo(x - xOffset, y + yOffset); - ctx.lineTo(x + xOffset, y - yOffset); - ctx.closePath(); - break; - case 'star': - ctx.beginPath(); - ctx.moveTo(x, y + radius); - ctx.lineTo(x, y - radius); - ctx.moveTo(x - radius, y); - ctx.lineTo(x + radius, y); - xOffset = Math.cos(Math.PI / 4) * radius; - yOffset = Math.sin(Math.PI / 4) * radius; - ctx.moveTo(x - xOffset, y - yOffset); - ctx.lineTo(x + xOffset, y + yOffset); - ctx.moveTo(x - xOffset, y + yOffset); - ctx.lineTo(x + xOffset, y - yOffset); - ctx.closePath(); - break; - case 'line': - ctx.beginPath(); - ctx.moveTo(x - radius, y); - ctx.lineTo(x + radius, y); - ctx.closePath(); - break; - case 'dash': - ctx.beginPath(); - ctx.moveTo(x, y); - ctx.lineTo(x + radius, y); - ctx.closePath(); - break; - } + ctx.strokeStyle = vm.borderColor || defaultColor; + ctx.lineWidth = helpers.getValueOrDefault(vm.borderWidth, globalOpts.elements.point.borderWidth); + ctx.fillStyle = vm.backgroundColor || defaultColor; - ctx.stroke(); + switch (pointStyle) { + // Default includes circle + default: + ctx.beginPath(); + ctx.arc(x, y, radius, 0, Math.PI * 2); + ctx.closePath(); + ctx.fill(); + break; + case 'triangle': + ctx.beginPath(); + edgeLength = 3 * radius / Math.sqrt(3); + height = edgeLength * Math.sqrt(3) / 2; + ctx.moveTo(x - edgeLength / 2, y + height / 3); + ctx.lineTo(x + edgeLength / 2, y + height / 3); + ctx.lineTo(x, y - 2 * height / 3); + ctx.closePath(); + ctx.fill(); + break; + case 'rect': + size = 1 / Math.SQRT2 * radius; + ctx.fillRect(x - size, y - size, 2 * size, 2 * size); + ctx.strokeRect(x - size, y - size, 2 * size, 2 * size); + break; + case 'rectRot': + ctx.translate(x, y); + ctx.rotate(Math.PI / 4); + size = 1 / Math.SQRT2 * radius; + ctx.fillRect(-size, -size, 2 * size, 2 * size); + ctx.strokeRect(-size, -size, 2 * size, 2 * size); + ctx.setTransform(1, 0, 0, 1, 0, 0); + break; + case 'cross': + ctx.beginPath(); + ctx.moveTo(x, y + radius); + ctx.lineTo(x, y - radius); + ctx.moveTo(x - radius, y); + ctx.lineTo(x + radius, y); + ctx.closePath(); + break; + case 'crossRot': + ctx.beginPath(); + xOffset = Math.cos(Math.PI / 4) * radius; + yOffset = Math.sin(Math.PI / 4) * radius; + ctx.moveTo(x - xOffset, y - yOffset); + ctx.lineTo(x + xOffset, y + yOffset); + ctx.moveTo(x - xOffset, y + yOffset); + ctx.lineTo(x + xOffset, y - yOffset); + ctx.closePath(); + break; + case 'star': + ctx.beginPath(); + ctx.moveTo(x, y + radius); + ctx.lineTo(x, y - radius); + ctx.moveTo(x - radius, y); + ctx.lineTo(x + radius, y); + xOffset = Math.cos(Math.PI / 4) * radius; + yOffset = Math.sin(Math.PI / 4) * radius; + ctx.moveTo(x - xOffset, y - yOffset); + ctx.lineTo(x + xOffset, y + yOffset); + ctx.moveTo(x - xOffset, y + yOffset); + ctx.lineTo(x + xOffset, y - yOffset); + ctx.closePath(); + break; + case 'line': + ctx.beginPath(); + ctx.moveTo(x - radius, y); + ctx.lineTo(x + radius, y); + ctx.closePath(); + break; + case 'dash': + ctx.beginPath(); + ctx.moveTo(x, y); + ctx.lineTo(x + radius, y); + ctx.closePath(); + break; } + + ctx.stroke(); } }); -}; \ No newline at end of file +};