d3 / d3-shape

Graphical primitives for visualization, such as lines and areas.
https://d3js.org/d3-shape
ISC License
2.48k stars 308 forks source link

Expose ascending -> sum #86

Closed ericsoco closed 7 years ago

ericsoco commented 7 years ago

When writing a custom stack order, it can be useful to have access to the sum() function exported from ascending.js.

Any reason not to expose it in d3-shape::index.js?

mbostock commented 7 years ago

I ask the opposite question, which is: is the utility of the sum function worth the cost to expose it? Every addition to the public API has a cost: writing the documentation, asking readers to read (or skip) said documentation, and committing to a stable API (that requires a major version bump to change).

By default, I prefer to keep as much as I can private. The sum function doesn’t seem useful enough to me to merit a new public API, especially when there’s already d3.sum which can be used as d3.sum(series, d => d[1]).

mbostock commented 7 years ago

Also, if you don’t want to use d3.sum, you can just copy the implementation of sum into your custom stack order. It’s pretty small, so a little duplication doesn’t seem harmful.

ericsoco commented 7 years ago

Didn't think about using d3.sum here, that fits the bill nicely. I was mostly trying to modify stackOrderInsideOut, and so needed ascending.sum -- indeed, I just copied it into my code.

Your reasons make plenty of sense, thanks for taking the time to explain them! Cheers.