-
Notifications
You must be signed in to change notification settings - Fork 2
Measurement FormatStyle (#1) #22
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
base: main
Are you sure you want to change the base?
Conversation
* FormatStyle WIP * added FormatStyle tests * updated README and @available --------- Co-authored-by: Jason Jobe <box2019@jasonjobe.com>
NeedleInAJayStack
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea! Thanks for contributing!
| // | ||
| // Formatter.swift | ||
| // Units | ||
| // (aka Fountation.FormatStyle) | ||
| // | ||
| // Created by Jason Jobe on 10/24/25. | ||
| // | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need for this header; we can track dates/contributions using Git.
| public extension Measurement { | ||
| struct Formatter<Output> { | ||
| let format: (Measurement) -> Output | ||
| } | ||
|
|
||
| func formatted<Output>(_ formatter: Formatter<Output>) -> Output { | ||
| formatter.format(self) | ||
| } | ||
|
|
||
| func formatted(_ formatter: Formatter<String> = .measurement()) -> String { | ||
| formatter.format(self) | ||
| } | ||
|
|
||
| func formatted( | ||
| minimumFractionDigits: Int = 0, | ||
| maximumFractionDigits: Int = 4 | ||
| ) -> String { | ||
| Formatter | ||
| .measurement( | ||
| minimumFractionDigits: minimumFractionDigits, | ||
| maximumFractionDigits: maximumFractionDigits) | ||
| .format(self) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about using Swift's built-in NumberFormatter instead of building our own format system? Example:
// Implementation
extension NumberFormatter {
func string(from measurement: Measurement) {
return "\(self.string(from: .init(value: measurement.value)) \(measurement.unit.symbol)"
}
}
// Usage
let measurement = 28.123.measured(in: .meter)
let formatter = NumberFormatter()
formatter.maximumFractionDigits = 2
print(formatter.string(from: measurement)) // Prints `28.12 m`This seems like this approach would inherit very configurable options, while also simplifying the implementation and usage. Thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I was leaning into the newer Format API. The custom implementation is limiting. I'll work on that.
| extension Percent: Equatable { | ||
| /// Implemented as "nearly" equal | ||
| public static func ==(lhs: Percent, rhs: Percent) -> Bool { | ||
| lhs.magnitude >= rhs.magnitude.nextDown | ||
| && lhs.magnitude <= lhs.magnitude.nextUp | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why change Equatable implementation to nearly equal? Is this related to the tests that were added?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The significant digits of a "percent" is usually small so representing them with a Double in the arithmetic can lead to values that might result in something like 37.989488484% leading to comparisons that break our intuition (like == 38%). This felt like a reasonable compromise.
| extension Percent { | ||
| /** | ||
| Returns a random value within the given range. | ||
|
|
||
| ``` | ||
| Percent.random(in: 10%...20%) | ||
| // 10%, 11%, 12%, 19.98%, etc. | ||
| ``` | ||
| */ | ||
| public static func random(in range: ClosedRange<Self>) -> Self { | ||
| self.init(magnitude: .random(in: range.lowerBound.magnitude...range.upperBound.magnitude)) | ||
| } | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Is Percent.random(in: 10%...20%)) a big improvement over Percent(magnitude: .random(in: 10...20))? I'm inclined to think that the existing approach is fine, but I'd be interested in your thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In one sense, it is just syntactic sugar but it makes the intent clearer and more succinct and encapsulates the internals reducing developer cognitive load.
Added Measurement and Percent FormatStyle. Tests and updated README included.