mathnet-spatial icon indicating copy to clipboard operation
mathnet-spatial copied to clipboard

Checking for equal input points in Plane.FromPoints() is redundant?

Open DavidHollman opened this issue 5 years ago • 0 comments

Ref: https://github.com/mathnet/mathnet-spatial/blob/master/src/Spatial/Euclidean/Plane.cs

Here's the main body of FromPoints:

        if (p1 == p2 || p1 == p3 || p2 == p3)
        {
            throw new ArgumentException("Must use three different points");
        }

        var v1 = new Vector3D(p2.X - p1.X, p2.Y - p1.Y, p2.Z - p1.Z);
        var v2 = new Vector3D(p3.X - p1.X, p3.Y - p1.Y, p3.Z - p1.Z);
        var cross = v1.CrossProduct(v2);

        if (cross.Length <= float.Epsilon)
        {
            throw new ArgumentException("The 3 points should not be on the same line");
        }

It seems to me that the initial check for having two or more equal points is redundant to the later check that the cross product result is > 0. (Two or three equal points would obviously also all fall on the same line.)

Is it worth having the first check? It incurs some extra overhead.

Although in cases where two points really are equal the function would do more work without the first check, since an exception is going to fire anyway I'm not sure that is a real concern.

DavidHollman avatar Nov 19 '20 14:11 DavidHollman