Suggestions for coding conventions

[y2k4life]

The way the code is written might be your preferences and therefore I did not submit a pull request.

1. Namespace - suggest camel case instead of codeclimber.raytracer use CodeClimber.RayTracer
2. There is a mix of code using this. and other parts of code not using this., particularly in Tuple.cs
3. Suggest implementing IEquatable<T> and also implement override bool Equals(object obj) with override int GetHashCode(). This is new for me, so the idea of the suggestion is sound but the implementation might be questionable. At first I was wondering why even have this and the reason is to assume equality for doubles with a particular precision, 4.000001 equals 4.000002 are equal. Performance it part of this hence the duplication of the testing for equality. In examples I have see the equality test is in both override bool Equals(object obj) and bool Equals(T other) instead of having override bool Equals(object obj) call bool Equals(T other) after unboxing. But what is a few milliseconds. See below for implementation.
4. Add a test where values are different yet are equal. Either indirectly with a Tuple having values Tuple(4.3, -4.000001, 3.1, 0) and Tuple(4.3, -4.000002, 3.1, 0) or testing the extension method.

public override int GetHashCode()
{
    return X.GetHashCode() ^ Y.GetHashCode() ^ Z.GetHashCode() ^ W.GetHashCode();
}

public override bool Equals(object obj)
{
    Tuple other = obj as Tuple;
    if (other == null)
    {
        return false;
    }

    return (other.X.EqualsD(X)
        && other.Y.EqualsD(Y)
        && other.Z.EqualsD(Z)
        && other.W.EqualsD(W));
}

public bool Equals(Tuple other)
{
    if (other == null)
    {
        return false;
    }

    return (other.X.EqualsD(X)
        && other.Y.EqualsD(Y)
        && other.Z.EqualsD(Z)
        && other.W.EqualsD(W));
}

[simonech]

Thank you. I indeed wanted to implement “better” the equality.

I’ll fix these in my “refactoring” sprint before going on to matrix ops.

Also the others “code style” issues.

Thx for the PR as well. I’ll merge it later today or tomorrow (on holiday now).

Sent from my iPhone

On 15 Jun 2019, at 21:40, Guy notifications@github.com wrote:

The way the code is written might be your preferences and therefore I did not submit a pull request.

Namespace - suggest camel case instead of codeclimber.raytracer use CodeClimber.RayTracer There is a mix of code using this. and other parts of code not using this., particularly in Tuple.cs Suggest implementing IEquatable and also implement override bool Equals(object obj) with override int GetHashCode(). This is new for me, so the idea of the suggestion is sound but the implementation might be questionable. At first I was wondering why even have this and the reason is to assume equality for doubles with a particular precision, 4.000001 equals 4.000002 are equal. Performance it part of this hence the duplication of the testing for equality. In examples I have see the equality test is in both override bool Equals(object obj) and bool Equals(T other) instead of having override bool Equals(object obj) call bool Equals(T other) after unboxing. But what is a few milliseconds. See below for implementation. Add a test where values are different yet are equal. Either indirectly with a Tuple having values Tuple(4.3, -4.000001, 3.1, 0) and Tuple(4.3, -4.000002, 3.1, 0) or testing the extension method. public override int GetHashCode() { return X.GetHashCode() ^ Y.GetHashCode() ^ Z.GetHashCode() ^ W.GetHashCode(); }

public override bool Equals(object obj) { Tuple other = obj as Tuple; if (other == null) { return false; }

return (other.X.EqualsD(X)
    && other.Y.EqualsD(Y)
    && other.Z.EqualsD(Z)
    && other.W.EqualsD(W));

}

public bool Equals(Tuple other) { if (other == null) { return false; }

return (other.X.EqualsD(X)
    && other.Y.EqualsD(Y)
    && other.Z.EqualsD(Z)
    && other.W.EqualsD(W));

} — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.