Skip to content
Closed
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix the typo as required.
  • Loading branch information
wmzhong committed May 21, 2018
commit 996c6af9692d71a0b10ac50f0eeac25b105bc279
37 changes: 17 additions & 20 deletions src/core/core.controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -694,7 +694,7 @@ module.exports = function(Chart) {
if (!dataset.data) {
dataset.data = [];
}
// format the data if the data array is missing some point according to the labels
// Format the data if the data array is missing some point according to the labels
dataset.data = me.formatDataset(dataset.data);
var meta = dataset._meta[me.id];
if (!meta) {
Expand Down Expand Up @@ -939,19 +939,17 @@ module.exports = function(Chart) {
return changed;
},
/**
* format the data list if the data is missing some points in the datasets.
* Format the data list if the data is missing some points in the datasets.
* @private
* @param {array} dataArray array to format
* @return {array} the formated array
*/
formatDataset: function(dataArray) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private functions should be prefixed with _

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The prefixed is added.
Pls check.

var labels = this.chart.data.labels;
var tmp = dataArray.slice(0);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there's no reason to create tmp here as a copy of dataArray. the only actions taken on it are read-only. just use dataArray directly and remove tmp

var labelLen = labels.length;
var dataLen = dataArray.length;
var result = dataArray;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this is where you should make the defensive copy. don't modify the input to the function. instead do var result = dataArray.slice(0); // create copy of input

var match = this.dataInLabel(tmp, labels);
if (match && dataLen < labelLen) {
if (match && dataArray.length < labels.length) {
for (var i = 0; i < labels.length; i++) {
var label = labels[i];
result[i] = {x: label, y: this.getY(label, tmp)};
Expand All @@ -960,12 +958,12 @@ module.exports = function(Chart) {
return result;
},
/**
* return the y data of the label, if no this label, return null
* @private
* @param {string} label the label in the labels array
* @param {array} dataArray the data in the datasets.
* @return {number} the y data according to the label
*/
* Return the y data of the label, if no this label, return null
* @private
* @param {string} label the label in the labels array
* @param {array} dataArray the data in the datasets.
* @return {number} the y data according to the label
*/
getY: function(label, dataArray) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be put with the other functions (line 25-68) since it doesn't use any class members

var y = null;
for (var i = 0; i < dataArray.length; i++) {
Expand All @@ -977,24 +975,23 @@ module.exports = function(Chart) {
return y;
},
/**
* find if the data in datasets is existed in the labels
* @private
* @param {array} dataArray the data in the datasets
* @param {array} labelArray the label in the labels array
* @return {boolean} the match result, true is included
*/
* Find if the data in datasets exists in the labels
* @private
* @param {array} dataArray the data in the datasets
* @param {array} labelArray the label in the labels array
* @return {boolean} the match result, true is included
*/
dataInLabel: function(dataArray, labelArray) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can be put with the other functions (line 25-68) since it doesn't use any class members

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have put these two functions on the other place.

var result = false;
for (var i = 0; i < dataArray.length; i++) {
var item = dataArray[i];
if (item === null || item === undefined || item.x === undefined) {
break;
}
if (labelArray.indexOf(item.x) > -1) {
result = true;
return true
}
}
return result;
return false
},
});

Expand Down