Struct for angles
Clash Royale CLAN TAG#URR8PPP
.everyoneloves__top-leaderboard:empty,.everyoneloves__mid-leaderboard:empty margin-bottom:0;
up vote
14
down vote
favorite
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?
c# performance object-oriented unit-conversion
add a comment |Â
up vote
14
down vote
favorite
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?
c# performance object-oriented unit-conversion
4
Highly recommend not abbreviatingAngleDeg
andAngleRad
. Are you really saving anything from doing this? Most people would just typeAngleD
and hit tab anyways, soAngleDegree
/AngleRadian
reads properly and doesn't change anything in that case.
â Shelby115
Jun 6 at 12:28
3
This feels like over kill. I would have oneAngle
class with a double Value property and a Type enum for either Degrees or Radians.
â Nkosi
Jun 6 at 13:00
1
Rather thanToDegrees()
andToRadians()
in each others'struct
s, why not have an implicit conversion to/from as you do withdouble
?
â Jesse C. Slicer
Jun 6 at 13:19
in the range of 0 and 2PI [...] in the range of 0 and 360°
Do you allow angles to be negative?
â Raimund Krämer
Jun 6 at 13:34
add a comment |Â
up vote
14
down vote
favorite
up vote
14
down vote
favorite
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?
c# performance object-oriented unit-conversion
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?
c# performance object-oriented unit-conversion
edited Jun 6 at 16:20
200_success
123k14143399
123k14143399
asked Jun 6 at 12:23
user171320
712
712
4
Highly recommend not abbreviatingAngleDeg
andAngleRad
. Are you really saving anything from doing this? Most people would just typeAngleD
and hit tab anyways, soAngleDegree
/AngleRadian
reads properly and doesn't change anything in that case.
â Shelby115
Jun 6 at 12:28
3
This feels like over kill. I would have oneAngle
class with a double Value property and a Type enum for either Degrees or Radians.
â Nkosi
Jun 6 at 13:00
1
Rather thanToDegrees()
andToRadians()
in each others'struct
s, why not have an implicit conversion to/from as you do withdouble
?
â Jesse C. Slicer
Jun 6 at 13:19
in the range of 0 and 2PI [...] in the range of 0 and 360°
Do you allow angles to be negative?
â Raimund Krämer
Jun 6 at 13:34
add a comment |Â
4
Highly recommend not abbreviatingAngleDeg
andAngleRad
. Are you really saving anything from doing this? Most people would just typeAngleD
and hit tab anyways, soAngleDegree
/AngleRadian
reads properly and doesn't change anything in that case.
â Shelby115
Jun 6 at 12:28
3
This feels like over kill. I would have oneAngle
class with a double Value property and a Type enum for either Degrees or Radians.
â Nkosi
Jun 6 at 13:00
1
Rather thanToDegrees()
andToRadians()
in each others'struct
s, why not have an implicit conversion to/from as you do withdouble
?
â Jesse C. Slicer
Jun 6 at 13:19
in the range of 0 and 2PI [...] in the range of 0 and 360°
Do you allow angles to be negative?
â Raimund Krämer
Jun 6 at 13:34
4
4
Highly recommend not abbreviating
AngleDeg
and AngleRad
. Are you really saving anything from doing this? Most people would just type AngleD
and hit tab anyways, so AngleDegree
/AngleRadian
reads properly and doesn't change anything in that case.â Shelby115
Jun 6 at 12:28
Highly recommend not abbreviating
AngleDeg
and AngleRad
. Are you really saving anything from doing this? Most people would just type AngleD
and hit tab anyways, so AngleDegree
/AngleRadian
reads properly and doesn't change anything in that case.â Shelby115
Jun 6 at 12:28
3
3
This feels like over kill. I would have one
Angle
class with a double Value property and a Type enum for either Degrees or Radians.â Nkosi
Jun 6 at 13:00
This feels like over kill. I would have one
Angle
class with a double Value property and a Type enum for either Degrees or Radians.â Nkosi
Jun 6 at 13:00
1
1
Rather than
ToDegrees()
and ToRadians()
in each others' struct
s, why not have an implicit conversion to/from as you do with double
?â Jesse C. Slicer
Jun 6 at 13:19
Rather than
ToDegrees()
and ToRadians()
in each others' struct
s, why not have an implicit conversion to/from as you do with double
?â Jesse C. Slicer
Jun 6 at 13:19
in the range of 0 and 2PI [...] in the range of 0 and 360°
Do you allow angles to be negative?â Raimund Krämer
Jun 6 at 13:34
in the range of 0 and 2PI [...] in the range of 0 and 360°
Do you allow angles to be negative?â Raimund Krämer
Jun 6 at 13:34
add a comment |Â
5 Answers
5
active
oldest
votes
up vote
8
down vote
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
1
I would however argue that reading code likeAngle toWaypoint
is more clean/descriptive thandouble angleToWaypoint
, but agree that having two structs might be confusing.
â D. Ben Knoble
Jun 6 at 14:07
@D.BenKnoble: You may have a point. But you can use the same argument on length and other measurements:Length lengthToPoint
and then it could be a mess IMO.
â Henrik Hansen
Jun 6 at 15:15
Fair point Henrik
â D. Ben Knoble
Jun 6 at 15:15
1
You could always just overrideToString
for the debugger as well.
â Shelby115
Jun 6 at 18:36
add a comment |Â
up vote
7
down vote
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.
1
Agree with implicit conversion. Would suggest there be static methods named FromDegrees and FromRadians, though FromRadians is same as doing new instance. This would be similar to a TimeSpan.FromMinutes.
â Rick Davin
Jun 6 at 17:44
add a comment |Â
up vote
2
down vote
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.
add a comment |Â
up vote
2
down vote
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.
1
You mentioned performance and interpolated strings ($"variable...") are the antithesis of performance where did you get this from? OP doesn't mention that :-|
â t3chb0t
Jun 7 at 6:19
I think OP is concerned with the performance of actual calculations.ToString
method takes no part in those and is unlikely to become a bottleneck. So, personally, in this case I would go for "prettier" code.
â Nikita B
Jun 7 at 7:29
1
Why do you think that Interpolated strings ($"variable...") are the antithesis of performance -- potentially worse than using StringBuilder for a similar construct - do you have any proof for that? They are just a syntactic sugar. The compiler will generate the same code as if you usedstring.Format
and if you are not doing this in a loop there will hardly be any difference noticable.
â t3chb0t
Jun 7 at 15:43
3
@t3chb0t Upon re-reading the documentation, I had overthought the FormattableString and IFormattable implicit conversions. My understanding was that an IFormattable implementation was always generated and run. I have confirmed your statements and am striking my performance claims.
â psaxton
Jun 7 at 17:14
add a comment |Â
up vote
1
down vote
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($"Fdecimals") rad";
public string DegreesAsString(int decimals = 2) => $"Degrees.ToString($"Fdecimals")ð";
public string GradiansAsString(int decimals = 2) => $"Gradians.ToString($"Fdecimals") 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($"Fdecimals") rad";
public string DegreesAsString(int decimals = 2) => $"Degrees.ToString($"Fdecimals")ð";
public string GradiansAsString(int decimals = 2) => $"Gradians.ToString($"Fdecimals") 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.
How come the Radians property is private?
â Pod
Jun 8 at 9:14
1
@Pod. That would be a typo. Fixed. Thx.
â Rick Davin
Jun 8 at 11:13
I'd written a comment but it was removed... and at that time I didn't vote at all so the first DV wasn't mine (but the second one is). I said that I didn't like calling the code an alternative which it isn't because it just implements suggestions from two different answers. If you could make it clear which part of the review is new and which one just shows an example implementation of the already mentioned improvements I'll gladly upvote the answer.
â t3chb0t
Jun 9 at 9:40
@t3chb0t Thanks for feedback. The mention of it being an alternative was in reference to the OP's code, not to other answers. I disagree that it "just implements" two other answers, since it also implements points that I was first to mention. Those are summarized in the updated first paragraph.
â Rick Davin
Jun 9 at 12:48
add a comment |Â
5 Answers
5
active
oldest
votes
5 Answers
5
active
oldest
votes
active
oldest
votes
active
oldest
votes
up vote
8
down vote
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
1
I would however argue that reading code likeAngle toWaypoint
is more clean/descriptive thandouble angleToWaypoint
, but agree that having two structs might be confusing.
â D. Ben Knoble
Jun 6 at 14:07
@D.BenKnoble: You may have a point. But you can use the same argument on length and other measurements:Length lengthToPoint
and then it could be a mess IMO.
â Henrik Hansen
Jun 6 at 15:15
Fair point Henrik
â D. Ben Knoble
Jun 6 at 15:15
1
You could always just overrideToString
for the debugger as well.
â Shelby115
Jun 6 at 18:36
add a comment |Â
up vote
8
down vote
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
1
I would however argue that reading code likeAngle toWaypoint
is more clean/descriptive thandouble angleToWaypoint
, but agree that having two structs might be confusing.
â D. Ben Knoble
Jun 6 at 14:07
@D.BenKnoble: You may have a point. But you can use the same argument on length and other measurements:Length lengthToPoint
and then it could be a mess IMO.
â Henrik Hansen
Jun 6 at 15:15
Fair point Henrik
â D. Ben Knoble
Jun 6 at 15:15
1
You could always just overrideToString
for the debugger as well.
â Shelby115
Jun 6 at 18:36
add a comment |Â
up vote
8
down vote
up vote
8
down vote
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
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
edited Jun 8 at 6:59
answered Jun 6 at 13:11
Henrik Hansen
3,7981417
3,7981417
1
I would however argue that reading code likeAngle toWaypoint
is more clean/descriptive thandouble angleToWaypoint
, but agree that having two structs might be confusing.
â D. Ben Knoble
Jun 6 at 14:07
@D.BenKnoble: You may have a point. But you can use the same argument on length and other measurements:Length lengthToPoint
and then it could be a mess IMO.
â Henrik Hansen
Jun 6 at 15:15
Fair point Henrik
â D. Ben Knoble
Jun 6 at 15:15
1
You could always just overrideToString
for the debugger as well.
â Shelby115
Jun 6 at 18:36
add a comment |Â
1
I would however argue that reading code likeAngle toWaypoint
is more clean/descriptive thandouble angleToWaypoint
, but agree that having two structs might be confusing.
â D. Ben Knoble
Jun 6 at 14:07
@D.BenKnoble: You may have a point. But you can use the same argument on length and other measurements:Length lengthToPoint
and then it could be a mess IMO.
â Henrik Hansen
Jun 6 at 15:15
Fair point Henrik
â D. Ben Knoble
Jun 6 at 15:15
1
You could always just overrideToString
for the debugger as well.
â Shelby115
Jun 6 at 18:36
1
1
I would however argue that reading code like
Angle toWaypoint
is more clean/descriptive than double angleToWaypoint
, but agree that having two structs might be confusing.â D. Ben Knoble
Jun 6 at 14:07
I would however argue that reading code like
Angle toWaypoint
is more clean/descriptive than double angleToWaypoint
, but agree that having two structs might be confusing.â D. Ben Knoble
Jun 6 at 14:07
@D.BenKnoble: You may have a point. But you can use the same argument on length and other measurements:
Length lengthToPoint
and then it could be a mess IMO.â Henrik Hansen
Jun 6 at 15:15
@D.BenKnoble: You may have a point. But you can use the same argument on length and other measurements:
Length lengthToPoint
and then it could be a mess IMO.â Henrik Hansen
Jun 6 at 15:15
Fair point Henrik
â D. Ben Knoble
Jun 6 at 15:15
Fair point Henrik
â D. Ben Knoble
Jun 6 at 15:15
1
1
You could always just override
ToString
for the debugger as well.â Shelby115
Jun 6 at 18:36
You could always just override
ToString
for the debugger as well.â Shelby115
Jun 6 at 18:36
add a comment |Â
up vote
7
down vote
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.
1
Agree with implicit conversion. Would suggest there be static methods named FromDegrees and FromRadians, though FromRadians is same as doing new instance. This would be similar to a TimeSpan.FromMinutes.
â Rick Davin
Jun 6 at 17:44
add a comment |Â
up vote
7
down vote
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.
1
Agree with implicit conversion. Would suggest there be static methods named FromDegrees and FromRadians, though FromRadians is same as doing new instance. This would be similar to a TimeSpan.FromMinutes.
â Rick Davin
Jun 6 at 17:44
add a comment |Â
up vote
7
down vote
up vote
7
down vote
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.
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.
edited Jun 6 at 15:38
answered Jun 6 at 15:21
Nikita B
12.3k11551
12.3k11551
1
Agree with implicit conversion. Would suggest there be static methods named FromDegrees and FromRadians, though FromRadians is same as doing new instance. This would be similar to a TimeSpan.FromMinutes.
â Rick Davin
Jun 6 at 17:44
add a comment |Â
1
Agree with implicit conversion. Would suggest there be static methods named FromDegrees and FromRadians, though FromRadians is same as doing new instance. This would be similar to a TimeSpan.FromMinutes.
â Rick Davin
Jun 6 at 17:44
1
1
Agree with implicit conversion. Would suggest there be static methods named FromDegrees and FromRadians, though FromRadians is same as doing new instance. This would be similar to a TimeSpan.FromMinutes.
â Rick Davin
Jun 6 at 17:44
Agree with implicit conversion. Would suggest there be static methods named FromDegrees and FromRadians, though FromRadians is same as doing new instance. This would be similar to a TimeSpan.FromMinutes.
â Rick Davin
Jun 6 at 17:44
add a comment |Â
up vote
2
down vote
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.
add a comment |Â
up vote
2
down vote
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.
add a comment |Â
up vote
2
down vote
up vote
2
down vote
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.
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.
answered Jun 6 at 16:15
paparazzo
4,8131730
4,8131730
add a comment |Â
add a comment |Â
up vote
2
down vote
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.
1
You mentioned performance and interpolated strings ($"variable...") are the antithesis of performance where did you get this from? OP doesn't mention that :-|
â t3chb0t
Jun 7 at 6:19
I think OP is concerned with the performance of actual calculations.ToString
method takes no part in those and is unlikely to become a bottleneck. So, personally, in this case I would go for "prettier" code.
â Nikita B
Jun 7 at 7:29
1
Why do you think that Interpolated strings ($"variable...") are the antithesis of performance -- potentially worse than using StringBuilder for a similar construct - do you have any proof for that? They are just a syntactic sugar. The compiler will generate the same code as if you usedstring.Format
and if you are not doing this in a loop there will hardly be any difference noticable.
â t3chb0t
Jun 7 at 15:43
3
@t3chb0t Upon re-reading the documentation, I had overthought the FormattableString and IFormattable implicit conversions. My understanding was that an IFormattable implementation was always generated and run. I have confirmed your statements and am striking my performance claims.
â psaxton
Jun 7 at 17:14
add a comment |Â
up vote
2
down vote
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.
1
You mentioned performance and interpolated strings ($"variable...") are the antithesis of performance where did you get this from? OP doesn't mention that :-|
â t3chb0t
Jun 7 at 6:19
I think OP is concerned with the performance of actual calculations.ToString
method takes no part in those and is unlikely to become a bottleneck. So, personally, in this case I would go for "prettier" code.
â Nikita B
Jun 7 at 7:29
1
Why do you think that Interpolated strings ($"variable...") are the antithesis of performance -- potentially worse than using StringBuilder for a similar construct - do you have any proof for that? They are just a syntactic sugar. The compiler will generate the same code as if you usedstring.Format
and if you are not doing this in a loop there will hardly be any difference noticable.
â t3chb0t
Jun 7 at 15:43
3
@t3chb0t Upon re-reading the documentation, I had overthought the FormattableString and IFormattable implicit conversions. My understanding was that an IFormattable implementation was always generated and run. I have confirmed your statements and am striking my performance claims.
â psaxton
Jun 7 at 17:14
add a comment |Â
up vote
2
down vote
up vote
2
down vote
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.
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.
edited Jun 7 at 17:18
answered Jun 6 at 22:28
psaxton
80938
80938
1
You mentioned performance and interpolated strings ($"variable...") are the antithesis of performance where did you get this from? OP doesn't mention that :-|
â t3chb0t
Jun 7 at 6:19
I think OP is concerned with the performance of actual calculations.ToString
method takes no part in those and is unlikely to become a bottleneck. So, personally, in this case I would go for "prettier" code.
â Nikita B
Jun 7 at 7:29
1
Why do you think that Interpolated strings ($"variable...") are the antithesis of performance -- potentially worse than using StringBuilder for a similar construct - do you have any proof for that? They are just a syntactic sugar. The compiler will generate the same code as if you usedstring.Format
and if you are not doing this in a loop there will hardly be any difference noticable.
â t3chb0t
Jun 7 at 15:43
3
@t3chb0t Upon re-reading the documentation, I had overthought the FormattableString and IFormattable implicit conversions. My understanding was that an IFormattable implementation was always generated and run. I have confirmed your statements and am striking my performance claims.
â psaxton
Jun 7 at 17:14
add a comment |Â
1
You mentioned performance and interpolated strings ($"variable...") are the antithesis of performance where did you get this from? OP doesn't mention that :-|
â t3chb0t
Jun 7 at 6:19
I think OP is concerned with the performance of actual calculations.ToString
method takes no part in those and is unlikely to become a bottleneck. So, personally, in this case I would go for "prettier" code.
â Nikita B
Jun 7 at 7:29
1
Why do you think that Interpolated strings ($"variable...") are the antithesis of performance -- potentially worse than using StringBuilder for a similar construct - do you have any proof for that? They are just a syntactic sugar. The compiler will generate the same code as if you usedstring.Format
and if you are not doing this in a loop there will hardly be any difference noticable.
â t3chb0t
Jun 7 at 15:43
3
@t3chb0t Upon re-reading the documentation, I had overthought the FormattableString and IFormattable implicit conversions. My understanding was that an IFormattable implementation was always generated and run. I have confirmed your statements and am striking my performance claims.
â psaxton
Jun 7 at 17:14
1
1
You mentioned performance and interpolated strings ($"variable...") are the antithesis of performance where did you get this from? OP doesn't mention that :-|
â t3chb0t
Jun 7 at 6:19
You mentioned performance and interpolated strings ($"variable...") are the antithesis of performance where did you get this from? OP doesn't mention that :-|
â t3chb0t
Jun 7 at 6:19
I think OP is concerned with the performance of actual calculations.
ToString
method takes no part in those and is unlikely to become a bottleneck. So, personally, in this case I would go for "prettier" code.â Nikita B
Jun 7 at 7:29
I think OP is concerned with the performance of actual calculations.
ToString
method takes no part in those and is unlikely to become a bottleneck. So, personally, in this case I would go for "prettier" code.â Nikita B
Jun 7 at 7:29
1
1
Why do you think that Interpolated strings ($"variable...") are the antithesis of performance -- potentially worse than using StringBuilder for a similar construct - do you have any proof for that? They are just a syntactic sugar. The compiler will generate the same code as if you used
string.Format
and if you are not doing this in a loop there will hardly be any difference noticable.â t3chb0t
Jun 7 at 15:43
Why do you think that Interpolated strings ($"variable...") are the antithesis of performance -- potentially worse than using StringBuilder for a similar construct - do you have any proof for that? They are just a syntactic sugar. The compiler will generate the same code as if you used
string.Format
and if you are not doing this in a loop there will hardly be any difference noticable.â t3chb0t
Jun 7 at 15:43
3
3
@t3chb0t Upon re-reading the documentation, I had overthought the FormattableString and IFormattable implicit conversions. My understanding was that an IFormattable implementation was always generated and run. I have confirmed your statements and am striking my performance claims.
â psaxton
Jun 7 at 17:14
@t3chb0t Upon re-reading the documentation, I had overthought the FormattableString and IFormattable implicit conversions. My understanding was that an IFormattable implementation was always generated and run. I have confirmed your statements and am striking my performance claims.
â psaxton
Jun 7 at 17:14
add a comment |Â
up vote
1
down vote
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($"Fdecimals") rad";
public string DegreesAsString(int decimals = 2) => $"Degrees.ToString($"Fdecimals")ð";
public string GradiansAsString(int decimals = 2) => $"Gradians.ToString($"Fdecimals") 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($"Fdecimals") rad";
public string DegreesAsString(int decimals = 2) => $"Degrees.ToString($"Fdecimals")ð";
public string GradiansAsString(int decimals = 2) => $"Gradians.ToString($"Fdecimals") 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.
How come the Radians property is private?
â Pod
Jun 8 at 9:14
1
@Pod. That would be a typo. Fixed. Thx.
â Rick Davin
Jun 8 at 11:13
I'd written a comment but it was removed... and at that time I didn't vote at all so the first DV wasn't mine (but the second one is). I said that I didn't like calling the code an alternative which it isn't because it just implements suggestions from two different answers. If you could make it clear which part of the review is new and which one just shows an example implementation of the already mentioned improvements I'll gladly upvote the answer.
â t3chb0t
Jun 9 at 9:40
@t3chb0t Thanks for feedback. The mention of it being an alternative was in reference to the OP's code, not to other answers. I disagree that it "just implements" two other answers, since it also implements points that I was first to mention. Those are summarized in the updated first paragraph.
â Rick Davin
Jun 9 at 12:48
add a comment |Â
up vote
1
down vote
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($"Fdecimals") rad";
public string DegreesAsString(int decimals = 2) => $"Degrees.ToString($"Fdecimals")ð";
public string GradiansAsString(int decimals = 2) => $"Gradians.ToString($"Fdecimals") 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($"Fdecimals") rad";
public string DegreesAsString(int decimals = 2) => $"Degrees.ToString($"Fdecimals")ð";
public string GradiansAsString(int decimals = 2) => $"Gradians.ToString($"Fdecimals") 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.
How come the Radians property is private?
â Pod
Jun 8 at 9:14
1
@Pod. That would be a typo. Fixed. Thx.
â Rick Davin
Jun 8 at 11:13
I'd written a comment but it was removed... and at that time I didn't vote at all so the first DV wasn't mine (but the second one is). I said that I didn't like calling the code an alternative which it isn't because it just implements suggestions from two different answers. If you could make it clear which part of the review is new and which one just shows an example implementation of the already mentioned improvements I'll gladly upvote the answer.
â t3chb0t
Jun 9 at 9:40
@t3chb0t Thanks for feedback. The mention of it being an alternative was in reference to the OP's code, not to other answers. I disagree that it "just implements" two other answers, since it also implements points that I was first to mention. Those are summarized in the updated first paragraph.
â Rick Davin
Jun 9 at 12:48
add a comment |Â
up vote
1
down vote
up vote
1
down vote
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($"Fdecimals") rad";
public string DegreesAsString(int decimals = 2) => $"Degrees.ToString($"Fdecimals")ð";
public string GradiansAsString(int decimals = 2) => $"Gradians.ToString($"Fdecimals") 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($"Fdecimals") rad";
public string DegreesAsString(int decimals = 2) => $"Degrees.ToString($"Fdecimals")ð";
public string GradiansAsString(int decimals = 2) => $"Gradians.ToString($"Fdecimals") 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.
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($"Fdecimals") rad";
public string DegreesAsString(int decimals = 2) => $"Degrees.ToString($"Fdecimals")ð";
public string GradiansAsString(int decimals = 2) => $"Gradians.ToString($"Fdecimals") 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($"Fdecimals") rad";
public string DegreesAsString(int decimals = 2) => $"Degrees.ToString($"Fdecimals")ð";
public string GradiansAsString(int decimals = 2) => $"Gradians.ToString($"Fdecimals") 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.
edited Jun 8 at 13:26
answered Jun 6 at 21:28
Rick Davin
2,897618
2,897618
How come the Radians property is private?
â Pod
Jun 8 at 9:14
1
@Pod. That would be a typo. Fixed. Thx.
â Rick Davin
Jun 8 at 11:13
I'd written a comment but it was removed... and at that time I didn't vote at all so the first DV wasn't mine (but the second one is). I said that I didn't like calling the code an alternative which it isn't because it just implements suggestions from two different answers. If you could make it clear which part of the review is new and which one just shows an example implementation of the already mentioned improvements I'll gladly upvote the answer.
â t3chb0t
Jun 9 at 9:40
@t3chb0t Thanks for feedback. The mention of it being an alternative was in reference to the OP's code, not to other answers. I disagree that it "just implements" two other answers, since it also implements points that I was first to mention. Those are summarized in the updated first paragraph.
â Rick Davin
Jun 9 at 12:48
add a comment |Â
How come the Radians property is private?
â Pod
Jun 8 at 9:14
1
@Pod. That would be a typo. Fixed. Thx.
â Rick Davin
Jun 8 at 11:13
I'd written a comment but it was removed... and at that time I didn't vote at all so the first DV wasn't mine (but the second one is). I said that I didn't like calling the code an alternative which it isn't because it just implements suggestions from two different answers. If you could make it clear which part of the review is new and which one just shows an example implementation of the already mentioned improvements I'll gladly upvote the answer.
â t3chb0t
Jun 9 at 9:40
@t3chb0t Thanks for feedback. The mention of it being an alternative was in reference to the OP's code, not to other answers. I disagree that it "just implements" two other answers, since it also implements points that I was first to mention. Those are summarized in the updated first paragraph.
â Rick Davin
Jun 9 at 12:48
How come the Radians property is private?
â Pod
Jun 8 at 9:14
How come the Radians property is private?
â Pod
Jun 8 at 9:14
1
1
@Pod. That would be a typo. Fixed. Thx.
â Rick Davin
Jun 8 at 11:13
@Pod. That would be a typo. Fixed. Thx.
â Rick Davin
Jun 8 at 11:13
I'd written a comment but it was removed... and at that time I didn't vote at all so the first DV wasn't mine (but the second one is). I said that I didn't like calling the code an alternative which it isn't because it just implements suggestions from two different answers. If you could make it clear which part of the review is new and which one just shows an example implementation of the already mentioned improvements I'll gladly upvote the answer.
â t3chb0t
Jun 9 at 9:40
I'd written a comment but it was removed... and at that time I didn't vote at all so the first DV wasn't mine (but the second one is). I said that I didn't like calling the code an alternative which it isn't because it just implements suggestions from two different answers. If you could make it clear which part of the review is new and which one just shows an example implementation of the already mentioned improvements I'll gladly upvote the answer.
â t3chb0t
Jun 9 at 9:40
@t3chb0t Thanks for feedback. The mention of it being an alternative was in reference to the OP's code, not to other answers. I disagree that it "just implements" two other answers, since it also implements points that I was first to mention. Those are summarized in the updated first paragraph.
â Rick Davin
Jun 9 at 12:48
@t3chb0t Thanks for feedback. The mention of it being an alternative was in reference to the OP's code, not to other answers. I disagree that it "just implements" two other answers, since it also implements points that I was first to mention. Those are summarized in the updated first paragraph.
â Rick Davin
Jun 9 at 12:48
add a comment |Â
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
StackExchange.ready(
function ()
StackExchange.openid.initPostLogin('.new-post-login', 'https%3a%2f%2fcodereview.stackexchange.com%2fquestions%2f195952%2fstruct-for-angles%23new-answer', 'question_page');
);
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Post as a guest
Sign up or log in
StackExchange.ready(function ()
StackExchange.helpers.onClickDraftSave('#login-link');
);
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
Sign up using Google
Sign up using Facebook
Sign up using Email and Password
4
Highly recommend not abbreviating
AngleDeg
andAngleRad
. Are you really saving anything from doing this? Most people would just typeAngleD
and hit tab anyways, soAngleDegree
/AngleRadian
reads properly and doesn't change anything in that case.â Shelby115
Jun 6 at 12:28
3
This feels like over kill. I would have one
Angle
class with a double Value property and a Type enum for either Degrees or Radians.â Nkosi
Jun 6 at 13:00
1
Rather than
ToDegrees()
andToRadians()
in each others'struct
s, why not have an implicit conversion to/from as you do withdouble
?â Jesse C. Slicer
Jun 6 at 13:19
in the range of 0 and 2PI [...] in the range of 0 and 360°
Do you allow angles to be negative?â Raimund Krämer
Jun 6 at 13:34