Shorthand the creation of reusable properties.

[tbranyen]

I've noticed in much of the documentation and examples the common definition of reusable properties, such as: width, height, tickFormat, etc. These functions are bulky and not particularly dry in my opinion.

The following proof of concept illustrates the concept:

d3.chart("SomeChart", {
  initialize: function(options) {
    // Configurable properties.
    this.prop("width");
    this.prop("height");

    // Configure dimensions.
    this.width(960);
    this.height(120);
  },

  // Create re-usable methods that act as getters/setters.
  prop: function(name, callback) {
    this[name] = function(val) {
      if (arguments.length === 0) {
        return this["_" + name];
      }

      // Reassign the value.
      this["_" + name] = val;

      if (callback) {
        callback.call(this, val);
      }

      return this;
    };
  }
});

Is this a common enough task to warrant a convenience method within d3.chart itself? I think it could easily cut down on a significant amount of common repeated methods.

[jeanlauliac]

+1, that'd remove a bunch of boilerplate code.

[iros]

This is definitely a common situation: needing to create getter/setter functions. The problem with making boilerplate ones is as follows:

1. Based on certain callbacks you may want to redraw your chart, making the line:
   if (chart.data) { chart.draw(chart.data); } a fairly common one. That doesn't always apply to all setters and requires that the data be stored on the chart as a property, something we've been reluctant to do.
2. In your example, you set the height and the width in the initialize function. This is not a common use case as most people will need to do this when initializing the chart like so:

var chart = d3.select("svg")
  .chart.("SomeChart")
  .height(100)
  .width(100);

What this means is that any internal changes that need to happen based on these settings will either need to be passed in by the user. As a result, I wouldn't recommend the callback approach, but rather broadcast an event like:

this.trigger("change:" + prop);

which would allow subscribing to it in a chart's initialize method and possibly on an initialized chart as well.

1. Assuming that one always wants to set the value, isn't always true. For example, in a recent chart I have a margins object that has 4 properties: top,left,right,bottom. In practice, in my setter one can pass only the changed attributes and the rest will remain:

 margins: function(newMargins) {
      var chart = this;

      if (arguments.length === 0) {
        return chart.state.margins;
      }

      // keep existing attributes, but overwrite the ones that were passed in.
      chart.state.margins = _.extend(chart.state.margins, newMargins);

      chart.trigger("change:margin", chart.state.margins);

      if (chart.data) { chart.draw(chart.data); }

      return this;
    },

In this example, I'm actually changing the colors of a chart by taking in an array of colors but then updating a scale!

colors: function(val){
      if(arguments.length > 0){
        this.state.colors = d3.scale.ordinal()
          .range(val);
        this.trigger("change:colors", this.state.colors);
      } else {
        return this.state.colors;
      }

      if (chart.data) { chart.draw(chart.data); }

      return this;
    },

Most importantly, I'll let @jugglinmike chime in on this.

[krishantaylor]

Sanitizing values is another thing to consider. Some of the charting logic might only work if certain properties are definitely functions, numbers, etc. That could be handled by passing a function to the proposed prop helper.

d3.chart("SomeChart", {
  initialize: function(options) {
    // Configurable properties.
    this.prop("width", null, true);
    this.prop("height", null, true);
  },

  // Create re-usable methods that act as getters/setters.
  prop: function(name, sanitize, redraw) {
    var chart = this;

    // default to the identity function if no sanitization function is provided
    sanitize = sanitize || function(_) { return _; };

    // rebind the sanitization function so it has access to the chart
    sanitize = sanitize.bind(chart);

    this[name] = function(val) {
      if (arguments.length === 0) {
        return chart["_" + name];
      }

      // Reassign the value.
      chart["_" + name] = sanitize(val);

      chart.trigger("change:" + name, chart["_" + name]);

      if (redraw && chart.data) { chart.draw(chart.data); }

      return chart;
    };
  }
});

This same sanitization function could also be used to implement the custom logic from the margins and colors examples.

this.prop("margins", function(margins) { return _.extend(this.state.margins, margins); }, true);
this.prop("colors", function(val) { return d3.scale.ordinal().range(val); }, true);

[jugglinmike]

@tbranyen Thanks for bringing this up. Irene and I have discussed this a bit,
but it's good to document our ideas publically and get more feedback.

@iros I agree about using events rather than a callback. One minor thing:
prefixing the name with change: is a bit misleading. We can't reliably
determine if the value has really changed. For example:

chart.margins({ top: 23 }); // triggers `change:margins`
chart.margins({ top: 23 }); // triggers `change:margins`

set: might be better?

@krishantaylor Regarding the redraw parameter: Irene's examples omitted
details regarding data caching (her charts store the most recent value passed
to the draw method in an instance property named data). d3.chart does not
implement this behavior, and I'm not convinced that it should. That's okay,
though, because you could move that to an event handler:

  initialize: function() {
    this.on("change:colors", function() {
      if (this.data) {
        this.draw(this.data);
      }
    });
  }

..or better yet:

  redraw: function() {
    if (this.data) {
      this.draw(this.data);
    }
  },
  initialize: function() {
    this.on("change:colors", this.redraw, this);
  }

Regarding the sanitize parameter: I think this is a bit too general to
warrant a place in the API.

This same sanitization function could also be used to implement the custom
logic from the margins and colors examples.

"sanitize" has a specific meaning, but in that implementation, it is more of a
catch-all for anything a chart may wish to do in response to a changing value.
(It reminds me of Chart#transform which is one of the weaker parts of the
API.) If we simply recommend that this type of functionality be implemented via
event handlers (which are broadcast in any case), then the API becomes more
consistent, and as a result, the charts created with it become easier to read.

[krishantaylor]

@jugglinmike I like moving the redraw to the second option you posted. And you're right on the naming of my sanitize method/param. I was aiming for something more like a setter.

[ghost]

Iros, to me, having a draw call in a setter doesn't seem like the best idea. I suppose it can make things easier for less advanced users, but I always called update (draw) manually after setting something that would change thing immediately. One main reason for this is if I was setting more than 1 thing that, in your case, would call update/draw. Instead of multiple calls to update/draw, I can just call it once at the end.