gradientspace / geometry3Sharp

C# library for 2D/3D geometric computation, mesh algorithms, and so on. Boost license.
http://www.gradientspace.com
Boost Software License 1.0
1.71k stars 389 forks source link

Vector3D.Normalize() return value. #136

Closed CBenghi closed 4 years ago

CBenghi commented 4 years ago

EDIT: The code in this comment had regressions, the new one in the reply below might be useful for the XML comment.

Hello @rms80,

I'm puzzled by the return value of Vector3D.Normalize(). You are returning length which only gets updated if the initial value is < 0.

If this is not the intended behavior I'd change the code to:

/// <summary>
/// Sets the lenght of the vector to 1 or 0.
///
/// If the vector's initial length is greater than <see cref="epsilon"/> the resulting length is 1, otherwise 0
/// </summary>
/// <param name="epsilon">the value against witch the initial length comparison is effected, to determine the normalization behaviour</param>
/// <returns>the updated length of the vector after normalization</returns>
public double Normalize(double epsilon = MathUtil.Epsilon)
{
    double length = Length;
    if (length > epsilon) {
        double invLength = 1.0 / length;
        x *= invLength;
        y *= invLength;
        z *= invLength;
        return 1;
    } else {
        x = y = z = 0;
        return 0;
    }
}

Happy to post a PR if you are interested. Best, Claudio

CBenghi commented 4 years ago

Ignore my previous question, I now understand that the return value behavior was intended.

I've added xml comment to my repository as follows:

/// <summary>
/// Sets the lenght of the vector to 1 or 0.
///
/// If the vector's initial length is smaller than <see cref="epsilon"/> the vector is degenerate and 
/// the resulting length is 0, otherwise 1
/// </summary>
/// <param name="epsilon">a value to determine the normalization behaviour threshold</param>
/// <returns>the original length of the vector, before normalization, if this was smaller than <see cref="epsilon"/> then zero is returned.</returns>
public double Normalize(double epsilon = MathUtil.Epsilon)
{
    double length = Length;
    if (length > epsilon)
    {
        double invLength = 1.0 / length;
        x *= invLength;
        y *= invLength;
        z *= invLength;
        return length;
    }
    else
    {
        x = y = z = 0;
        return 0;
    }
}