Struct for angles

Question

I would like to ask about the code below:

[System.Runtime.InteropServices.StructLayout(System.Runtime.InteropServices.LayoutKind.Sequential)]
public struct AngleDeg
{
    public readonly double Degrees; // immutable struct

    public AngleDeg(double degrees = 0)
    {
        Degrees = degrees;
    }

    public override string ToString()
    {
        return $"{Degrees:F2}°";
    }

    #region Conversion

    public static implicit operator AngleDeg(double rawAngleInDeg)
    {
        return new AngleDeg(rawAngleInDeg);
    }

    public static implicit operator double(AngleDeg v)
    {
        return v.Degrees;
    }

    // TODO: Implement arithmetic operations with operator overloads

    [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)]
    public AngleRad ToRadians()
    {
        // I could have used an explicit cast operator overload, but I think that a simple cast is not verbose/indicative enough.
        return new AngleRad(Degrees / 180.0 * System.Math.PI);
    }

    #endregion
}

[System.Runtime.InteropServices.StructLayout(System.Runtime.InteropServices.LayoutKind.Sequential)]
public struct AngleRad
{
    public readonly double Radians; // immutable struct

    public AngleRad(double radians = 0)
    {
        Radians = radians;
    }

    public override string ToString()
    {
        // I want the debugger to display angles in rounded degrees. Much easier to picture angles-in-degrees in my head
        return $"{Radians / System.Math.PI * 180:F2}°";
    }

    #region Conversion

    public static implicit operator AngleRad(double rawAngleInRad)
    {
        return new AngleRad(rawAngleInRad);
    }

    public static implicit operator double(AngleRad v)
    {
        return v.Radians;
    }

    // TODO: Implement arithmetic operations with operator overloads

    [System.Runtime.CompilerServices.MethodImpl(System.Runtime.CompilerServices.MethodImplOptions.AggressiveInlining)]
    public AngleDeg ToDegrees()
    {
        // I could have used an explicit cast operator overload, but I think that a simple cast is not verbose/indicative enough.
        return new AngleDeg(Radians / System.Math.PI * 180);
    }

    #endregion
}

We have tons of geometry related code in our codebase with lots of calculations including angles. We use double-precision floating point numbers to represent angles right now.

We usually measure angles in radians during the calculations, mainly because they all boil down to System.Math's static methods which expect angles to be in radians. However, it is a nuisance to debug such a code, because a double in the range of 0 and 2PI is nowhere near as informative as an angle in the range of 0 and 360°. Also when these angles displayed to a user, or a user has to type in an angle we always have to use degrees. A conversion must happen somewhere between the GUI-related layers and the maths calculation layers.

These are the pros I can think of:

• ToString() override always displays angle in degrees (great help for the debugger)
• Compile time checking of incommensurate angles on the boundaries of code using degrees and code using radians.
• Can write some handy utility methods, like ClampInRangeOf0And360Degrees().

Possible cons:

• Performance cost? Is there any?
• All codes using these structs will have a dependency on them. Using System.Double is maybe more 'reusable code'

The question is: What do you think of the possible drawbacks, pitfalls if we used these structs in a performance critical application? What about readability and code smell?

Answer 1

IMO you should always use radians internally. Therefore a simple double should be appropriate as type for angle measurements. The only times you should want to convert to degrees, are in the UI. So you'll have to make functionality to convert between radians and degrees (for instance an extension method to double).

A good UI that displays angle measurements should let the user be able to set the format (choose between degrees and radians), because in some situations it is in fact more convenient to view angles in radians. The UI should be able to handle angle input in both degrees and radians according to a general setting.

So, although your structs seem well designed (an immutable struct is a good choice) they are in my opinion more or less redundant and may cause confusion among the developers about what to choose.

---

When it comes to debugging, you could maybe benefit from the possibilities to customize the display by using the DebuggerDisplayAttribute

Answer 2

AngleRad class solves your problem, by displaying radians as degrees in debugger. But what problem is solved by AngleDeg class? I don't see any benifits:

• It makes things more complicated, because now you have two similar classes with very similar names. Easy to confuse them.
• Implicit conversion from double amplifies this confusion. AngleDeg angle = Math.Atan(...) - this is now a valid code. I strongly recommend against such implicit conversion.
• Converting back and forth between two representations slowly builds up a round-off error, that otherwise can be avoided (by making all your calculations using radians).

So bottom line is: I would drop the first class and keep the second one.

P.S. How it would affect performance is something you have to measure on your own. Run a profiler, or write a benchmark that models your calculations. The cost of implicit casting is negligible in most cases, but it can be noticeable if your app, say, multiplies angles 24/7. In that case this cost quickly adds up.

Answer 3

I agree with other comments on the two classes have overlap.

You perform conversion in the ToString() and ToDegrees(). Reference the ToDegrees(). Actually I would just make Degrees a property.

I think one struct Angle with properties Radians and Degrees would be better.

Answer 4

The question is: What do you think of the possible drawbacks, pitfalls if we used these structs in a performance critical application? 

About your ToString methods:

Interpolated strings ($"{variable}...") are the antithesis of performance -- potentially worse than using StringBuilder for a similar construct. Each time the string is interpolated at runtime: a new compile time generated class is instantiated; a closure is created; the operations you expect are performed; and the closure and class instance are cleaned up. If your displaying a single value, this is negligible. If your displaying 1,000s of values (such as for UI display or reporting), this will begin take a hit on the CPU and memory pool. Consider just concatenating the double.ToString output with the degrees symbol.

The default ToString method makes assumptions about the display format. Consider adding an overload taking a string format parameter and passing it to the double.ToString call.

Answer 5

UPDATE

