Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Introducing Angle type [Issue #88] #169

Open
wants to merge 23 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 44 additions & 0 deletions Sources/RealModule/Angle.swift
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,50 @@ public extension Angle {
static func atan2(y: T, x: T) -> Self { Angle.radians(T.atan2(y: y, x: x)) }
}

extension Angle: AdditiveArithmetic {
public static var zero: Angle<T> { .radians(0) }

public static func + (lhs: Angle<T>, rhs: Angle<T>) -> Angle<T> {
Angle(radians: lhs.radians + rhs.radians)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems unfortunate to me that adding two angles specified in degrees incurs rounding error seven times if I want the result in degrees. I think this dedicated type needs to store values given in degrees as-is if it offers such arithmetic.

Copy link
Author

@jkalias jkalias Dec 14, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I intended with my earlier comment, so degrees and radians are stored separately.
#169 (comment)

We could then switch on all operations performed on the input and essentially handle three cases:

  • degrees with degrees -> perform operation in degrees
  • radians with radians -> perform operation in radians
  • mixed units -> perform in a common unit

I'm not sure whether people agree on that

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 That would yield a superior result in the case of operations performed entirely in degrees or radians.

I wonder if there are alternative designs that can improve the result when working with both degrees and radians. If the type stored both radians and degrees, for example, then you could store the result of "90 degrees plus 1 radian" exactly. I haven't thought through the remainder of the design, but I offer it for your consideration here.

}

public static func += (lhs: inout Angle<T>, rhs: Angle<T>) {
lhs.radians += rhs.radians
}

public static func - (lhs: Angle<T>, rhs: Angle<T>) -> Angle<T> {
Angle(radians: lhs.radians - rhs.radians)
}

public static func -= (lhs: inout Angle<T>, rhs: Angle<T>) {
lhs.radians -= rhs.radians
}
}

public extension Angle {
static func * (lhs: Angle<T>, rhs: T) -> Angle<T> {
Angle(radians: lhs.radians * rhs)
}

static func *= (lhs: inout Angle<T>, rhs: T) {
lhs.radians *= rhs
}

static func * (lhs: T, rhs: Angle<T>) -> Angle<T> {
Angle(radians: rhs.radians * lhs)
}

static func / (lhs: Angle<T>, rhs: T) -> Angle<T> {
assert(rhs != 0)
return Angle(radians: lhs.radians / rhs)
}

static func /= (lhs: inout Angle<T>, rhs: T) {
assert(rhs != 0)
lhs.radians /= rhs
}
}

private func normalize<T>(_ input: T, limit: T) -> T
where T: Real {
var normalized = input
Expand Down