8
\$\begingroup\$

I made an object oriented fractal tree in javascript using the p5 library, it consists of three files:

  • Fraternal Tree.js
  • branch.js
  • flower.js

Here is the github repo with the source code. All the main work is done in Fraternal Tree.js I want to know if I am following good coding practices and naming conventions or not. Also, if you increase the number of branches to anything above 1000, the fps drops significantly, is there a way I can optimize this to get better fps? Maybe using more efficient data structures? Or some other practice?

You can run the sketch HERE

Code :

Fraternal Tree.js -

var tree = []; //stores branch objects including the root
var flowers = []; //stores flower objects

//flower objects are assigned only to ungrown branches on clicking "grow" button
//this array is cleared on clicking "shed" button

var branchNumber = 0; //stores number of branches(excluding root)

//used for buttons and checkboxes    
var shrink, shake, intensity, grow = false,
    shed, gravity, flsize, grav, wind_dir, windcheck;
var cnv;

function setup() {
    cnv = createCanvas(500, 500);
    //create a root at the bottom middle of the screen
    var root = new Branch(createVector(width / 2, height), createVector(width / 2, height - 100));
    tree[0] = root;
    //code in between to setup dom elements
    cnv.mousePressed(branchIt); //create branch on mouse press

function draw() {
    //this is the animation segment, gets called fps times every second
    background(51);
    fill(255);
    textSize(24);
    text("Number of branches = " + branchNumber, 15, 30);

    //determine value of wind vector based on dom elements
    if (windcheck.checked()) {
        if (wind_dir.value() == "Left")
            wind = createVector(-0.2, 0);
        else if (wind_dir.value() == "Right")
            wind = createVector(0.2, 0);
    }

    //loop through branch objects
    for (var i = 0; i < tree.length; i++) {
        tree[i].show(); //display every branch object
        if (tree[i].flower) //if flower for current branch exists, set its value
            tree[i].flower.size = flsize.value();
        //shake function shakes the tree
        if (shake.checked()) {
            tree[i].shake(intensity.value());
        }
        if (!tree[i].grown && grow && tree[i].flower) {
            tree[i].growFlower();
        }
        //apply forces if shed button is pressed and flower exists 
        if (shed && tree[i].flower) {
            if (grav.checked()) {
                tree[i].flower.applyForce(gravity);
            }
            if (windcheck.checked())
                tree[i].flower.applyForce(wind);
            if (!tree[i].flower.done)
                tree[i].flower.shed();
        }
    }
}
function branchIt() {

    for (var i = tree.length - 1; i >= 0; i--) {
        if (!tree[i].grown) {
            tree.push(tree[i].spawnA(shrink.value()));
            tree.push(tree[i].spawnB(shrink.value()));
            branchNumber += 2;
        }
        tree[i].grown = true;
    }
}

Some less important code :

shedFlowers is called when shed button is pressed. growFlowers is called when grow button is pressed.

function shedFlowers() {
    if (grow)
        shed = true;
    flowers.splice(0, flowers.length);
}
function growFlowers() {
    grow = true;
    for (var i = 0; i < tree.length; i++) {
        if (!tree[i].grown) {
            var flower = new Flower(tree[i]);
            flowers.push(flower);
            tree[i].setFlower(flower);
        }
    }
    shed = false;
}

branch.js -

class Branch() {

    constructor(start, end){
      this.start = start;
      this.end = end;
      this.grown = false;
      this.flower = null; //every branch has a null flower
    }    
    shake(intensity) {

        this.end.x = constrain(this.end.x + random(-intensity, intensity), this.end.x - 2, this.end.x + 2);
        this.end.y = constrain(this.end.y + random(-intensity, intensity), this.end.y - 2, this.end.y + 2);
    }

    setFlower(flower) {
        this.flower = flower;
    }

    show() {

        stroke(255);
        line(this.start.x, this.start.y, this.end.x, this.end.y);
    }
    spawnA(size) {

        var dir = p5.Vector.sub(this.end, this.start);
        dir.mult(size);
        dir.rotate(PI / 4);

        var newEnd = p5.Vector.add(dir, this.end);
        var right = new Branch(this.end, newEnd);
        return right;
    }
    spawnB(size) {

        var dir = p5.Vector.sub(this.end, this.start);
        dir.mult(size);
        dir.rotate(-PI / 4);

        var newEnd = p5.Vector.add(dir, this.end);
        var left = new Branch(this.end, newEnd);
        return left;
    }

    growFlower() {

        this.flower.draw();
        this.flower.update();
    }    
}

flower.js -

class Flower() {


    constructor(branch) {      
      this.size = 10;
      this.velocity = createVector(0, 0);
      this.acceleration = createVector(0, 0);
      this.pos = branch.end.copy();
      this.done = false;
      this.lifespan = 255;
    }

    draw() {
        //get colour from colour picker            
        var colour = document.getElementById("colour").jscolor.rgb;
        fill(colour[0],colour[1],colour[2], this.lifespan);
        // fill(document.getElementById("colour").jscolor.toHEXString(),,,this.lifespan);
        noStroke();
        ellipse(this.pos.x, this.pos.y, this.size, this.size);
    }
    update() {
        if (!this.done)
            this.pos = branch.end.copy();

        this.pos.add(this.velocity);
        this.velocity.add(this.acceleration);

        this.acceleration.mult(0);
        if (this.done) //only start fading after shed button is pressed
            this.lifespan-=0.8;
    }
    shed() {
        this.applyForce(createVector(random(-1, 1), random(-1, -0.1)));
        this.update();
        this.draw();
        this.done = true;
    }
    applyForce(force) {
        this.acceleration.add(force);
    }
}

Update #1

I replaced all the functions in branch.js and flower.js with prototype functions, this helped but the results were no where near significant. And some ES6 sugar syntax.

\$\endgroup\$
6
  • \$\begingroup\$ Would SVG not be a good alternative since it doesn't require re-drawing the tree each time? \$\endgroup\$ Mar 2, 2017 at 17:14
  • \$\begingroup\$ Of course it would be, but that's not what the point, I want to improve this, not completely replace it with something else/better. \$\endgroup\$
    – Ayush Seth
    Mar 2, 2017 at 17:22
  • \$\begingroup\$ does it need to be object oriented is functional programming style solution acceptable? \$\endgroup\$
    – user93
    Mar 29, 2017 at 12:59
  • \$\begingroup\$ @user93 I guess it could be, in another language, however it needs to be in javascript, which is object oriented. Moreover, I don't have any experience with functional programming style \$\endgroup\$
    – Ayush Seth
    Mar 30, 2017 at 11:05
  • \$\begingroup\$ @MasterYushi you can write functional code in javascript. Libraries like ramdaJs help you do that ramdajs.com \$\endgroup\$
    – user93
    Mar 30, 2017 at 12:11

2 Answers 2

2
\$\begingroup\$

It is pretty easy to read and understand. However here are some minor suggestions for improvement:

The top level fields could be named in a way that does not require the almost trivial comments. If it is important to note that the array flowers stores flower objects, just call it flowerObjects. The tree is a bit misleading because one could assume that it is a tree object. The array could be named allBranches or even just branches.

I break up functions in smaller parts wherever possible. A good indicator are comments before a block of some coherent lines of code like in this example:

//determine value of wind vector based on dom elements
if (windcheck.checked()) {
    if (wind_dir.value() == "Left")
        wind = createVector(-0.2, 0);
    else if (wind_dir.value() == "Right")
        wind = createVector(0.2, 0);
}

This could be rewritten like this:

wind = getWindVector(windcheck.checked(), wind_dir.value());

function getWindVector(windChecked, dir) {
    let x = windChecked ? 0.2 : 0;
    if (dir == "Left") x = x * -1; 
    return createVector(x, 0);
}

Note that again the comment is made redundant.

Another indication that some pieces of code ought to be extracted to separate functions is that you are directly accessing the UI elements in the code that builds the tree. Note how hiding the wind_dir.value() behind the dir parameter made the code a bit easier to read.

\$\endgroup\$
1
\$\begingroup\$

Good work updating to ES6 syntax. With ES6 come block scoped variables - declared with const and let. It is advised to default to using const and then switch to let when re-assignment is deemed necessary. This helps avoid accidental re-assignment and other bugs.

If you want to optimize this to get better fps you could explore ways to reduce calculations - e.g. consider looping in reverse if acceptable.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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