1
\$\begingroup\$

I'm putting together a short demo on two things:

  1. Finding the angle between two line segments and direction (right or left) as you travel counterclockwise around a shape
  2. Using newer ecmascript syntax (e.g. .map, classes, const, let (no vars) backticks in console logging)

I'd like some feedback on whether the approach I've chosen for finding angles and directions is sound mathematically (I dislike demonstrating something that isn't decent logic). I know I could do without the Vector usage but am trying to explain to my audience relative vs. absolute coordinates, particularly when using atan2.

And feedback on the syntax/methods. Please bear in mind that since this is a demo, the code is verbose in output. (I purposely use console.warn for each angle just to make them easier to spot, so no comments on that are necessary :-)).

For this set of points, starting from the upper left-hand corner and working counterclockwise, the expected output is 45°, 45°, -90°, 45°, 45°, -90°.

graph showing points

There are two classes, a constant to convert between degrees/radians and a small dataset:

class Vector {

    constructor(x, y, z) {
        this.x = x || 0;
        this.y = y || 0;
        this.z = z || 0;
    }
}

class Point {
    constructor(x, y) {
        this.x = x || 0;
        this.y = y || 0;
    }
}

const degRad = 180 / Math.PI;

const testCoordSet = [
    [-5, 15],
    [- 5, 0],
    [0, 5],
    [5, 0],
    [5, 15],
    [0, 10]
];

Then:

// Generate our test set of points - using map because we need to return a new collection
const testPts = testCoordSet.map(coord => new Point(coord[0], coord[1]));
console.log(testPts);

// Using forEach because we're not returning anything. In practice, further
//  operations could benefit from map because we can chain
testPts.forEach((pt, index, array) => {

    console.log(`\r\n${index}`);

    let pointA, pointC;

    const pointB = pt;

    // Determine which points will make up left, center and right points
    if (index === 0) { // If at beginning, left is the last point in the array

        pointA = array[array.length - 1];
        pointC = array[index + 1];

    } else if (index === array.length - 1) { // If at end, right is the first point in the array

        pointA = array[index - 1];
        pointC = array[0];

    } else {

        pointA = array[index - 1];
        pointC = array[index + 1];
    }

    const vectorAB = new Vector(pointA.x - pointB.x,
        pointA.y - pointB.y);

    const vectorCB = new Vector(pointC.x - pointB.x,
        pointC.y - pointB.y);

    console.log("vectorAB:", vectorAB);
    console.log("vectorCB:", vectorCB);

    // Stay in radians until the final result is calculated
    const absAngleABRad = Math.atan2(vectorAB.y, vectorAB.x);
    console.log(`absAngleABRad: ${absAngleABRad}`);

    const absAngleCBRad = Math.atan2(vectorCB.y, vectorCB.x);
    console.log(`absAngleCBRad: ${absAngleCBRad}`);

    const angularDiffRad = absAngleABRad - absAngleCBRad;
    console.log(`angularDiffRad: ${angularDiffRad}`);

    const angleDeg = angularDiffRad * degRad;
    console.warn(`angleDeg: ${angleDeg}`);
});

For comparison, I've thrown in a traditional for-next loop:

for (let i = 0; i < testPts.length; i++) {

    console.log(`\r\n${i}`);

    let pointA, pointC;

    const pointB = testPts[i];

    // Determine which points will make up left, center and right points
    if (i === 0) { // If at beginning, left is the last point in the array

        pointA = testPts[testPts.length - 1];
        pointC = testPts[i + 1];

    } else if (i === testPts.length - 1) { // If at end, right is the first point in the array

        pointA = testPts[i - 1];
        pointC = testPts[0];

    } else {

        pointA = testPts[i - 1];
        pointC = testPts[i + 1];
    }

    const vectorAB = new Vector(pointA.x - pointB.x,
        pointA.y - pointB.y);

    const vectorCB = new Vector(pointC.x - pointB.x,
        pointC.y - pointB.y);

    console.log("vectorAB:", vectorAB);
    console.log("vectorCB:", vectorCB);

    // Stay in radians until the final result is calculated
    const absAngleABRad = Math.atan2(vectorAB.y, vectorAB.x);
    console.log(`absAngleABRad: ${absAngleABRad}`);

    const absAngleCBRad = Math.atan2(vectorCB.y, vectorCB.x);
    console.log(`absAngleCBRad: ${absAngleCBRad}`);

    const angularDiffRad = absAngleABRad - absAngleCBRad;
    console.log(`angularDiffRad: ${angularDiffRad}`);

    const angleDeg = angularDiffRad * degRad;
    console.warn(`angleDeg: ${angleDeg}`);
}

The complete, updated code may be found here: https://gist.github.com/stonetip/e79254e001bda862119e97d7646e4c66

\$\endgroup\$
  • \$\begingroup\$ You should use Array.forEach not map. \$\endgroup\$ – Blindman67 Feb 12 '18 at 11:00
  • \$\begingroup\$ @Blindman67 thanks for that suggestion. I've updated the code, comments and gist. It's an added bonus to be able to explain the difference between map and forEach when I demo this. \$\endgroup\$ – Stonetip Feb 12 '18 at 14:51
2
\$\begingroup\$

Vector

The z is not needed. You are working exclusively in 2D, you Points are 2D, and it is never set or referenced. Get rid of it.

One addition I would make is to have an option to make the vector based on two point. Your class would then look like this

class Vector {
  constructor(a, b) {
    if(a instanceof Point && b instanceof Point) {
      this.x = b.x - a.x;
      this.y = b.y - a.y;
    }
    else {
      this.x = a || 0;
      this.y = b || 0;
    }
  }
}

I have renamed the arguments to make it easier to follow. You can then use it like this

const vectorAB = new Vector(pointA, pointB);
const vectorCB = new Vector(pointC, pointB);

Converting the angle

I would use a function to do this instead. What do you have to do with this constant value? Multiply with it? Add it? Divide with it? With a function you know what you've got. You put a value in, you get a value out. The name could also be a little more descriptive.

function degToRad(deg) {
  return deg * 180 / Math.PI;
}

Rest parameters

If you want more ES6 you can use the spread operator when generating your points.

const testPts = testCoordSet.map(coord => new Point(...coord));

DRY

When getting the right points you are repeating yourself a bit. The points are decided by a simple condition, so you can use a conditional operator.

const pointA = array[index === 0 ? array.length-1 : index-1];
const pointB = pt;
const pointC = array[index === array.length-1 ? 0 : index+1];
\$\endgroup\$
  • \$\begingroup\$ Those are awesome suggestions to improve the code! Thanks for helping me learn. I really like the idea of having the Vector class accept either points or x,y vals. FYI, I have z in the class in case it's used with 3D code in the future. \$\endgroup\$ – Stonetip Feb 14 '18 at 18:11
  • \$\begingroup\$ For even more ES6 you could use default parameters for the constructor: constructor(a = 0, b = 0) whereby simplifying the assignment later on: this.x = a; this.y = b;. \$\endgroup\$ – altocumulus Mar 2 '18 at 13:25

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.