Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Could easy optimization be helpful? #24

Open
david-wagstaff opened this issue Aug 1, 2021 · 1 comment
Open

Could easy optimization be helpful? #24

david-wagstaff opened this issue Aug 1, 2021 · 1 comment

Comments

@david-wagstaff
Copy link

Your code is simple and well explained. I WOULD NOT REPLACE IT. I'm wondering if you wish to add an appendix on simple optimization. I would start the chapter with Donald Knuth's quote about premature optimization being the root of all programming evil.

Optimization could mean removing unnecessary code. A good example is you omitted an "else" in your code. You could show eliminating the "if" too.

Optimization could mean faster execution time. Since a prime target for collision detection is games where speed is king, every msec counts. Detecting collisions between 1,000 particles is a killer. It's reason my google foo lead me to you.

No one should ever optimize
A. without running tests before and after
B. without timing before and after
C. more than one change at a time

FWIW, I expect some of the optimizations below to be the same speed, and at least one to be slower.

Original Function

boolean circleCircle(float c1x, float c1y, float c1r, float c2x, float c2y, float c2r) {

  // get distance between the circle's centers
  // use the Pythagorean Theorem to compute the distance
  float distX = c1x - c2x;
  float distY = c1y - c2y;
  float distance = sqrt( (distX*distX) + (distY*distY) );

  // if the distance is less than the sum of the circle's
  // radii, the circles are touching!
  if (distance <= c1r+c2r) {
    return true;
  }
  return false; // no need for an else
}

Proposed Refactoring 1

boolean circleCircle(float c1x, float c1y, float c1r, float c2x, float c2y, float c2r) {

  float distX = c1x - c2x;
  float distY = c1y - c2y;
  float distance = sqrt( (distX*distX) + (distY*distY) );

  // the condition is exactly the boolean we want to return
  return distance <= c1r+c2r;

// DOES IT GIVE THE SAME RESULTS?
// IS IT THE SAME OR FASTER?
}

Proposed Refactoring 2

boolean circleCircle(float c1x, float c1y, float c1r, float c2x, float c2y, float c2r) {

  float distX = c1x - c2x;
  float distY = c1y - c2y;
  // no need for variable to hold distance

  return sqrt( (distX*distX) + (distY*distY) ) <= c1r+c2r;

// DOES IT GIVE THE SAME RESULTS?
// IS IT THE SAME OR FASTER?
}

Proposed Refactoring 3

boolean circleCircle(float c1x, float c1y, float c1r, float c2x, float c2y, float c2r) {

  float distX = c1x - c2x;
  float distY = c1y - c2y;
  // let's trade the expensive sqrt for a square

  return  (distX*distX) + (distY*distY) ) <= (c1r+c2r) * (c1r+c2r);

// DOES IT GIVE THE SAME RESULTS?
// IS IT THE SAME OR FASTER?
}

Proposed Refactoring 4a

boolean circleCircle(float c1x, float c1y, float c1r, float c2x, float c2y, float c2r) {

  return  (c1x-c2x * c1x-c2x) + (c1y-c2y * c1y-c2y) ) <= (c1r+c2r) * (c1r+c2r);
// good idea, but there's a bug. Can you spot it and fix it?

// DOES IT GIVE THE SAME RESULTS?
// IS IT THE SAME OR FASTER?
}

Proposed Refactoring 4b

boolean circleCircle(float c1x, float c1y, float c1r, float c2x, float c2y, float c2r) {
  // can we make the squaring easier?
  return  Math.pow(c1x-c2x, 2) + Math.pow(c1y-c2y, 2 ) <= Math.pow(c1r+c2r, 2);
// good idea, but there's a bug. Can you spot it and fix it?

// DOES IT GIVE THE SAME RESULTS?
// IS IT THE SAME OR FASTER?
}
@jeffThompson
Copy link
Owner

This is a wonderful suggestion, thank you! Optimization has been on my to-do list forever but haven't had time to get to it. I'm also def not an expert in that, so any other suggestions would be great! Not going to have time to get into this too much in the near-term but thanks for bumping this :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants