-1
\$\begingroup\$

Are two points too close?
Trying to do the math as efficiently as possible.

deltaX + deltaY is going to be > than the actual distance

You might use this in game play - are players in range?

// test
bool distanceTooClose = DistanceTooClose(new System.Windows.Point(12, 12), new System.Windows.Point(0, 0), 17);
// end test

static bool DistanceTooClose(System.Windows.Point x, System.Windows.Point y, Double minDistance)
{
    double deltaX = Math.Abs(y.X - x.X);
    double deltaY = Math.Abs(y.Y - x.Y);
    if((deltaX + deltaY) < minDistance)
    {
        return false;
    }
    double distanceSquared = deltaX * deltaX + deltaY * deltaY;
    //double distance = Math.Sqrt(distanceSquared);
    Double minDistanceSquared = minDistance * minDistance;
    return (distanceSquared <= minDistanceSquared);
}
\$\endgroup\$
  • 5
    \$\begingroup\$ What prompted you to write this? How's it used? You went quite minimalistic on the explanation this time ;-) \$\endgroup\$ – Mast Jul 13 '17 at 12:01
  • \$\begingroup\$ @Mast Just playing around. For a game are player in range. \$\endgroup\$ – paparazzo Jul 13 '17 at 12:04
  • \$\begingroup\$ Don’t you have a hypot function as in C? That is an intrinsic and can use the best special CPU instructions that are made for that. \$\endgroup\$ – JDługosz Jul 13 '17 at 14:44
  • \$\begingroup\$ @JDługosz There is the option of Point.Subtract(p1, p2).Length) (and .LengthSquared. Whether or not it would have the best CPU usage or not, I don't know. \$\endgroup\$ – Brian J Jul 13 '17 at 16:04
  • 1
    \$\begingroup\$ See en.wikipedia.org/wiki/Hypot @BrianJ yes, using Point arithmetic rather than doing x and y separately is better. Especially since it already has a Point value! \$\endgroup\$ – JDługosz Jul 14 '17 at 7:25
4
\$\begingroup\$

This is something that you should run a benchmark on to see whether the branch is worth avoiding the 3 multiplications.

It is very likely that the branch will not be worth it due to how branch prediction works. But that depends on what data you will be feeding into it and in how tight of a loop you call it.

\$\endgroup\$
  • \$\begingroup\$ exactly... without a proof there is no optimization ;-) \$\endgroup\$ – t3chb0t Jul 13 '17 at 12:02
  • \$\begingroup\$ You are correct it does not appear to speed it up. Shocked me. \$\endgroup\$ – paparazzo Jul 13 '17 at 12:15
  • 2
    \$\begingroup\$ Also you can just ask the caller to pass minDistanceSquared instead of minDistance, since they may have that handy anyway or be able to compute it at compile time. \$\endgroup\$ – amalloy Jul 13 '17 at 18:46
10
\$\begingroup\$

Use types consistently

I prefer double over Double, but it doesn't really matter which you use as long as it's consistent. Mixing them, though, means unnecessary cognitive load.

Naming

    ...(y.X - x.X);
    ...(y.Y - x.Y);

In my opinion, x and y are about the worst possible names for a type which has members called X and Y. If you insist on one-character names for points, mathematical conventions would typically name them A, B, ... or P, Q, ...

Also the method itself: DistanceTooClose. Whether the distance is too close or close enough is a question of the calling context. The method really checks IsDistanceCloserThan.

Argument validation

What should the following test give?

DistanceTooClose(new System.Windows.Point(1, 1), new System.Windows.Point(0, 0), -2);

KISS

The simplest implementation of the core test is

    var deltaX = q.X - p.X;
    var deltaY = q.Y - p.Y;
    return deltaX * deltaX + deltaY * deltaY <= minDistance * minDistance;

Anything more complicated than that should have a comment explaining why the complication has been added.

\$\endgroup\$
  • 1
    \$\begingroup\$ <= is not CloserThan. I like the variables for testing. \$\endgroup\$ – paparazzo Jul 13 '17 at 13:17
1
\$\begingroup\$

The if((deltaX + deltaY) < minDistance) did not speed it up
Math.Abs is not required as squaring

static bool DistanceTooClose(System.Windows.Point pointA, System.Windows.Point pointB, Double minDistance)
{
    double deltaX = pointA.X - pointB.X;
    double deltaY = pointA.Y - pointB.Y;
    double distanceSquared = deltaX * deltaX + deltaY * deltaY;
    //double distance = Math.Sqrt(distanceSquared);
    //return (distance <= minDistance);
    double minDistanceSquared = minDistance * minDistance;
    return (distanceSquared <= minDistanceSquared);
}
\$\endgroup\$

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.