Reviews are an important part of the Swift evolution process. All review feedback should be either on this forum thread or, if you would like to keep your feedback private, directly to me as the review manager via email or direct message on the forums. If you send me email, please put "SE-0255" somewhere in the subject line.
What goes into a review of a proposal?
The goal of the review process is to improve the proposal under review through constructive criticism and, eventually, determine the direction of Swift.
When reviewing a proposal, here are some questions to consider:
What is your evaluation of the proposal?
Is the problem being addressed significant enough to warrant a change to Swift?
Does this proposal fit well with the feel and direction of Swift?
If you have used other languages or libraries with a similar feature, how do you feel that this proposal compares to those?
How much effort did you put into your review? A glance, a quick reading, or an in-depth study?
+1. Like hashing, this is an area where most people are probably getting it wrong, and the bugs that result from this error are not consistent and hard to track down. Having an API that makes doing the right thing easy is a clear win.
Big +1. This is definitely tackling a problem that is not trivially implementable by users, and something that 100% should reside in the standard library.
The alternatives considered did a good job of answering the one question I had over why there was a special case for zero.
Ginormous +1. Every programming language should have this built in.
I used to teach intro CS and we had to cover how to compare floating-point values properly. After looking at the implementation here, guess what—we didn't do it properly! We stopped at the naïve "subtract and check if under a tolerance" approach, but that doesn't handle more advanced cases. And that illustrates the point: those edge cases aren't teachable at early stages because this is a deceptively complex problem that requires advanced understanding of FP representation and behavior. Building this notion into the standard library makes Swift much more teachable with respect to floating point numbers.
I'm far more comfortable with someone like @scanon with boundless expertise in this area implementing this once and for all in Swift than doing it myself and hoping other third-party libraries do it correctly as well. That illustrates another issue—putting it in the language itself means you don't have conflicting implementations across multiple libraries (i.e., two APIs that take floats and return different values for approximate equality because they use different tolerances).
This is a useful addition appropriate for the standard library for the reasons articulated by Tony. Even the documentation that will be added as part of this proposal will have important educational value for users and tend to improve the correctness of their code.
One potential improvement to the API would be to rename isAlmostEqual to isApproximately or something like that. When reading the suggested API, I felt that isAlmostEqual feels 'off' in a way I can't really articulate.
Examples below
From the proposal
if x.isAlmostEqual(to: y) {
// equal enough!
}
Suggested
if x.isApproximately(y, tolerance: 0.01) {
// equal enough!
}
The "right" thing to do here is probably outside the scope of what a general library function can implement, and would depend on where the values are coming from.
E.g. if the values being computed slightly perturbed integers and you are testing which integer you almost have? A simple comparison with an absolute tolerance is probably correct--or even just using .round( ) to get the closest integer value.
For other scenarios you probably want to do something else; it's likely outside the scope of situations that this API can cover.
The intended behavior is that infinity be considered--for the purposes of this function--to have what the next larger value than T.greatestFiniteMagnitude would be if it existed. This isn't correct for every case, but is probably the most useful default behavior (in fact, this is basically the only detail that I would really expect to warrant significant discussion--it would certainly be reasonable to ask that nothing be approximately equal to infinity except itself).
Discoverability: Will anyone even think to look for a function to replace what they've always done with fabs(a - b) < THRESHOLD or (shudders) a == b?
Warning: Will there be a compiler warning?
Deprecation: Can/should == be deprecated outright for floats and doubles?
My main concern is that == and the functions will be easily confused, like comparing Java strings with == (i.e. reference equality) vs. equals() (value equality).
Definitely not. Contra what the internet says, comparing floats with exact equality is frequently perfectly appropriate. It's also an IEEE 754 required operation.
A super-simple example of a situation where it's absolutely correct:
func sinc(_ x: Double) -> Double {
// detect x == 0 to avoid returning NaN from sin(x)/x = 0/0.
if x == 0 { return 1 }
return sin(x)/x
}
For using ==? Definitely not. For fabs(a - b) < THRESHOLD? Yes, because fabs is deprecated and renamed abs =). For using an absolute tolerance? No, because that is also sometimes perfectly correct.
Huge +1. I don’t have much of substance to add - @allevato articulated the reasons why this is an important addition. It’s good to see attention on details like this with a focus on helping people avoid common mistakes.
Does assert create a warning before debug compile? If not it would be nice to get a a warning like assertionFailure() does.
// tolerances outside of [.ulpOfOne,1) yield well-defined but useless results,
// so this is enforced by an assert rathern than a precondition.
assert(tolerance >= .ulpOfOne && tolerance < 1, "tolerance should be in [.ulpOfOne, 1).")