1
\$\begingroup\$

I started studying JavaScript about a week ago and I would like some opinions and advice on my code. This program just creates a canvas and instantiates 5 planets that orbit around a sun, giving them each a different speed, color, and size.

It uses the p5.js library, the code can be run in the p5.js web editor. (You might want to expand the preview panel on the right after pasting the code, before you run the script.)

The code is in different files (class Orbiter, extra math functions and main code) but I'll put it here in one go:

// Converts from degrees to radians.
Math.radians = function(degrees) {
    return degrees * Math.PI / 180;
  };

// Converts from radians to degrees.
Math.degrees = function(radians) {
    return radians * 180 / Math.PI;
  };


class Orbiter {

    constructor(rad, orbitAlt, x = 0, y = 0, orbitAngle = 0, orbitAngleMod = 1, colorR=255, colorG=255, colorB=255, colorA=255){
        this.orbitAngle = orbitAngle; // Angle formed by the radius of the orbit and the x plane.
        this.orbitAngleMod = orbitAngleMod; // Increment/decrement of  orbitAngle
        this.rad = rad; // Radius
        this.orbitAlt = orbitAlt; // Distance to the orbited object's position (Alt for altitude)

        // Position
        this.x = x;
        this.y = y;

        // Color variables
        this.colorR = colorR;
        this.colorG = colorG;
        this.colorB = colorB;
        this.colorA = colorA;
    }

    orbit(object){
        this.x = object.x + this.orbitAlt * cos(Math.radians(this.orbitAngle));
        this.y = object.y + this.orbitAlt * sin(Math.radians(this.orbitAngle));
        this.orbitAngle = this.orbitAngle + this.orbitAngleMod;

        // Reset the angle to 0 after a complete revolution to avoid an ever increasing value.
        if(this.orbitAngle >= 360){
            this.orbitAngle = 0;
        }
    }

    display(){
        noStroke();
        fill(this.colorR, this.colorG, this.colorB, this.colorA);
        return ellipse(this.x, this.y, this.rad, this.rad);
    }


}

let planets = [];
let sun = new Orbiter(100, 0);

function setup(){
    createCanvas(windowWidth-3, windowHeight-3);
    frameRate(144);

    // Set up the Sun's colors and coordinates
    sun.colorR = 255;
        sun.colorG = 200;
        sun.colorB = 0;
    sun.x = windowWidth/2;
    sun.y = windowHeight/2;

    // Instantiate 5 planets
    for(i = 0; i < 5; i++){
        planets[i] = new Orbiter(5 + i * 15, 110 + i*70);
        planets[i].orbitAngleMod= 1.4 - i/7;
        planets[i].orbitAngle= i*5;

        planets[i].colorR = i * 50 + 5;
        planets[i].colorG = 255 - planets[i].colorR;
        planets[i].colorB = 255 - planets[i].colorR;
    }
}

function draw(){
    background(0, 10, 40);

    for(planet of planets){
        planet.orbit(sun);
        planet.display();
    sun.display()
    }

}

I'd be really grateful if you could give me some feedback on the structure of my code, whether it's in line with JavaScript best practices, or in general if there's anything you see in there that you think is just wrong or should be written in a different way.

\$\endgroup\$
0
\$\begingroup\$

Looks fine for me. But I'm no JS expert. ;)

One thing that "jumps" me:

if(this.orbitAngle >= 360){
    this.orbitAngle = 0;
}

Instead I'd do:

while (this.orbitAngle>360) this.orbitAngle-=360;
while (this.orbitAngle<0) this.orbitAngle+=360;

Would be nice if you could provide a playground like jsfiddle.

@Jamal asked me to edit:

  • Your code misses the case when orbitAngles are decreasing.
  • If the orbitAngle goes over 360 you set it to 0 but that might be incorrect. What if you get an orbitAngle of 365.1? Setting it to 0 is wrong because 5.1 would be the correct value. Same applies for decreasing values. -5.9 should become 350.1 and not something like 360.

That is why I suggested a version where you correct the value by adding/substracting 360.

And if somebody abuses the code and enters very high values to change the angle you might end up with orbitAngle>720 so substracting 320 would not be enough.

Another suggestion might be to use modulo - now that I think about it. ;)

\$\endgroup\$

Your Answer

By clicking "Post Your Answer", you acknowledge that you have read our updated terms of service, privacy policy and cookie policy, and that your continued use of the website is subject to these policies.

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