Turning a function based sketch object oriented

Question

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>

Answer 1

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>