6
\$\begingroup\$

I made this sketch in Processing (basically Java) that consists of a bunch of particles moving around a Perlin Noise field. Each particle is aware of the position of one other particle and this creates a rope like effect. The sketch is intended to be aesthetic, like a screensaver, and I in that way it might be difficult to judge the efficiency of the code.

Nevertheless, I'm hoping that there is something that I'm doing wrong stylistically that could be pointed out.

    int emergenceInterval = int(random(75, 100));
    float a, freq = 0.05;
    float alphDelta, alphAccel, alphSpringing = 0.0009, alphDamping = 0.98, alphX, maxAlph;
    int pAmt = int(random(2000, 4000));
    Particle p[] = new Particle[pAmt]; 

    void setup() {
      background(0);
      size(1000, 500); 
      for (int i = 0; i < pAmt; i++) {
        p[i] = new Particle(random(width), random(height), i);
      }
    }

    void draw() {
      if(frameCount % emergenceInterval == 0) {
       if(maxAlph == .05) maxAlph = 1;
       else maxAlph = .05; 
      }
      a += freq;
      fill(50, findAlpha() * 255);
      rect(0, 0, width, height);
      runParticles();
    }

    void runParticles() {
      for (Particle _p : p) {
        Particle lastP = _p;
        if (_p.index != 0) lastP = p[_p.index - 1];
        _p.update(450, lastP.pos, abs(pow(sin(radians(a)), 3)));
        point(_p.pos.x, _p.pos.y);
        _p.vel.x = 1;
      }
    }

    float findAlpha() {
      if (!(alphAccel < 0)) alphDelta = width - alphX;
      else alphDelta = 0 - alphX;
      alphDelta *= alphSpringing;
      alphAccel += alphDelta;
      alphAccel *= alphDamping;
      alphX += alphAccel;
      float alph = (alphX * (maxAlph * abs(pow(sin(radians(a)), 2))))/width;

      if(alphX > width) alphAccel *= -1;
      else if(alphX < 0) alphAccel *= -1;
      return alph;
    }


  class Particle {

  int index;
  float theta;
  float stride;
  float maxSpeed = 5;
  float maxChaos = 10;
  PVector accel, vel, pos;


  Particle(float startX, float startY, int index) {
    this.index = index;
    accel = new PVector(0, 0);
    vel = new PVector(0, 0);
    pos = new PVector(startX, startY);
  } 

  void update(float stride, PVector target, float chaos) {
    this.stride = stride;
    stroke(50*(noise(pos.x)),0,50*(noise(pos.x)), noise(pos.x, pos.y) * 255);
    accel = noiseDir(chaos);
    accel.add(followDir(target));
    if (pos.x > width) pos.x = 0;
    if (pos.y > height) pos.y = 0;
    if (pos.y < 0) pos.y = height;
    vel.add(accel);
    float speed = noise(pos.x, pos.y) * maxSpeed;
    vel.limit(speed);
    pos.add(vel);
  }

  PVector noiseDir(float chaos) {
    theta = abs(noise(pos.x, pos.y)) * 180;
    float x = pos.x + stride * cos(theta);
    float y = pos.y + stride * sin(theta);
    PVector dir = new PVector(x - pos.x, y - pos.y);
    dir.normalize();
    dir.mult(chaos * maxChaos);
    return dir;
  }

  PVector followDir(PVector target) {
    PVector dir = new PVector(target.x - pos.x, target.y - pos.y);
    dir.normalize();
    dir.mult(5);
    return dir;
  }
}
\$\endgroup\$
  • \$\begingroup\$ Is this code complete? If this is in Java why are there a bunch of functions and variables defined in the middle of nowhere that are not inside a class? \$\endgroup\$ – Casey Aug 21 '11 at 5:57
  • \$\begingroup\$ In Processing there are self contained setup() and draw() functions so you can create programs really quickly. It's based on Java but I'm not sure of the details. \$\endgroup\$ – Miles Aug 21 '11 at 10:17
3
\$\begingroup\$

(1). From the point of style I'd recommend you not to mix the level of details in a function. For example.

Instead of writing something like this:

void setup() {
  background(0);
  size(1000, 500); 
  for (int i = 0; i < pAmt; i++) { // this is bad
    p[i] = new Particle(random(width), random(height), i);
  }
}

You should write something like this. The same is applicable for other functions.

void setup() {
  background(0);
  size(1000, 500); 
  fillArray(); // there goes the loop and it's load easier to read.
}

(2). As for naming conventions I'd recommend you to use a verb as a first part for the function's name. Code like this looks unusual.

background(0);
size(1000, 500); 

(3). You should be consistent with names for your variables. For example in your update() function you can see a mix of full and short names like stride, target, chaos vs accel, vel, pos. It really makes me think a lot more about names than I should.

(4).If you have to call this "normalize()" method a lot after construction of the object I'd recommend to think about adding this method to consturctor for example with the flag called "shouldNormalize".

PVector dir = new PVector(x - pos.x, y - pos.y);
dir.normalize();

vs

PVector dir = new PVector(x - pos.x, y - pos.y, true); 
//or make it to normalize the vector by default and pass false when you don't need it
//but I guess that you should normalize it quite often after creation  

(5). I would strongly recommend to make Particle class immutable. It would save you from a lot of hacks like this and make your code cleaner:

 void update(float stride, PVector target, float chaos) {
 this.stride = stride;
 ... } 
\$\endgroup\$
  • \$\begingroup\$ I understand your first point, but I'm not positive what level of details means. Does it mean that if a function deals with high level areas like setting the size of the program specific things like filling an array should be done in other more specialized functions? \$\endgroup\$ – Miles Aug 22 '11 at 20:43
  • 1
    \$\begingroup\$ Yes. It's very hard to read a code that jumps from high level abstractions to the low level and back. \$\endgroup\$ – Andrey Taptunov Aug 23 '11 at 10:08
1
\$\begingroup\$

Some small observations:

//maybe it works here, but generally you should use Math.abs(a-b) < epsilon 
//with a very small epsilon instead a == b for double comparision
if(maxAlph == .05) maxAlph = 1;
else maxAlph = .05; 
-->
maxAlph = (maxAlph == .05) ? 1 : 0.05;  

if(alphX > width) alphAccel *= -1;
else if(alphX < 0) alphAccel *= -1;
-->
if(alphX > width || alphX < 0) alphAccel *= -1;

if (!(alphAccel < 0)) alphDelta = width - alphX;
else alphDelta = 0 - alphX;
-->
alphDelta  = (alphAccel >= 0) ? width - alphX : -alphX;

And why is theta a member variable?

\$\endgroup\$
  • \$\begingroup\$ I'm not sure I understand why theta wouldn't be a member variable. Do you think you could clarify that? \$\endgroup\$ – Miles Aug 22 '11 at 20:45
  • 1
    \$\begingroup\$ @Miles: As far as I can see theta is used only in noiseDir, where it could be a local variable. \$\endgroup\$ – Landei Aug 23 '11 at 7:22

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.