Turning a function based sketch object oriented-CodePudding

I am trying to take Dan Shiffman's prime spiral program and make it object oriented.

I am putting all variables into the constructor and making the functions into methods to encapsulate them.

But the OOP version of the code does not draw more than 1 triangle to the screen. I can't see why there should be a problem, as I have all the variables and functions included in the scope of the primeSpiral class.

Working code without classes

let x, y;
let step = 1;
let stepSize = 20;
let numSteps = 1;
let state = 0;
let turnCounter = 1;
let offset = 0;

function setup() {
  createCanvas(900, 900);
  background(0);
  const cols = width / stepSize;
  const rows = height / stepSize;

  x = width / 2;
  y = height / 2;

  for (let i = 0; i < 500; i  ) {
    noStroke();
    primeSpiral(20, 1)
    primeSpiral(30, 200)
    incrementStep();
  }
}

function incrementStep() {
  switch (state) {
    case 0:
      x  = stepSize;
      break;
    case 1:
      y -= stepSize;
      break;
    case 2:
      x -= stepSize;
      break;
    case 3:
      y  = stepSize;
      break;
  }

  if (step % numSteps == 0) {
    state = (state   1) % 4;
    turnCounter  ;
    if (turnCounter % 2 == 0) {
      numSteps  ;
    }
  }
  step  ;


}

function primeSpiral(offset, color) {
  if (!isPrime(step   offset)) {
    //might put something here

  } else {
    let r = stepSize * 0.5;
    fill(color, 99, 164);
    push();
    translate(x, y);
    rotate(-PI / 4);
    triangle(-r,  r, 0, -r,  r,  r);
    pop();
  }
}

function isPrime(value) {
  if (value == 1) return false;
  for (let i = 2; i <= sqrt(value); i  ) {
    if (value % i == 0) {
      return false;
    }
  }
  return true;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.4.1/p5.js"></script>

With classes:

let stepSize = 20;

function setup() {
  createCanvas(900, 900);
  background(0);
  const cols = width / stepSize;
  const rows = height / stepSize;
  x = width / 2;
  y = height / 2;
  background(0);
  prime = new PrimeSpiral(0, 0, 2, stepSize, 1, 0) //: x,y,offset,color
}

function draw() {
  prime.walk();
}

class PrimeSpiral {
  constructor(x, y, number) {
    this.x = x;
    this.y = y;
    this.number = number;
    this.stepSize = 20;
    this.numSteps = 1;
    this.state = 0;
    this.turnCounter = 1;
  }

  walk() {
    if (!this.isPrime(this.number)) {
      console.log(this.succes);
    } else {
      let r = stepSize * 0.5;
      fill(color, 99, 164);
      push();
      translate(x, y);
      rotate(-PI / 4);
      triangle(-r,  r, 0, -r,  r,  r);
      pop();
      this.incrementStep()
    }
  }

  isPrime(value) {
    if (value == 1) return false;
    for (let i = 2; i <= Math.sqrt(value); i  ) {
      if (value % i == 0) {
        return false;
      }
    }
    return true;
  }

  incrementStep() {
    switch (this.state) {
      case 0:
        this.x  = this.stepSize;
        break;
      case 1:
        this.y -= this.stepSize;
        break;
      case 2:
        this.x -= this.stepSize;
        break;
      case 3:
        this.y  = this.stepSize;
        break;
    }

    if (this.step % this.numSteps == 0) {
      this.state = (this.state   1) % 4;
      this.turnCounter  ;
      if (this.turnCounter % 2 == 0) {
        this.numSteps  ;
      }
    }
    this.step  ;
  }
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.4.1/p5.js"></script>

CodePudding user response：

Sometimes you need to slow down to go faster :)

My hunch is you might attempted to write the class all in one go, without slowing down and testing one function at a time to ensure no errors snuck through.

Here are some of the main errors:

you forgot to add the step property to the class (this.step = 1;) in the constructor. In incrementStep() this.step which is undefined gets incremented which results in NaN throwing the rest off (e.g. state, etc.)
you left stepSize mostly as global variable, but are also using this.stepSize: try to avoid ambiguity and use one or the other (I recommend using this.stepSize since it will allow multiple instances to have independent step sizes)
In walk you're still using translate(x, y); when you probably meant translate(this.x, this.y);

Here's a version with the above notes applied (and optionally no longer incrementing or drawing triangles if x,y go outside the screen bounds):

let spiral1;
let spiral2;

function setup() {
  createCanvas(900, 900);
  background(0);
  noStroke();
  // instantiate spirals here so width, height set
  spiral1 = new PrimeSpiral(20, 1);
  spiral2 = new PrimeSpiral(30, 200);
}

function draw(){
  spiral1.walk();
  spiral2.walk();
}

class PrimeSpiral{
  
  constructor(offset, color){
    this.x = width / 2;
    this.y = height / 2;
    this.step = 1;
    this.stepSize = 20;
    this.numSteps = 1;
    this.state = 0;
    this.turnCounter = 1;
    this.offset = offset;
    this.color = color;
  }
  
  incrementStep() {
    switch (this.state) {
      case 0:
        this.x  = this.stepSize;
        break;
      case 1:
        this.y -= this.stepSize;
        break;
      case 2:
        this.x -= this.stepSize;
        break;
      case 3:
        this.y  = this.stepSize;
        break;
    }

    if (this.step % this.numSteps == 0) {
      this.state = (this.state   1) % 4;
      this.turnCounter  ;
      if (this.turnCounter % 2 == 0) {
        this.numSteps  ;
      }
    }
    this.step  ;
  }
  
  walk(){
    // optional, early exit function if we're offscreen
    if(this.x < 0 || this.x > width) return;
    if(this.y < 0 || this.y > height) return;
    
    if (!isPrime(this.step   this.offset)) {
      //console.log('not prime:', step   offset);
    } else {
      let r = this.stepSize * 0.5;
      fill(this.color, 99, 164);
      push();
      translate(this.x, this.y);
      rotate(-PI / 4);
      triangle(-r,  r, 0, -r,  r,  r);
      pop();
    }
    this.incrementStep();
  }
  
}

function isPrime(value) {
  if (value == 1) return false;
  for (let i = 2; i <= sqrt(value); i  ) {
    if (value % i == 0) {
      return false;
    }
  }
  return true;
}

<script src="https://cdnjs.cloudflare.com/ajax/libs/p5.js/1.4.1/p5.min.js"></script>