From 5daa462d93e850a64a7f6644afb8122336ccf84d Mon Sep 17 00:00:00 2001 From: Robert Konigsberg Date: Sat, 24 Nov 2012 05:38:46 -0500 Subject: [PATCH] Options code review, plus, remove original attr_ code. --- dygraph-options.js | 69 +++++++++++++++++++++++++++++++++++++++++------------- dygraph.js | 40 +------------------------------ 2 files changed, 54 insertions(+), 55 deletions(-) diff --git a/dygraph-options.js b/dygraph-options.js index 4864de9..9a4f6bf 100644 --- a/dygraph-options.js +++ b/dygraph-options.js @@ -11,7 +11,7 @@ * dygraph_ - the graph. * global - global attributes (common among all graphs, AIUI) * user - attributes set by the user - * axes + * axes - map of options specific to the axis. * series - { seriesName -> { idx, yAxis, options } * labels - used as mapping from index to series name. */ @@ -19,6 +19,10 @@ /** * This parses attributes into an object that can be easily queried. * + * It doesn't necessarily mean that all options are available, specifically + * if labels are not yet available, since those drive details of the per-series + * and per-axis options. + * * @param {Dyraph} dygraph The chart to which these options belong. * @constructor */ @@ -27,23 +31,29 @@ var DygraphOptions = function(dygraph) { this.axes_ = []; this.series_ = {}; - // Once these two objects are initialized, you can call find(); + // Once these two objects are initialized, you can call get(); this.global_ = this.dygraph_.attrs_; this.user_ = this.dygraph_.user_attrs_ || {}; - this.highlightSeries_ = this.find("highlightSeriesOpts") || {}; + this.highlightSeries_ = this.get("highlightSeriesOpts") || {}; // Get a list of series names. - var labels = this.find("labels"); + var labels = this.get("labels"); if (!labels) { return; // -- can't do more for now, will parse after getting the labels. - }; + } this.reparseSeries(); -} +}; +/** + * Reparses options that are all related to series. This typically occurs when + * options are either updated, or source data has been made avaialble. + * + * TODO(konigsberg): The method name is kind of weak; fix. + */ DygraphOptions.prototype.reparseSeries = function() { - this.labels = this.find("labels").slice(1); + this.labels = this.get("labels").slice(1); this.axes_ = [ {} ]; // Always one axis at least. this.series_ = {}; @@ -97,7 +107,12 @@ DygraphOptions.prototype.reparseSeries = function() { } }; -DygraphOptions.prototype.find = function(name) { +/** + * Get a global value. + * + * @param {String} name the name of the option. + */ +DygraphOptions.prototype.get = function(name) { if (this.user_.hasOwnProperty(name)) { return this.user_[name]; } @@ -105,19 +120,41 @@ DygraphOptions.prototype.find = function(name) { return this.global_[name]; } return null; -} +}; -DygraphOptions.prototype.findForAxis = function(name, axis) { - var axisIdx = (axis == "y2" || axis == "y2" || axis == 1) ? 1 : 0; +/** + * Get a value for a specific axis. If there is no specific value for the axis, + * the global value is returned. + * + * @param {String} name the name of the option. + * @param {String|number} axis the axis to search. Can be the string representation + * ("y", "y2") or the axis number (0, 1). + */ +DygraphOptions.prototype.getForAxis = function(name, axis) { + var axisIdx = 0; + if (typeof(axis) == 'number') { + axisIdx = axis; + } else { + // TODO(konigsberg): Accept only valid axis strings? + axisIdx = (axis == "y2") ? 1 : 0; + } var axisOptions = this.axes_[axisIdx]; if (axisOptions.hasOwnProperty(name)) { return axisOptions[name]; } - return this.find(name); -} + return this.get(name); +}; -DygraphOptions.prototype.findForSeries = function(name, series) { +/** + * Get a value for a specific series. If there is no specific value for the series, + * the value for the axis is returned (and afterwards, the global value.) + * + * @param {String} name the name of the option. + * @param {String|number} series the series to search. Can be the string representation + * or 0-offset series number. + */ +DygraphOptions.prototype.getForSeries = function(name, series) { // Honors indexes as series. var seriesName = (typeof(series) == "number") ? this.labels[series] : series; @@ -137,6 +174,6 @@ DygraphOptions.prototype.findForSeries = function(name, series) { return seriesOptions[name]; } - return this.findForAxis(name, seriesObj["yAxis"]); -} + return this.getForAxis(name, seriesObj["yAxis"]); +}; diff --git a/dygraph.js b/dygraph.js index 42e87c8..3783907 100644 --- a/dygraph.js +++ b/dygraph.js @@ -574,45 +574,7 @@ Dygraph.prototype.attr_ = function(name, seriesName) { } // - // Building an array which we peruse in backwards order to find the correct value. - // Options are checked in this order: - // series, axis, user attrs, global attrs. - // TODO(konigsberg): Can this be made faster by starting with the series and working outward, - // rather than building an array? - - var sources = []; - sources.push(this.attrs_); - if (this.user_attrs_) { - sources.push(this.user_attrs_); - if (seriesName) { - if (this.user_attrs_.hasOwnProperty(seriesName)) { - sources.push(this.user_attrs_[seriesName]); - } - - // TODO(konigsberg): This special case ought to be documented. - if (seriesName === this.highlightSet_ && - this.user_attrs_.hasOwnProperty('highlightSeriesOpts')) { - sources.push(this.user_attrs_.highlightSeriesOpts); - } - } - } - - var ret = null; - for (var i = sources.length - 1; i >= 0; --i) { - var source = sources[i]; - if (source.hasOwnProperty(name)) { - ret = source[name]; - break; - } - } - - var computedValue = seriesName ? this.attributes_.findForSeries(name, seriesName) : this.attributes_.find(name); - if (ret !== computedValue) { - console.log("Mismatch", name, seriesName, ret, computedValue); - } - - var USE_NEW_VALUE = true; - return USE_NEW_VALUE ? computedValue : ret; + return seriesName ? this.attributes_.getForSeries(name, seriesName) : this.attributes_.get(name); }; /** -- 2.7.4