Options code review, plus, remove original attr_ code.

diff --git a/dygraph-options.js b/dygraph-options.js
index 4864de9..9a4f6bf 100644 (file)
--- 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 (file)
--- a/dygraph.js
+++ b/dygraph.js
@@ -574,45 +574,7 @@ Dygraph.prototype.attr_ = function(name, seriesName) {
   }
 // </REMOVE_FOR_COMBINED>
 
-  // 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);
 };
 
 /**