I've gotten several downvotes with no comments for improvement. I think I gave sound advice that it is overkill for LayoutKind.Sequential and AggressiveInlining. I think it's good advice to have a Normalize method - as requested by the OP - which is a better name than ClampInRangeOf0To360Degrees. I also think its good advice that the ToString methods could be more flexible to allow for a varying number of decimal digits. If I am wrong on these points, please educate me.

I am going to assume the silent downvotes were from the code. I have modified the code. The previous constant factors have been made static methods. I trust readers understand that my code is offered as an example of possibilities the OP can explore.

I have nothing to add to Henrik Hansen and Nikita B's answers. I agree with them. I think you have way too much overkill and premature optimization with InteropServices.LayoutKind.Sequential or AggressiveInlining. No reason for LayoutKind if there is really only 1 value object in the struct.

String methods will be slower than purely numeric methods. I find your structs rigid in that they fix the decimals at 2.

I offer an alternative to demonstrate Henrik's and Nikita's answers:

public struct Angle
{
    // You don't have to make this private.  
    // It's offered merely as an alternative to always
    // force someone to use FromRadians or FromDegrees.
    private Angle(double radians)
    {
        Radians = Normalize(radians);
    }

    public double Radians { get; }
    public double Degrees => RadiansToDegrees(Radians);
    public double Gradians => RadiansToGradians(Radians);

    private static double Normalize(double radians) => radians % TwoPI;

    public const double TwoPI = 2 * Math.PI;
    public static double RadiansToDegrees(double radians) => radians * 180 / Math.PI;
    public static double RadiansToGradians(double radians) => radians * 200 / Math.PI;
    public static double DegreesToRadians(double degrees) => degrees * Math.PI / 180;
    public static double GradiansToRadians(double gradians) => gradians * Math.PI / 200;

    public static Angle FromRadians(double radians) => new Angle(radians);
    public static Angle FromDegrees(double degrees) => new Angle(DegreesToRadians(degrees));
    public static Angle FromGradians(double gradians) => new Angle(GradiansToRadians(gradians));

    public override string ToString() => RadiansAsString();
    public string ToString(int decimals) => RadiansAsString(decimals);
    public string RadiansAsString(int decimals = 2) => $"{Radians.ToString($"F{decimals}")} rad";
    public string DegreesAsString(int decimals = 2) => $"{Degrees.ToString($"F{decimals}")}°";
    public string GradiansAsString(int decimals = 2) => $"{Gradians.ToString($"F{decimals}")} g";
}

You only need 1 struct and it should have a simple name: Angle. I have constants defined to replace the need for aggressive inlining. Since you mentioned the need to normalize between 0-360 degrees, I have a Normalize method.

I intentionally use a private constructor so that I force others to use FromRadians, FromDegrees, or even FromGradians. The private constructor is purely optional. The implicit operators are gone. They really can lead to hard-to-find bugs since what double did you want to implicitly convert: radians or degrees? Your code will be more understandable by clearly stating the purpose that its FromDegrees versus FromRadians.

To put it another way, let's say I have a variable named "myDegrees" despite the fact that I hate prefacing variable named with "my". This code would be easy to gloss over:

var angle = new Angle(myDegrees);  // if constructor was public

But this code should catch your eye that there is an issue:

var angle = Angle.FromRadians(myDegrees);

Because there is clear conflict with the names.

My string methods allow for a varying number of decimal digits to be displayed.

There is no error checking and stuff like that in my code.

If you only input and output with degrees

Then you may still have one structure where all internal calculations are based on radians but the inputs and ToString outputs are in your preference of degrees. This also means you could keep an implicit cast from double degrees to Angle. That code could look something like:

public struct Angle
{
    // Prefer input as degrees
    private Angle(double degrees)
    {
        Radians = Normalize(DegreesToRadians(degrees));
    }

    public double Radians { get; }
    public double Degrees => RadiansToDegrees(Radians);
    public double Gradians => RadiansToGradians(Radians);

    private static double Normalize(double radians) => radians % TwoPI;

    public const double TwoPI = 2 * Math.PI;
    public static double RadiansToDegrees(double radians) => radians * 180 / Math.PI;
    public static double RadiansToGradians(double radians) => radians * 200 / Math.PI;
    public static double DegreesToRadians(double degrees) => degrees * Math.PI / 180;
    public static double GradiansToRadians(double gradians) => gradians * Math.PI / 200;

    public static Angle FromRadians(double radians) => new Angle(radians);
    public static Angle FromDegrees(double degrees) => new Angle(DegreesToRadians(degrees));
    public static Angle FromGradians(double gradians) => new Angle(GradiansToRadians(gradians));

    // Prefer output as degrees
    public override string ToString() => DegreesAsString();
    public string ToString(int decimals) => DegreesAsString(decimals);

    public string RadiansAsString(int decimals = 2) => $"{Radians.ToString($"F{decimals}")} rad";
    public string DegreesAsString(int decimals = 2) => $"{Degrees.ToString($"F{decimals}")}°";
    public string GradiansAsString(int decimals = 2) => $"{Gradians.ToString($"F{decimals}")} g";

    // Prefer implicit input of degrees
    public static implicit operator Angle(double degrees) => Angle.FromDegrees(degrees);

    // but internal operations are on radians
    public static Angle operator +(Angle a, Angle b) => Angle.FromRadians(a.Radians + b.Radians);
    public static Angle operator -(Angle a, Angle b) => Angle.FromRadians(a.Radians - b.Radians);
    public static Angle operator *(Angle a, Angle b) => Angle.FromRadians(a.Radians * b.Radians);
    public static Angle operator /(Angle a, Angle b) => Angle.FromRadians(a.Radians / b.Radians);
}

The math operators offer the benefit of creating a new, immutable Angle, which also means the Normalize method is applied with each math operation.