1
\$\begingroup\$

I've developed a function to calculate the exit angle of a projectile after it collides with a boundary/edge inside a rectangular space, based on the entry angle and the axis it is colliding with.

Given those two factors, the actual boundary is implied, since a projectile travelling at an angle of 315° (where 0°/360° sit at 3 o'clock) and colliding with a horizontal boundary (the x axis) must be colliding with the top boundary.

The current implementation came about through iterative refactoring of a much more verbose and redundant version, however I can't vocalise exactly why some parts are necessary, but they appear to satisfy the requirements and I'm confident it works based on a number of tests.

function reflectAngle(angle, axis) {

    if (angle % 90 === 0)
        return (angle + 180) % 360;

    let reflected;
    let segment = (angle % 90) * 2;

    if (axis === 'X')
        reflected = angle + (180 - segment);

    else if (axis === 'Y')
        reflected = angle - segment;

    if (angle < 90)
        reflected += 180;

    return reflected % 360;
}

Can this function be simplified or improved for readability?

\$\endgroup\$
  • \$\begingroup\$ Could you include example cases that you tested? I'm not convinced that all of these calculations make sense. \$\endgroup\$ – 200_success Feb 14 '18 at 3:32
2
\$\begingroup\$

Review

A little ambiguous, and overly complex. You need to familiarize your self with radians and forget about degrees

Style

Block-less blocks

A block is a section of code delimited by {...} many C like syntax languages allow you to skip the block delimiters for single line blocks after conditional statements.

This can be a major source of life long frustration, and there is not a programmer alive that does not know the insidious nature of the bugs that can result when you make changes to the code and forget the {...}

// worst
if (foo === bar)
       foo = poo

// bad
if (foo === bar)
       foo = poo;

// better but be consistent never mix this with above
if (foo === bar) foo = poo;

// best 
if (foo === bar) { foo = poo } // note that ; and } denote the end of 
                                // an expression, you don't need to use both

// or
if (foo === bar) { 
   foo = poo;
}

Constants

So many magic numbers. If you find your self adding the same numbers over and over it is a good sign that a defined constant is better. You never know when things may change. I.E. switch units from degrees to radians.

const A90  = 90;
const A180 = 180;
const A360 = 360;

Then if you want to change to radians

const A90  = Math.PI / 2;
const A180 = Math.PI;
const A360 = Math.PI * 2;

or clock angles

const A90  = 3;
const A180 = 6;
const A360 = 12;

No need to find each number in the code base.


Logic

  1. The axis argument is ambiguous. Axis? is it the axis of reflection or the axis to reflex from. Also you use a string and to humans "x", "X" and "y", "Y" have the same meaning, but your code sees them differently, this is never a good thing. It would pay to check both (upper and lower) or convert to lowercase, or best use a defined constant,

  2. Why are you using degrees, NO Math functions uses degrees, the only time you need to use degrees is for output, which generally is never needed. Learn to work in radians, it makes a lot of angle related maths so much easier.

  3. The code is just too complex for something so simple. The problem is just one of negating the scalar associated with the direction of reflection. I.E. a vector has two scalars x,y if you reflect along X you negate x, if you reflex along Y you negate y.

    Messing around with the cyclic angle is not needed. I have a general rule of thumb that if I ever find my self having to normalize a cyclic value E.G. (angle % 360) I am doing it wrong and there is a better way.


Rewrite

This is how I would write the function. I would never use degrees, that is only ever needed for display only. That makes the function much simplier.

// defined axis names
const AXIS = {x : 0, y : 1}

// function computes the reflected angle
// rad Incoming angle in radians
// dir direction of reflection. Any of AXIS, defaults to AXIS.y
function reflectAngle(rad, dir) {
    return dir === AXIS.x ? Math.acos(-Math.cos(rad)) : Math.asin(-Math.sin(rad));
}



// usage
var reflected = reflectAngle(1, AXIS.x);

And a version for radian dislike'rs

// Use the constants to convert degree to radians and back
const deg2Rad = Math.PI / 180;
const rad2Deg = 1 / deg2Rad;

function reflectAngleDegrees(deg, dir) {
    const rad = deg * deg2Rad;
    return (dir === AXIS.x ? 
            Math.acos(-Math.cos(rad)) : 
            Math.asin(-Math.sin(rad))
        ) * rad2Deg;
}
\$\endgroup\$

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.