From 69521477a1f77c8c149c166c5c6b77a93bb3a009 Mon Sep 17 00:00:00 2001 From: Simon Brunel Date: Mon, 16 May 2016 23:31:47 +0200 Subject: [PATCH] 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 | 132 ++++++++++++++++++--------------------- 2 files changed, 63 insertions(+), 71 deletions(-) diff --git a/src/core/core.element.js b/src/core/core.element.js index e4b09d969..fd495b08a 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 5d4094713..ab3700270 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); + 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]; + } 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) { + 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 (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; } });