Skip to content

Uniform access #98

@samselikoff

Description

@samselikoff

The problem

I'm running into some issues with uniform access, and am wondering if there's a place on d3.chart's main chart object for get and set methods. This is more a discussion than a question.

Let's say I have a core-line-chart, and by default it sets an internal property _duration equal to 300:

// core-line-chart.js
initialize: function(options) {
  this._duration = 300;
}

Throughout its code it can access this property using this._duration. Now let's say we want this property to be configurable from the outside. We add a getter/setter:

  // core-line-chart.js
  initialize: function(options) {
    this._duration = 300;
  }
+ ...
+ duration: function(_) {
+   if (arguments.length === 0) {return this._duration;}
+
+   this._duration = _;
+
+   return this;
+  }

Now outside users can set .duration(500), and all the code in the chart that uses this._duration will work as expected.

Now, say that we also want other d3.charts that attach our core line chart to be able to modify its duration. There are two approaches. First, they could just use the core line chart's existing public interface:

// some-other-chart.js
initialize: function() {
  ...
  this.lineChart = this.base.append('g')
    .chart('core-line-chart')
    .duration(500);
  this.attach('line-chart', this.lineChart);
}

Alternatively, we could alter our core line chart to override its defaults on initialization:

  // core-line-chart.js
  initialize: function(options) {
-   this._duration = 300;
+   this._duration = options.duration || 300;
  }

Now the parent (attacher) chart can specify an optional duration when it attaches the line chart.

All this is fine, and everything still works as expected. The tricky part is when property values need to be determined at run time.

Let's say the parent chart's duration property is dynamic, and can itself be changed by a getter/setter. Now, even though it may change later, the core-line-chart's duration property is set as soon as it's attached, so it may fall out of sync. One solution is for the parent chart to, in its getter/setter, update the attached's charts property:

  // some-other-chart.js
  initialize: function() {
    ...
    this.lineChart = this.base.append('g')
      .chart('core-line-chart')
-     .duration(500);
+     .duration(this._duration);
    this.attach('line-chart', this.lineChart);
  },

  ...

  duration: function(_) {
    if (arguments.length === 0) {return this._duration;}

    this._duration = _;
+   this.lineChart.duration(_);

    return this;
  }

But this method is cumbersome, can quickly become unwieldy with lots of dependent properties, and is even more difficult when the parent is a subclass and the accessor lives in the superclass. We could also use pub/sub, but this suffers from the same problems.

Our child (attached) chart could also call the parent's accessor method directly (this._parent.duration()), but this violates the Law of Demeter and makes our child chart less configurable (the child chart has now made assumptions about which duration to use).

Another approach is for our parent chart to simply pass in its accessor function to the child chart:

  // some-other-chart.js
  initialize: function() {
    ...
    this.lineChart = this.base.append('g')
      .chart('core-line-chart')
-     .duration(this._duration);
+     .duration(this.duration);
    this.attach('line-chart', this.lineChart);
  }

Then, we'd alter our core-line-chart a bit, so that it works whether duration is a static property or a function

// core-line-chart.js
duration: function(_) {
  if (arguments.length === 0) {
    return (typeof this._duration === 'function')
      ? this._duration()
      : this._duration;
  }

  this._duration = _;

  return this;
},

and change our property access throughout the code from this._duration to this.duration(). Now, whenever our core-line-chart needs the duration, it will return either the static value, or it will invoke the getter method on the parent chart to dynamically compute the value.

This is certainly a solution, though using this.duration() to get a static value of 300 seems to go against the grain. We could simply use this._property for values we know are static and this.property() for calculated values, but this violates uniform access and requires a heavier cognitive load when composing and modifying charts.


Discussion

More importantly though, every user of d3.chart will run into these problems at some point, and have to go through a similar process to come up with a solution that takes care of all these cases.

One way we could prevent users from having to answer these questions themselves would be to add a .get and .set method to the core chart class. The .get method would essentially do the same thing as the last code snippet above: inspect the internal property; if it's a function then invoke it; otherwise, return the value. (Some properties like scales are both functions and objects, so in those cases we'd want to return the object).

This would mean throughout our charts we would use

this.get('duration')
this.get('xScale')

and so on, and reliably get the desired property whether it's a static default, a static property from the parent, or a dynamic property from the parent.

The framework could also endorse one way for parent charts to configure child charts. I'm not entirely sure this would work, but perhaps the default could be that properties passed in the options object could automatically be set on the child chart instance, so the child could access them via .get without any code. We could also allow users to redefine set if they needed to do validation/other work.

I'm interested to hear others' thoughts and experiences. If there's a simple solution to all this that I'm missing, by all means please share. d3.chart (like d3) gives us a lot of flexibility to build our charts as we see fit; but if we are all solving these problems in only trivially different ways, perhaps the framework can help push us in the right - and same - direction.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions