Draw sine wave going around a circle

Question

I have used d3 to draw a sine wave going around a circle. This is the very first time I've used d3 or drawn a SVG and I'm fairly new to JS as well, so I don't know if I've overcomplicated it/if there is a simpler way to achieve this. I'd appreciate some feedback - especially if there's any way to make my code more concise.

See my codepen.

const svg = d3.select('svg');
const margin = { top: 50, right: 50, bottom: 50, left: 50 };
const width = +svg.attr('width') - margin.left - margin.right;
const height = +svg.attr('height') - margin.top - margin.bottom;

// content area of your visualization
const vis = svg
.append('g')
.attr('transform', `translate(${margin.left+width/2},${margin.top+height/2})`);

// show area inside of margins
const rect = vis
.append('rect')
.attr('class', 'content')
.attr('width', width)
.attr('height', height)
.attr('transform', `translate(${-width/2},${-height/2})`);

// show scales
const xScale = d3
.scaleLinear()
.domain([-100, 100])
.range([-width/2, width/2]);
const yScale = d3
.scaleLinear()
.domain([100, -100])
.range([-height/2, height/2]);
vis.append('g').call(d3.axisTop(xScale));
vis.append('g').call(d3.axisLeft(yScale));

// draw circle
const pi = Math.PI
const radius = 63.66
const circle = vis
.append('circle')
.style('stroke-dasharray', '3, 3')
.style('stroke', 'black')
.style("fill", "transparent")
.attr("r", xScale(radius))
.attr("cx", 0)
.attr("cy", 0)

// get coordinates for a sine wave
const getSineWave = ({ 
  numWaves, 
  wavelength,
  amplitude,
  phase,
  numPointsPerWave,
}) => {
  return (
    d3.range(numWaves*numPointsPerWave+1).map(function(k) {
      const x = k * wavelength/numPointsPerWave
      return [x, amplitude * Math.sin(phase + 2 * pi * x/wavelength)];
    })
  )
}

// tranform a coordinate from linear space to circular space
const rotate = (cx, cy, x, y, radius) => {
  const theta = x/radius,
        sin = Math.sin(theta),
        cos = Math.cos(theta),
        nx = cx + (radius + y) * sin,
        ny = cy + (radius + y) * cos
  return [nx, ny];
}

// generate sine wave
const numWaves = 4
const amplitude = 10
const phase = pi/2
const circumference = 2 * pi * radius
const wavelength = circumference / numWaves
const numPointsPerWave = 4
const sineWave = getSineWave({
  numWaves, 
  numPointsPerWave,
  wavelength,
  amplitude,
  phase,
  wavelength
})
var rotatedSine = sineWave.map( d => {
  const rotatedCoords = rotate(0, 0, d[0], d[1], radius)
  return rotatedCoords
})
// remove the last point as it would overlap the first point of the circle
rotatedSine.pop() 

// get Path commands for given coordinates
const getPath = d3.line()
.x(d => xScale(d[0]))
.y(d => yScale(d[1]))
.curve(d3.curveCardinalClosed) 

// draw sine wave going around a circle
const wave = vis
.append('path')
.attr('d', getPath(rotatedSine))
.attr('fill', 'none')
.attr('stroke', 'black')
.attr('stroke-width', '1px')

svg {
        background-color: steelblue;
    }
    
    .content {
        fill: lightsteelblue;
    }

<script src="https://d3js.org/d3.v4.js"></script>
<svg width="1000" height="1000" </ svg>

Answer 1

Overall you have a good D3 code here, congrats (I'm fairly impressed with the questions I've seen here at CR lately, from people claiming "This is the very first time I've used d3 or drawn a SVG").

However, before sharing my proposed alternative, I'd like to tell you that, unfortunately, you're using the wrong tool for the task!

As you can see in my answer here, the problem is that D3 is designed to create visualizations based on data, normally qualitative or discrete quantitative data sets. According to Mike Bostock, D3 creator:

D3 is designed primarily for data visualization, mostly empirical datasets rather than continuous functions, and so there is no built-in method for generating abscissa values. (emphasis mine)

As you can see in your case, the line gets better if you push more datapoint into the array, increasing any of the two constants in...

d3.range(numWaves*numPointsPerWave+1)

In your particular case we can get a good line with numPointsPerWave = 10, which is not a big problem... however, the advice remains: D3 is not the correct tool here, you should look for a proper plotting library. As you can see in the linked answer above, in some situations we have to increase the data points a lot to have a good looking graph.

D3 radial line

All that being said, here is my proposed alternative: instead of all that complicated math and 2 functions to set the path's d attribute, use a D3 radial line generator.

In this answer I'll focus only on the use of the radial line generator, nothing more. I'm sure that other users will soon post answers regarding your JavaScript code (use of functions, constants, destructuring, currying etc...)

According to the API, d3.lineRadial():

Constructs a new radial line generator with the default settings. A radial line generator is equivalent to the standard Cartesian line generator, except the x and y accessors are replaced with angle and radius accessors. Radial lines are always positioned relative to ⟨0,0⟩; use a transform (see: SVG, Canvas) to change the origin.

So, all we need is the line generator...

const radialGenerator = d3.lineRadial()
    .angle(d => d.angle)
    .radius(d => d.radius)
    .curve(d3.curveCardinalClosed);

And the adequate data:

const length = 100;
const amplitude = 20;
const radialScale = d3.scaleLinear()
  .domain([0, length])
  .range([0, Math.PI * 2]);

const data = d3.range(length).map(function(d) {
  return {
    angle: radialScale(d),
    radius: xScale(radius) + Math.sin(d) * amplitude
  }
});

Then, you append the path:

const wave = vis.append('path')
  .attr('d', radialGenerator(data))
  .attr('fill', 'none')
  .attr('stroke', 'black')
  .attr('stroke-width', '1px');

Have in mind that my function here is not as complex as yours, which accepts several different parameters: this answer is just to show you the existence of d3.lineRadial(), you can certainly improve it.

Here is the demo:

const svg = d3.select('svg');
const margin = {
  top: 50,
  right: 50,
  bottom: 50,
  left: 50
};
const width = +svg.attr('width') - margin.left - margin.right;
const height = +svg.attr('height') - margin.top - margin.bottom;

// content area of your visualization
const vis = svg.append('g')
  .attr('transform', `translate(${margin.left+width/2},${margin.top+height/2})`);

// show area inside of margins
const rect = vis.append('rect')
  .attr('class', 'content')
  .attr('width', width)
  .attr('height', height)
  .attr('transform', `translate(${-width/2},${-height/2})`);

// show scales
const xScale = d3.scaleLinear()
  .domain([-100, 100])
  .range([-width / 2, width / 2]);
const yScale = d3.scaleLinear()
  .domain([100, -100])
  .range([-height / 2, height / 2]);
vis.append('g').call(d3.axisTop(xScale));
vis.append('g').call(d3.axisLeft(yScale));

// draw circle
const pi = Math.PI
const radius = 63.66
const circle = vis.append('circle')
  .style('stroke-dasharray', '3, 3')
  .style('stroke', 'black')
  .style("fill", "transparent")
  .attr("r", xScale(radius))
  .attr("cx", 0)
  .attr("cy", 0);

const length = 100;
const amplitude = 20;

const radialGenerator = d3.lineRadial()
  .angle(d => d.angle)
  .radius(d => d.radius)
  .curve(d3.curveCardinalClosed)

const radialScale = d3.scaleLinear()
  .domain([0, length])
  .range([0, Math.PI * 2]);

const data = d3.range(length).map(function(d) {
  return {
    angle: radialScale(d),
    radius: xScale(radius) + Math.sin(d) * amplitude
  }
});

const wave = vis.append('path')
  .attr('d', radialGenerator(data))
  .attr('fill', 'none')
  .attr('stroke', 'black')
  .attr('stroke-width', '1px')

svg {
  background-color: steelblue;
}

.content {
  fill: lightsteelblue;
}

<script src="https://d3js.org/d3.v5.min.js"></script>
<svg width="1000" height="1000"></svg>

Answer 2

Dry out, and separate style, data and logic.

D3 seams to encourage a lot of repeated code with many lines of chained calls to .style and .attr and with that a tendency to mix data, style, and logic into one hard to follow and maintain source code mass.

With a little additional effort you can create helper functions to remove the repeating code and help separate data, style and logic.

With the helper function you can move all the optional style related information to a single encapsulated object.

The wavy line

The creation of the wave is rather long winded, first you create the points, then you transform them to fit the 360deg of the circle. And you have to pop the last value.

This can be simplified to one function that creates all the points transformed and scaled, reducing the complexity and thus attainability of the code. You don't need rotate, and the (hacky) need to pop the last point from the array.

Eg

    d3.range(numPoints - 1).map(k => {
        const u = k / numPoints * PI2 ;
        const r = radius + Math.sin(u * numWaves + phase) * amplitude;
        return [scaleX(Math.cos(u) * r), scaleY(Math.sin(u) * r)];
    });

Also the number of point should be related to the size of the circle. Creating a segment length option you can get the number of points needed to match a desired resolution.

Coercion

You don't need to coerce values if the expression does will do it anyways.

const height = +svg.attr('height') - margin.top - margin.bottom;

The + is not needed as - can only operate on numbers and will thus coerce the string to a number.

const height = svg.attr('height') - margin.top - margin.bottom;

Example.

The example code has a helper that sets object attributes, styles, or calls functions with supplied data to remove the need to repeat the long list of chained attr, style, and calls.

The object options has most of the data needed to style and display the graph.

A single function getSineWave creates the line.

The code to create and display the final result is at the bottom.

You can make many changes to the style and data without needing to navigate the code to find and replace one or more copies of the styling you are changing.

const d3Helper = (obj, props) => {
  if (props.call || props.attr || props.style) {
    const call = Object.entries(props.call || {});
    const style = Object.entries(props.style || {});
    const attr = Object.entries(props.attr || {});
    for (const [name, value] of call) {
      obj[name](value)
    }
    for (const [name, value] of style) {
      obj.style(name, typeof value === "function" ? value() : value);
    }
    for (const [name, value] of attr) {
      obj.attr(name, typeof value === "function" ? value() : value);
    }
  } else {
    for (const [name, value] of Object.entries(props)) {
      obj[name](value)
    }
  }
  return obj;
}

const svg = d3.select('svg');
const marg = { top: 50,right: 50,bottom: 50,left: 50};
const width = svg.attr('width') - marg.left - marg.right;
const height = svg.attr('height') - marg.top - marg.bottom;
const widthH = width / 2, heightH = height / 2;
const PI = Math.PI, PI2 = PI * 2;
const options = {
  scale: 100,
  radius: 63.66,
  numWaves: 4,
  amplitude: 10,
  phase: PI / 2,
  segmentLength: 2, // pixels per line segment (approx)
  color: "black",
  lineWidth: "2px",
  dash: "3, 3",
  get domain() {
    return [-options.scale, options.scale]
  },
};
Object.assign(options, { ...{
    numPoints: options.radius * PI2 / options.segmentLength | 0,
    g: {
      attr: {
        transform: `translate(${marg.left + widthH},${marg.top + heightH})`
      }
    },
    rect: {
      attr: {
        "class": "content",
        width: width,
        height: height,
        transform: `translate(${-widthH},${-heightH})`,
      }
    },
    circle: {
      style: {
        "stroke-dasharray": () => options.dash,
        stroke: () => options.color,
        fill: "transparent"
      },
      attr: {
        cx: 0,
        cy: 0,
        r: () => scaleX(options.radius)
      }
    },
    wave: {
      attr: {
        d: () => line(sineWave),
        fill: "none",
        stroke: () => options.color,
        "stroke-width": () => options.lineWidth,
      }
    },
    line: {
      x: d => d[0],
      y: d => d[1],
      curve: d3.curveCardinalClosed
    },
    scaleX: {
      domain: options.domain,
      range: [-widthH, widthH]
    },
    scaleY: {
      domain: options.domain,
      range: [-heightH, heightH]
    },
  }
});

const getSineWave = ({numWaves, amplitude, phase, radius, numPoints}) =>
  d3.range(numPoints - 1).map(i => {
    const a = i / numPoints * PI2;
    const r = radius + Math.sin(a * numWaves + phase) * amplitude;
    return [scaleX(Math.cos(a) * r), scaleY(Math.sin(a) * r)];
  });

const vis = d3Helper(svg.append('g'), options.g);
d3Helper(vis.append('rect'), options.rect);
const scaleX = d3Helper(d3.scaleLinear(), options.scaleX);
const scaleY = d3Helper(d3.scaleLinear(), options.scaleY);
vis.append('g').call(d3.axisTop(scaleX));
vis.append('g').call(d3.axisLeft(scaleY));
d3Helper(vis.append('circle'), options.circle);
const sineWave = getSineWave(options);
const line = d3Helper(d3.line(), options.line);
d3Helper(vis.append('path'), options.wave);

svg {
  background-color: steelblue;
}

.content {
  fill: lightsteelblue;
}

<script src="https://d3js.org/d3.v4.js"></script>
<svg width="1000" height="1000"> </ svg>

Answer 3

Javascript Feedback

Because the request is for "any way to make my code more concise" the feedback below focuses on simplifying the code while maintaining existing functionality.

Constants

It may be wise to put all of the configuration values at the top. If they are truly constant then some would say the names should be in all capital letters. There are various opinions on how true constants should be defined - for example, see answers to Defining constants in JavaScript, as well as style guides like Google JS S.G., AirBnB, etc.

rorated Sine-wave

The code to generate rotatedSine, declares a non-constant variable, and the arrow function declares a constant rotatedCoords which is immediately returned on the next line. Perhaps you prefer the readability of this - knowing exactly what is returned. And was var used because of the call to .pop() later on? If so, const could have been used since .pop() doesn't reassign the value - it merely mutates it.

var rotatedSine = sineWave.map( d => {
  const rotatedCoords = rotate(0, 0, d[0], d[1], radius)
  return rotatedCoords
})

That mapping function could be simplified - perhaps giving the mapping function a name like getRotatedCoords

const getRotatedCoords = d => rotate(0, 0, d[0], d[1], radius);

And then that function could be passed to the call to .map():

const rotatedSine = sineWave.map(getRotatedCoords);

But because that function isn't used anywhere else, there isn't much need for it to be named.

const rotatedSine = sineWave.map(d => rotate(0, 0, d[0], d[1], radius));

function getSineWave()

This function is only used once so unless you plan on using it more frequently, it could be eliminated by setting sineWave to the mapping of the range:

const sineWave = d3.range(numWaves*numPointsPerWave+1).map(function(k) {
  const x = k * wavelength/numPointsPerWave
  return [x, amplitude * Math.sin(phase + 2 * pi * x/wavelength)];
})

I also noticed that the original code calls getSineWave() with wavelength passed twice - while this doesn't lead to an error because object destructuring is used, it is superfluous.

function rotate simplification

In the function rorate, some of the variables are only used once after their assignment - e.g. sin, cos, nx, ny so that function could be condensed - see the example below. This does however impact readability, since the return statement grows to 82 characters, which many believe is too long for a single line.

const rotate = (cx, cy, x, y, radius) => {
  const theta = x/radius;
  return [cx + (radius + y) * Math.sin(theta), cy + (radius + y) * Math.cos(theta)];
} 

HTML Feedback

Un-terminated <svg>tag

Obviously modern browsers have been able to handle it - the opening <svg> tag is un-terminated:

<svg width="1000" height="1000" </ svg>

For proper HTML, add a closing angle bracket after the height attribute, and remove the space in the closing tag:

<svg width="1000" height="1000"> </svg>

Simplified code

The snippet below contains JS code that contains ~23% fewer lines.

//ACTUAL CONSTANTS
const pi = Math.PI
const radius = 63.66
const numWaves = 4
const amplitude = 10
const phase = pi/2
const circumference = 2 * pi * radius
const wavelength = circumference / numWaves
const numPointsPerWave = 4

const svg = d3.select('svg');
const margin = { top: 50, right: 50, bottom: 50, left: 50 };
const width = +svg.attr('width') - margin.left - margin.right;
const height = +svg.attr('height') - margin.top - margin.bottom;

// content area of your visualization
const vis = svg
.append('g')
.attr('transform', `translate(${margin.left+width/2},${margin.top+height/2})`);

// show area inside of margins
const rect = vis
.append('rect')
.attr('class', 'content')
.attr('width', width)
.attr('height', height)
.attr('transform', `translate(${-width/2},${-height/2})`);

// show scales
const xScale = d3
.scaleLinear()
.domain([-100, 100])
.range([-width/2, width/2]);
const yScale = d3
.scaleLinear()
.domain([100, -100])
.range([-height/2, height/2]);
vis.append('g').call(d3.axisTop(xScale));
vis.append('g').call(d3.axisLeft(yScale));

// draw circle
const circle = vis
.append('circle')
.style('stroke-dasharray', '3, 3')
.style('stroke', 'black')
.style("fill", "transparent")
.attr("r", xScale(radius))
.attr("cx", 0)
.attr("cy", 0)

// tranform a coordinate from linear space to circular space
const rotate = (cx, cy, x, y, radius) => {
  const theta = x/radius;
  return [cx + (radius + y) * Math.sin(theta), cy + (radius + y) * Math.cos(theta)];
}

// generate sine wave
const sineWave = d3.range(numWaves*numPointsPerWave+1).map(function(k) {
    const x = k * wavelength/numPointsPerWave
    return [x, amplitude * Math.sin(phase + 2 * pi * x/wavelength)];
})

const rotatedSine = sineWave.map( d => rotate(0, 0, d[0], d[1], radius));
// remove the last point as it would overlap the first point of the circle
rotatedSine.pop() 

// get Path commands for given coordinates
const getPath = d3.line()
.x(d => xScale(d[0]))
.y(d => yScale(d[1]))
.curve(d3.curveCardinalClosed) 

// draw sine wave going around a circle
const wave = vis
.append('path')
.attr('d', getPath(rotatedSine))
.attr('fill', 'none')
.attr('stroke', 'black')
.attr('stroke-width', '1px')

svg {
    background-color: steelblue;
}
 
.content {
    fill: lightsteelblue;
}

<script src="https://d3js.org/d3.v4.js"></script>
<svg width="1000" height="1000"></svg>